fix(office): make repeat task reassignment work again after 24 hours (#3533)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Henry
2026-09-13 17:24:16 +08:00
committed by GitHub
parent b110796226
commit bd7974dc8d
90 changed files with 5149 additions and 365 deletions
@@ -17,6 +17,7 @@ import (
officeroutines "github.com/kandev/kandev/internal/office/routines"
officeservice "github.com/kandev/kandev/internal/office/service"
officewakeup "github.com/kandev/kandev/internal/office/wakeup"
runsservice "github.com/kandev/kandev/internal/runs/service"
"github.com/kandev/kandev/internal/task/models"
tasksqlite "github.com/kandev/kandev/internal/task/repository/sqlite"
taskservice "github.com/kandev/kandev/internal/task/service"
@@ -267,7 +268,12 @@ func (a *routineWakeupAdapter) CreateWakeupRequest(
}
if err := a.repo.CreateWakeupRequest(ctx, row); err != nil {
if errors.Is(err, officesqlite.ErrWakeupIdempotencyConflict) {
return officeroutines.ErrWakeupAlreadyRequested
runsservice.ReportDurableDedup(runsservice.QueueSourceWakeup, req.Reason, req.IdempotencyKey, req.AgentProfileID)
// Wraps both sentinels so a caller can check either: routines
// callers key off ErrWakeupAlreadyRequested to treat this as
// success by another route, while errors.Is against the
// sqlite-layer ErrWakeupIdempotencyConflict still matches.
return fmt.Errorf("%w: %w", officeroutines.ErrWakeupAlreadyRequested, err)
}
return err
}
@@ -0,0 +1,97 @@
package backendapp
import (
"context"
"errors"
"expvar"
"strings"
"testing"
"github.com/jmoiron/sqlx"
_ "github.com/mattn/go-sqlite3"
settingsstore "github.com/kandev/kandev/internal/agent/settings/store"
officesqlite "github.com/kandev/kandev/internal/office/repository/sqlite"
officeroutines "github.com/kandev/kandev/internal/office/routines"
)
func newTestRoutineWakeupAdapter(t *testing.T) *routineWakeupAdapter {
t.Helper()
db, err := sqlx.Open("sqlite3", ":memory:")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
t.Cleanup(func() { _ = db.Close() })
if _, _, err := settingsstore.Provide(db, db, nil); err != nil {
t.Fatalf("settings store init: %v", err)
}
repo, err := officesqlite.NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("new office repo: %v", err)
}
return &routineWakeupAdapter{repo: repo}
}
// AC-OFFICE-RUN-DEDUP-004.6: a durable idempotency conflict on the wakeup
// queue moves office_run_dedup_total{queue="wakeup",kind="durable"} through
// ReportDurableDedup, not ReportInsertResult - the adapter does its own
// errors.Is on the repository's sentinel and reports the conflict itself.
func TestRoutineWakeupAdapter_CreateWakeupRequest_DurableConflictReportsCounter(t *testing.T) {
adapter := newTestRoutineWakeupAdapter(t)
ctx := context.Background()
reason := "test_routine_wakeup_conflict_" + t.Name()
key := "routine:r1:t1:tick:1234567890"
first := &officeroutines.WakeupRequest{
ID: "wakeup-1", AgentProfileID: "agent-1", Source: "cron",
Reason: reason, IdempotencyKey: key,
}
if err := adapter.CreateWakeupRequest(ctx, first); err != nil {
t.Fatalf("first create: %v", err)
}
second := &officeroutines.WakeupRequest{
ID: "wakeup-2", AgentProfileID: "agent-1", Source: "cron",
Reason: reason, IdempotencyKey: key,
}
err := adapter.CreateWakeupRequest(ctx, second)
if !errors.Is(err, officesqlite.ErrWakeupIdempotencyConflict) {
t.Fatalf("second create error = %v, want ErrWakeupIdempotencyConflict", err)
}
// reason isn't in runs/service's bounded metricReasons allowlist, so the
// label buckets it to "custom" rather than carrying it verbatim.
if !counterHasLabel(t, "office_run_dedup_total", "reason=custom", "kind=durable", "queue=wakeup") {
t.Fatal("expected office_run_dedup_total to carry a custom/durable/wakeup entry")
}
}
// counterHasLabel reports whether the named expvar.Map has an entry whose
// key contains every given substring. Counters are process-global expvar
// state (no reset hook), so tests assert presence/growth rather than exact
// values that could collide with other tests in the same binary run.
func counterHasLabel(t *testing.T, mapName string, substrs ...string) bool {
t.Helper()
v := expvar.Get(mapName)
if v == nil {
t.Fatalf("expvar map %q not registered", mapName)
}
m, ok := v.(*expvar.Map)
if !ok {
t.Fatalf("expvar %q is not a *expvar.Map", mapName)
}
found := false
m.Do(func(kv expvar.KeyValue) {
matches := true
for _, s := range substrs {
if !strings.Contains(kv.Key, s) {
matches = false
break
}
}
if matches {
found = true
}
})
return found
}
@@ -18,6 +18,7 @@ import (
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/repository/sqlite"
"github.com/kandev/kandev/internal/office/shared"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// approvalHandlerFixture wires the minimal stack needed to exercise the
@@ -72,7 +73,9 @@ func (s *silentActivityLogger) LogActivityWithRun(_ context.Context, _, _, _, _,
type silentRunQueuer struct{}
func (s *silentRunQueuer) QueueRun(_ context.Context, _, _, _, _ string) error { return nil }
func (s *silentRunQueuer) QueueRun(_ context.Context, _, _, _, _ string) (runsservice.QueueOutcome, error) {
return runsservice.QueueOutcomeQueued, nil
}
// seedApprovalAgent creates an agent_profiles row with the given role
// and permissions, returning the persisted instance.
@@ -9,6 +9,7 @@ import (
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/shared"
runsservice "github.com/kandev/kandev/internal/runs/service"
"go.uber.org/zap"
)
@@ -29,7 +30,7 @@ type AgentWriter interface {
// RunQueuer enqueues run requests for agent instances.
type RunQueuer interface {
QueueRun(ctx context.Context, agentInstanceID, reason, payload, idempotencyKey string) error
QueueRun(ctx context.Context, agentInstanceID, reason, payload, idempotencyKey string) (runsservice.QueueOutcome, error)
}
// ApprovalService handles approval CRUD and decide logic.
@@ -215,6 +216,7 @@ func (s *ApprovalService) queueApprovalRun(ctx context.Context, approval *Approv
approval.ID, approval.Type, approval.Status, approval.DecisionNote,
)
idempotencyKey := "approval:" + approval.ID
return s.runs.QueueRun(ctx, approval.RequestedByAgentProfileID,
_, err := s.runs.QueueRun(ctx, approval.RequestedByAgentProfileID,
"approval_resolved", payload, idempotencyKey)
return err
}
@@ -11,6 +11,7 @@ import (
"github.com/kandev/kandev/internal/office/approvals"
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/repository/sqlite"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// noopActivityLogger implements shared.ActivityLogger without importing shared.
@@ -23,7 +24,9 @@ func (n *noopActivityLogger) LogActivityWithRun(_ context.Context, _, _, _, _, _
// noopRunQueuer implements approvals.RunQueuer as a no-op.
type noopRunQueuer struct{}
func (n *noopRunQueuer) QueueRun(_ context.Context, _, _, _, _ string) error { return nil }
func (n *noopRunQueuer) QueueRun(_ context.Context, _, _, _, _ string) (runsservice.QueueOutcome, error) {
return runsservice.QueueOutcomeQueued, nil
}
type fakeAgentWriter struct {
statuses map[string]string
@@ -103,7 +103,7 @@ func (s *ChannelService) SetupChannel(ctx context.Context, channel *models.Chann
// workflow_step_participants. Channel tasks have no workflow_step_id;
// the participant row is keyed at (step_id="", task_id) and the
// runner projection still resolves it.
if err := s.repo.UpdateTaskAssignee(ctx, taskID, channel.AgentProfileID); err != nil {
if _, err := s.repo.UpdateTaskAssignee(ctx, taskID, channel.AgentProfileID); err != nil {
_ = s.repo.DeleteChannel(ctx, channel.ID)
return fmt.Errorf("set channel task assignee: %w", err)
}
@@ -141,6 +141,7 @@ func newTestDeps(t *testing.T) *testDeps {
project_id TEXT DEFAULT '',
assignee_agent_profile_id TEXT DEFAULT '',
assignee_user_id TEXT NOT NULL DEFAULT '',
assignment_generation INTEGER NOT NULL DEFAULT 0,
labels TEXT DEFAULT '[]',
metadata TEXT DEFAULT '{}',
identifier TEXT DEFAULT '',
@@ -74,8 +74,11 @@ type Repository interface {
GetRunsByCommentIDs(ctx context.Context, commentIDs []string) (map[string]sqlite.CommentRunStatus, error)
UpdateTaskState(ctx context.Context, taskID, state string) error
GetTaskExecutionFields(ctx context.Context, taskID string) (*sqlite.TaskExecutionFields, error)
// UpdateTaskAssignee returns the task's assignment_generation after the
// bump, read back inside the same transaction that wrote the runner
// seat (see the sqlite implementation's doc comment).
UpdateTaskAssignee(ctx context.Context, taskID, assigneeID string) (int64, error)
UpdateTaskStateIfWorkflowStep(ctx context.Context, taskID, expectedStepID, state string) (bool, error)
UpdateTaskAssignee(ctx context.Context, taskID, assigneeID string) error
UpdateTaskPriority(ctx context.Context, taskID, priority string) error
UpdateTaskProjectID(ctx context.Context, taskID, projectID string) error
GetTaskProjectID(ctx context.Context, taskID string) (string, error)
@@ -270,6 +273,11 @@ type MarkFixedHandler interface {
type TaskReactivityChange struct {
NewStatus *string
NewAssigneeID *string
// AssignmentGeneration is the value UpdateTaskAssignee's transaction
// committed and read back, carried here rather than re-read. Nil means
// the caller could not supply one (e.g. the read-back itself failed);
// the pipeline then enqueues the task_assigned wake keyless.
AssignmentGeneration *int64
// PrevAssigneeID is the assignee BEFORE the mutation. Required when
// NewAssigneeID is set so the pipeline can detect a real change and
// hand off the previous assignee's session.
@@ -920,7 +920,8 @@ func (s *DashboardService) SetTaskAssigneeAsAgent(ctx context.Context, callerAge
zap.String("task_id", taskID), zap.Error(err))
}
}
if err := s.repo.UpdateTaskAssignee(ctx, taskID, assigneeID); err != nil {
generation, err := s.repo.UpdateTaskAssignee(ctx, taskID, assigneeID)
if err != nil {
return err
}
@@ -928,14 +929,16 @@ func (s *DashboardService) SetTaskAssigneeAsAgent(ctx context.Context, callerAge
// Reactivity pipeline — wakes the new assignee with task_assigned
// and hard-cancels the previous assignee's active session.
s.runReactivityForAssigneeChange(ctx, taskID, prevAssignee, assigneeID, callerAgentID)
s.runReactivityForAssigneeChange(ctx, taskID, prevAssignee, assigneeID, callerAgentID, generation)
return nil
}
// runReactivityForAssigneeChange invokes the reactivity pipeline for an
// assignee change. Best-effort — failures are logged, never propagated.
// generation is the value UpdateTaskAssignee's transaction just committed
// and read back; it is carried onto the mutation rather than re-read.
func (s *DashboardService) runReactivityForAssigneeChange(
ctx context.Context, taskID, prevAssigneeID, newAssigneeID, callerAgentID string,
ctx context.Context, taskID, prevAssigneeID, newAssigneeID, callerAgentID string, generation int64,
) {
if s.reactivity == nil {
return
@@ -945,10 +948,11 @@ func (s *DashboardService) runReactivityForAssigneeChange(
actorType = "agent"
}
change := TaskReactivityChange{
NewAssigneeID: &newAssigneeID,
PrevAssigneeID: prevAssigneeID,
ActorID: callerAgentID,
ActorType: actorType,
NewAssigneeID: &newAssigneeID,
AssignmentGeneration: &generation,
PrevAssigneeID: prevAssigneeID,
ActorID: callerAgentID,
ActorType: actorType,
}
// preStatus="" — assignee changes don't depend on the prev status.
result, err := s.reactivity.ApplyTaskMutation(ctx, taskID, "", change)
@@ -963,7 +967,10 @@ func (s *DashboardService) runReactivityForAssigneeChange(
// Flip the prev assignee's office session row to COMPLETED so it leaves
// the active sessions list. The reactivity pipeline already hard-cancels
// the running execution above; this is the persistent-row counterpart.
if prevAssigneeID != "" && s.sessionTerm != nil {
// A same-agent reassignment is not a handoff — the pipeline above never
// interrupts it — so this must not terminate the agent's own live
// session out from under it.
if prevAssigneeID != "" && prevAssigneeID != newAssigneeID && s.sessionTerm != nil {
if err := s.sessionTerm.TerminateOfficeSession(ctx, taskID, prevAssigneeID, sessionTermReasonReassigned); err != nil {
s.logger.Warn("terminate prev-assignee office session failed",
zap.String("task_id", taskID),
@@ -975,7 +982,7 @@ func (s *DashboardService) runReactivityForAssigneeChange(
// the user isn't asked to triage a failure they already worked
// around by reassigning. Counter is intentionally not reset — the
// root cause may still be unfixed for the old agent.
if prevAssigneeID != "" && s.failureNotifier != nil {
if prevAssigneeID != "" && prevAssigneeID != newAssigneeID && s.failureNotifier != nil {
s.failureNotifier.OnAssigneeChanged(ctx, taskID, prevAssigneeID)
}
}
@@ -13,6 +13,18 @@ type recordingTerminator struct {
calls []termCall
}
type recordingFailureNotifier struct {
calls []struct {
taskID, agentID string
}
}
func (r *recordingFailureNotifier) OnAssigneeChanged(_ context.Context, taskID, agentID string) {
r.calls = append(r.calls, struct {
taskID, agentID string
}{taskID: taskID, agentID: agentID})
}
type termCall struct {
taskID, agentID, reason string
}
@@ -85,12 +97,14 @@ func (r *recordingReactivity) ApplyTaskMutation(_ context.Context, _ string, _ s
func TestSetTaskAssignee_TerminatesPrevSession(t *testing.T) {
deps := newTestDeps(t)
rt := &recordingTerminator{}
fn := &recordingFailureNotifier{}
deps.svc.SetSessionTerminator(rt)
deps.svc.SetFailureNotifier(fn)
deps.svc.SetReactivityApplier(&recordingReactivity{result: &dashboard.TaskReactivityResult{}})
insertTestTask(t, deps.db, "task-r", "ws-r", "Reassign", "todo", 2)
// Seed prev assignee directly via the underlying repo update.
if err := deps.repo.UpdateTaskAssignee(context.Background(), "task-r", "agent-prev"); err != nil {
if _, err := deps.repo.UpdateTaskAssignee(context.Background(), "task-r", "agent-prev"); err != nil {
t.Fatalf("seed prev assignee: %v", err)
}
@@ -105,4 +119,38 @@ func TestSetTaskAssignee_TerminatesPrevSession(t *testing.T) {
if got.taskID != "task-r" || got.agentID != "agent-prev" {
t.Errorf("term call: got %+v", got)
}
if len(fn.calls) != 1 || fn.calls[0].taskID != "task-r" || fn.calls[0].agentID != "agent-prev" {
t.Fatalf("expected prior-assignee failure notification, got %+v", fn.calls)
}
}
// TestSetTaskAssignee_SameAgent_DoesNotTerminateSession is the regression
// test for Review round 3 Finding 5: a repeat assignment to the agent that
// already holds the seat must not flip that agent's own live session row to
// COMPLETED. The reactivity pipeline correctly declines to hard-cancel this
// case (AC-OFFICE-RUN-DEDUP-001.3 treats it as a real occurrence, not an
// interrupt), and the persisted-row side effect must agree.
func TestSetTaskAssignee_SameAgent_DoesNotTerminateSession(t *testing.T) {
deps := newTestDeps(t)
rt := &recordingTerminator{}
fn := &recordingFailureNotifier{}
deps.svc.SetSessionTerminator(rt)
deps.svc.SetFailureNotifier(fn)
deps.svc.SetReactivityApplier(&recordingReactivity{result: &dashboard.TaskReactivityResult{}})
insertTestTask(t, deps.db, "task-same", "ws-r", "Reassign", "todo", 2)
if _, err := deps.repo.UpdateTaskAssignee(context.Background(), "task-same", "agent-x"); err != nil {
t.Fatalf("seed prev assignee: %v", err)
}
if err := deps.svc.SetTaskAssigneeAsAgent(context.Background(), "", "task-same", "agent-x"); err != nil {
t.Fatalf("set assignee: %v", err)
}
if len(rt.calls) != 0 {
t.Fatalf("same-agent reassignment must not terminate the agent's own session, got %d calls (%+v)", len(rt.calls), rt.calls)
}
if len(fn.calls) != 0 {
t.Fatalf("same-agent reassignment must not dismiss its failure inbox entry, got %+v", fn.calls)
}
}
@@ -17,6 +17,7 @@ import (
"github.com/kandev/kandev/internal/office/repository/sqlite"
"github.com/kandev/kandev/internal/office/routing"
"github.com/kandev/kandev/internal/office/shared"
runsservice "github.com/kandev/kandev/internal/runs/service"
taskservice "github.com/kandev/kandev/internal/task/service"
"go.uber.org/zap"
@@ -349,7 +350,11 @@ func (s *OnboardingService) maybeCreateOnboardingTask(
return ""
}
if s.runQueuer != nil {
if wakeErr := s.runQueuer.QueueRun(ctx, agentID, runReasonTaskAssigned,
// A third task_assigned producer alongside queueTaskAssignedRun; it is
// never handed the assigning transaction's generation, so it enqueues
// keyless rather than deriving a divergent key.
runsservice.ReportKeylessEnqueue(runReasonTaskAssigned, runsservice.KeylessCauseUnresolved, "onboarding_no_generation")
if _, wakeErr := s.runQueuer.QueueRun(ctx, agentID, runReasonTaskAssigned,
fmt.Sprintf(`{"task_id":%q}`, taskID), ""); wakeErr != nil {
s.logger.Warn("enqueue onboarding run failed", zap.Error(wakeErr))
}
@@ -625,6 +625,7 @@ func (r *Repository) runTaskPriorityRecreate() error {
{"external_id", `ALTER TABLE tasks ADD COLUMN external_id TEXT COLLATE BINARY`},
{"external_id_settled_at", `ALTER TABLE tasks ADD COLUMN external_id_settled_at TIMESTAMP`},
{"assignee_user_id", `ALTER TABLE tasks ADD COLUMN assignee_user_id TEXT NOT NULL DEFAULT ''`},
{"assignment_generation", `ALTER TABLE tasks ADD COLUMN assignment_generation INTEGER NOT NULL DEFAULT 0`},
}
for _, column := range legacyColumns {
if _, err := conn.ExecContext(ctx, column.stmt); err != nil && !db.IsDuplicateColumnError(err) {
@@ -687,7 +688,8 @@ func taskPriorityMigrationStatements() []string {
checkout_run_id TEXT,
external_id TEXT COLLATE BINARY,
external_id_settled_at TIMESTAMP,
assignee_user_id TEXT NOT NULL DEFAULT ''
assignee_user_id TEXT NOT NULL DEFAULT '',
assignment_generation INTEGER NOT NULL DEFAULT 0
)`,
// archived_by_cascade_id and external_id/external_id_settled_at are
// added to the task schema by task/repository/sqlite/base.go
@@ -707,7 +709,7 @@ func taskPriorityMigrationStatements() []string {
origin, project_id,
labels, identifier,
checkout_agent_id, checkout_at, checkout_run_id,
external_id, external_id_settled_at, assignee_user_id
external_id, external_id_settled_at, assignee_user_id, assignment_generation
) SELECT
id, COALESCE(workspace_id,''), COALESCE(workflow_id,''),
COALESCE(workflow_step_id,''), title, COALESCE(description,''),
@@ -722,7 +724,7 @@ func taskPriorityMigrationStatements() []string {
COALESCE(labels,'[]'), identifier,
checkout_agent_id, checkout_at, checkout_run_id,
external_id, external_id_settled_at,
COALESCE(assignee_user_id,'')
COALESCE(assignee_user_id,''), COALESCE(assignment_generation,0)
FROM tasks`,
`DROP TABLE tasks`,
`ALTER TABLE tasks_priority_new RENAME TO tasks`,
@@ -0,0 +1,79 @@
package sqlite_test
import (
"path/filepath"
"testing"
"github.com/jmoiron/sqlx"
_ "github.com/mattn/go-sqlite3"
"github.com/kandev/kandev/internal/office/repository/sqlite"
)
// TestMigrate_PriorityRebuildPreservesAssignmentGeneration is the regression
// test for the same rebuild hazard TestMigrate_PriorityRebuildPreservesAssignee
// covers one column over: taskPriorityMigrationStatements' INSERT ... SELECT
// carries tasks.assignment_generation through
// COALESCE(assignment_generation,0) (docs/specs/office/system-design/
// run-dedup-generation-02.md#failure-and-recovery). That expression had zero
// coverage before this test — the sibling assignee test seeds a table where
// runTaskPriorityRecreate's own defensive ALTER TABLE ADD COLUMN supplies the
// column fresh (every row defaults to 0), which never exercises copying a
// genuinely non-zero value through the recreate. Here the legacy table
// already carries the column with a non-zero value, as an install upgrading
// mid-feature (assignment_generation shipped, priority rebuild still pending)
// would.
func TestMigrate_PriorityRebuildPreservesAssignmentGeneration(t *testing.T) {
dbPath := filepath.Join(t.TempDir(), "test.db") + "?_journal_mode=WAL"
db, err := sqlx.Open("sqlite3", dbPath)
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
t.Cleanup(func() { _ = db.Close() })
if _, err := db.Exec(`
CREATE TABLE tasks (
id TEXT PRIMARY KEY,
workspace_id TEXT NOT NULL DEFAULT '',
workflow_id TEXT NOT NULL DEFAULT '',
workflow_step_id TEXT NOT NULL DEFAULT '',
title TEXT NOT NULL,
description TEXT DEFAULT '',
state TEXT DEFAULT 'TODO',
priority INTEGER DEFAULT 0,
position INTEGER DEFAULT 0,
metadata TEXT DEFAULT '{}',
is_ephemeral INTEGER NOT NULL DEFAULT 0,
parent_id TEXT DEFAULT '',
archived_at TIMESTAMP,
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
origin TEXT DEFAULT 'manual',
project_id TEXT DEFAULT '',
labels TEXT DEFAULT '[]',
identifier TEXT,
assignee_user_id TEXT NOT NULL DEFAULT '',
assignment_generation INTEGER NOT NULL DEFAULT 0
);
`); err != nil {
t.Fatalf("seed legacy schema: %v", err)
}
if _, err := db.Exec(`
INSERT INTO tasks (id, workspace_id, title, assignment_generation)
VALUES ('task-1', 'ws-1', 'reassigned task', 3)
`); err != nil {
t.Fatalf("seed reassigned task: %v", err)
}
if _, err := sqlite.NewWithDB(db, db, nil); err != nil {
t.Fatalf("init office repo (run migrations): %v", err)
}
var generation int64
if err := db.Get(&generation, `SELECT assignment_generation FROM tasks WHERE id = 'task-1'`); err != nil {
t.Fatalf("tasks.assignment_generation missing after priority rebuild: %v", err)
}
if generation != 3 {
t.Fatalf("assignment_generation lost by priority rebuild: got %d, want 3", generation)
}
}
@@ -165,21 +165,32 @@ func (r *Repository) UpdateTaskStateIfWorkflowStep(
// task_id) so the projection's per-task runner clause still resolves it
// (the (step_id="" / step_id="") match holds because the SELECT joins
// step_id = task.workflow_step_id which is also "").
func (r *Repository) UpdateTaskAssignee(ctx context.Context, taskID, assigneeID string) error {
//
// This is one of the two assignment_generation bump sites (the other is
// insertTaskTx -> upsertRunnerInTx on create). It increments
// tasks.assignment_generation unconditionally on every committed call —
// including a repeat assignment to the agent that already holds the seat,
// which is a real occurrence, not a no-op — and reads the new value back
// inside this same transaction before Commit, returning it so callers carry
// it forward instead of re-reading it later (a later re-read could observe a
// different, more recent occurrence's value). A read-back failure rolls the
// whole assignment back rather than commit a write whose generation could
// not be reported.
func (r *Repository) UpdateTaskAssignee(ctx context.Context, taskID, assigneeID string) (int64, error) {
var stepID string
err := r.ro.QueryRowxContext(ctx, r.ro.Rebind(
`SELECT COALESCE(workflow_step_id, '') FROM tasks WHERE id = ?`),
taskID).Scan(&stepID)
if err != nil {
if err == sql.ErrNoRows {
return fmt.Errorf("task not found: %s", taskID)
return 0, fmt.Errorf("task not found: %s", taskID)
}
return err
return 0, err
}
tx, err := r.db.BeginTxx(ctx, nil)
if err != nil {
return err
return 0, err
}
defer func() { _ = tx.Rollback() }()
@@ -188,7 +199,7 @@ func (r *Repository) UpdateTaskAssignee(ctx context.Context, taskID, assigneeID
DELETE FROM workflow_step_participants
WHERE step_id = ? AND task_id = ? AND role = 'runner'
`), stepID, taskID); err != nil {
return err
return 0, err
}
} else {
var existing string
@@ -201,7 +212,7 @@ func (r *Repository) UpdateTaskAssignee(ctx context.Context, taskID, assigneeID
if _, err := tx.ExecContext(ctx, tx.Rebind(
`UPDATE workflow_step_participants SET agent_profile_id = ? WHERE id = ?`),
assigneeID, existing); err != nil {
return err
return 0, err
}
case sql.ErrNoRows:
if _, err := tx.ExecContext(ctx, tx.Rebind(`
@@ -209,19 +220,30 @@ func (r *Repository) UpdateTaskAssignee(ctx context.Context, taskID, assigneeID
(id, step_id, task_id, role, agent_profile_id, decision_required, position, created_at)
VALUES (?, ?, ?, 'runner', ?, 0, 0, ?)
`), newParticipantUUID(), stepID, taskID, assigneeID, time.Now().UTC()); err != nil {
return err
return 0, err
}
default:
return probeErr
return 0, probeErr
}
}
if _, err := tx.ExecContext(ctx, tx.Rebind(`
UPDATE tasks SET updated_at = CURRENT_TIMESTAMP WHERE id = ?
UPDATE tasks SET assignment_generation = assignment_generation + 1, updated_at = CURRENT_TIMESTAMP WHERE id = ?
`), taskID); err != nil {
return err
return 0, err
}
return tx.Commit()
var generation int64
if err := tx.QueryRowxContext(ctx, tx.Rebind(
`SELECT assignment_generation FROM tasks WHERE id = ?`),
taskID).Scan(&generation); err != nil {
return 0, err
}
if err := tx.Commit(); err != nil {
return 0, err
}
return generation, nil
}
// TaskBasicInfo contains the minimal task fields needed for prompt building.
@@ -330,9 +330,13 @@ func TestUpdateTaskAssignee_WritesUpdatesAndClearsRunnerRow(t *testing.T) {
t.Fatalf("seed task: %v", err)
}
if err := repo.UpdateTaskAssignee(ctx, "as-1", "agent-one"); err != nil {
gen1, err := repo.UpdateTaskAssignee(ctx, "as-1", "agent-one")
if err != nil {
t.Fatalf("UpdateTaskAssignee (insert): %v", err)
}
if gen1 != 1 {
t.Fatalf("generation = %d, want 1 for the first assignment", gen1)
}
if got := taskRunner(t, repo, "as-1"); got != "agent-one" {
t.Fatalf("runner = %q, want agent-one", got)
}
@@ -340,10 +344,15 @@ func TestUpdateTaskAssignee_WritesUpdatesAndClearsRunnerRow(t *testing.T) {
t.Fatalf("runner rows = %d, want 1", n)
}
// A second assignment updates the existing row instead of adding one.
if err := repo.UpdateTaskAssignee(ctx, "as-1", "agent-two"); err != nil {
// A second assignment updates the existing row instead of adding one,
// and bumps the generation again.
gen2, err := repo.UpdateTaskAssignee(ctx, "as-1", "agent-two")
if err != nil {
t.Fatalf("UpdateTaskAssignee (update): %v", err)
}
if gen2 != 2 {
t.Fatalf("generation = %d, want 2 for the second assignment", gen2)
}
if got := taskRunner(t, repo, "as-1"); got != "agent-two" {
t.Errorf("runner = %q, want agent-two", got)
}
@@ -351,10 +360,15 @@ func TestUpdateTaskAssignee_WritesUpdatesAndClearsRunnerRow(t *testing.T) {
t.Errorf("runner rows = %d, want the row updated in place", n)
}
// An empty assignee deletes the runner row.
if err := repo.UpdateTaskAssignee(ctx, "as-1", ""); err != nil {
// An empty assignee deletes the runner row and still bumps the
// generation: an unassignment is itself an assignment write.
gen3, err := repo.UpdateTaskAssignee(ctx, "as-1", "")
if err != nil {
t.Fatalf("UpdateTaskAssignee (clear): %v", err)
}
if gen3 != 3 {
t.Fatalf("generation = %d, want 3 for the clear", gen3)
}
if n := runnerRowCount(t, repo, "as-1"); n != 0 {
t.Errorf("runner rows = %d, want 0 after clearing", n)
}
@@ -362,7 +376,7 @@ func TestUpdateTaskAssignee_WritesUpdatesAndClearsRunnerRow(t *testing.T) {
t.Errorf("runner = %q, want empty after clearing", got)
}
err := repo.UpdateTaskAssignee(ctx, "missing", "agent-one")
_, err = repo.UpdateTaskAssignee(ctx, "missing", "agent-one")
if err == nil {
t.Fatal("UpdateTaskAssignee(missing) = nil error, want not-found")
}
@@ -371,6 +385,34 @@ func TestUpdateTaskAssignee_WritesUpdatesAndClearsRunnerRow(t *testing.T) {
}
}
// TestUpdateTaskAssignee_RepeatAssignmentBumpsGeneration covers AC-001.3: a
// repeat assignment to the agent that already holds the seat is a real
// occurrence (the operator is asking for the work again), not a no-op, so it
// must mint a new generation rather than leave the counter unchanged.
func TestUpdateTaskAssignee_RepeatAssignmentBumpsGeneration(t *testing.T) {
repo := newSearchTestRepo(t)
ctx := context.Background()
if _, err := repo.ExecRaw(ctx, `
INSERT INTO tasks (id, workspace_id, workflow_step_id, title, created_at, updated_at)
VALUES ('as-repeat', 'ws-1', 'step-1', 'Repeat assign', datetime('now'), datetime('now'))
`); err != nil {
t.Fatalf("seed task: %v", err)
}
gen1, err := repo.UpdateTaskAssignee(ctx, "as-repeat", "agent-a")
if err != nil {
t.Fatalf("first assignment: %v", err)
}
gen2, err := repo.UpdateTaskAssignee(ctx, "as-repeat", "agent-a")
if err != nil {
t.Fatalf("repeat assignment: %v", err)
}
if gen2 != gen1+1 {
t.Fatalf("repeat assignment generation = %d, want %d (one more than %d)", gen2, gen1+1, gen1)
}
}
// Tasks created outside a workflow have no workflow_step_id; the runner row
// is keyed on the empty step so the projection still resolves it.
func TestUpdateTaskAssignee_WorksWithoutAWorkflowStep(t *testing.T) {
@@ -384,7 +426,7 @@ func TestUpdateTaskAssignee_WorksWithoutAWorkflowStep(t *testing.T) {
t.Fatalf("seed task: %v", err)
}
if err := repo.UpdateTaskAssignee(ctx, "as-nostep", "agent-channel"); err != nil {
if _, err := repo.UpdateTaskAssignee(ctx, "as-nostep", "agent-channel"); err != nil {
t.Fatalf("UpdateTaskAssignee: %v", err)
}
if got := taskRunner(t, repo, "as-nostep"); got != "agent-channel" {
@@ -405,13 +447,13 @@ func TestUpdateTaskAssignee_LeavesOtherTasksAlone(t *testing.T) {
t.Fatalf("seed tasks: %v", err)
}
if err := repo.UpdateTaskAssignee(ctx, "as-a", "agent-a"); err != nil {
if _, err := repo.UpdateTaskAssignee(ctx, "as-a", "agent-a"); err != nil {
t.Fatalf("assign as-a: %v", err)
}
if err := repo.UpdateTaskAssignee(ctx, "as-b", "agent-b"); err != nil {
if _, err := repo.UpdateTaskAssignee(ctx, "as-b", "agent-b"); err != nil {
t.Fatalf("assign as-b: %v", err)
}
if err := repo.UpdateTaskAssignee(ctx, "as-b", ""); err != nil {
if _, err := repo.UpdateTaskAssignee(ctx, "as-b", ""); err != nil {
t.Fatalf("clear as-b: %v", err)
}
@@ -36,6 +36,7 @@ func newSearchTestRepo(t *testing.T) *sqlite.Repository {
workflow_step_id TEXT NOT NULL DEFAULT '',
title TEXT NOT NULL DEFAULT '',
assignee_user_id TEXT NOT NULL DEFAULT '',
assignment_generation INTEGER NOT NULL DEFAULT 0,
description TEXT DEFAULT '',
state TEXT DEFAULT 'TODO',
priority TEXT NOT NULL DEFAULT 'medium' CHECK (priority IN ('critical','high','medium','low')),
@@ -0,0 +1,90 @@
package routines_test
import (
"context"
"fmt"
"testing"
"time"
"github.com/kandev/kandev/internal/office/models"
)
// AC-OFFICE-RUN-DEDUP-001.9: the cron dispatch path threads the trigger's
// claimed tick into the wakeup-request's idempotency key rather than
// re-reading trigger.next_run_at after TickScheduledTriggers has already
// advanced it to the next slot. Driving this through TickScheduledTriggers
// (not the private key builder) is what actually pins the wiring: a future
// change that re-reads the trigger row after the advance would still pass
// the unit-level key-format tests but fail here.
func TestTickScheduledTriggers_WakeupKeyNamesClaimedTick_NotAdvancedRow(t *testing.T) {
svc := newTestRoutineService(t)
ctx := context.Background()
routine := newLightweightTestRoutine(t, svc)
trigger := &models.RoutineTrigger{
RoutineID: routine.ID,
Kind: "cron",
CronExpression: "* * * * *",
Timezone: "UTC",
Enabled: true,
}
if err := svc.CreateRoutineTrigger(ctx, trigger); err != nil {
t.Fatalf("create trigger: %v", err)
}
triggers, err := svc.ListRoutineTriggers(ctx, routine.ID)
if err != nil || len(triggers) != 1 {
t.Fatalf("list triggers: %v (n=%d)", err, len(triggers))
}
claimedTick := *triggers[0].NextRunAt
enq := &fakeWakeupEnqueuer{}
svc.SetWakeupEnqueuer(enq)
if err := svc.TickScheduledTriggers(ctx, claimedTick.Add(2*time.Minute)); err != nil {
t.Fatalf("tick scheduled triggers: %v", err)
}
if len(enq.created) != 1 {
t.Fatalf("expected 1 wakeup-request created, got %d", len(enq.created))
}
want := fmt.Sprintf("routine:%s:%s:tick:%d", routine.ID, triggers[0].ID, claimedTick.Unix())
got := enq.created[0].IdempotencyKey
if got != want {
t.Fatalf("idempotency key = %q, want %q (the claimed tick, not the row's advanced next_run_at)", got, want)
}
}
// AC-OFFICE-RUN-DEDUP-001.2: two manual "Fire now" calls inside the same
// minute are two distinct occurrences (an operator asking twice) and must
// mint two distinct keys - the exact collision the old unix-minute key
// format reached (both fires share triggerID == "" and can share a minute).
func TestFireManual_TwoFiresInSameMinute_MintDistinctKeys(t *testing.T) {
svc := newTestRoutineService(t)
ctx := context.Background()
routine := newLightweightTestRoutine(t, svc)
routine.ConcurrencyPolicy = "always_create"
if err := svc.UpdateRoutine(ctx, routine); err != nil {
t.Fatalf("update routine: %v", err)
}
enq := &fakeWakeupEnqueuer{}
svc.SetWakeupEnqueuer(enq)
if _, err := svc.FireManual(ctx, routine.ID, map[string]string{"name": "one"}); err != nil {
t.Fatalf("first fire: %v", err)
}
if _, err := svc.FireManual(ctx, routine.ID, map[string]string{"name": "two"}); err != nil {
t.Fatalf("second fire: %v", err)
}
if len(enq.created) != 2 {
t.Fatalf("expected 2 wakeup-requests created, got %d", len(enq.created))
}
first, second := enq.created[0].IdempotencyKey, enq.created[1].IdempotencyKey
if first == "" || second == "" {
t.Fatalf("expected non-empty keys, got %q and %q", first, second)
}
if first == second {
t.Fatalf("two distinct manual fires must mint distinct keys, both got %q", first)
}
}
@@ -0,0 +1,104 @@
package routines
import (
"fmt"
"testing"
"time"
"github.com/kandev/kandev/internal/office/shared"
)
// AC-OFFICE-RUN-DEDUP-001.1 / .4: a cron fire's key names the claimed
// scheduled slot, and redelivering the same slot (the same claimedTick)
// reproduces the same key byte for byte.
func TestBuildRoutineIdempotencyKey_Cron_UsesClaimedTick(t *testing.T) {
tick := time.Date(2026, 5, 10, 12, 0, 0, 0, time.UTC)
key := buildRoutineIdempotencyKey(shared.RoutineSourceCron, "routine-1", "trigger-1", "", &tick, "run-ignored")
want := fmt.Sprintf("routine:%s:%s:tick:%d", "routine-1", "trigger-1", tick.Unix())
if key != want {
t.Fatalf("key = %q, want %q", key, want)
}
// A second claim of the identical slot (redelivery) mints the same key.
repeat := buildRoutineIdempotencyKey(shared.RoutineSourceCron, "routine-1", "trigger-1", "", &tick, "run-different")
if repeat != key {
t.Fatalf("redelivery of the same slot produced %q, want %q (identical to the first)", repeat, key)
}
}
// AC-OFFICE-RUN-DEDUP-001.2: two distinct cron slots (a genuinely later
// tick) mint two distinct keys.
func TestBuildRoutineIdempotencyKey_Cron_DistinctTicksDiffer(t *testing.T) {
tickOne := time.Date(2026, 5, 10, 12, 0, 0, 0, time.UTC)
tickTwo := tickOne.Add(time.Minute)
keyOne := buildRoutineIdempotencyKey(shared.RoutineSourceCron, "routine-1", "trigger-1", "", &tickOne, "run-a")
keyTwo := buildRoutineIdempotencyKey(shared.RoutineSourceCron, "routine-1", "trigger-1", "", &tickTwo, "run-b")
if keyOne == keyTwo {
t.Fatalf("distinct cron slots must mint distinct keys, both got %q", keyOne)
}
}
// AC-OFFICE-RUN-DEDUP-003.3: a cron fire with no claimed tick (the
// exported entry point called directly with a nil tick, bypassing the
// live processCronTrigger guard) has no occurrence identity and goes
// keyless with cause=unresolved.
func TestBuildRoutineIdempotencyKey_Cron_NoClaimedTickGoesKeyless(t *testing.T) {
key := buildRoutineIdempotencyKey(shared.RoutineSourceCron, "routine-1", "trigger-1", "", nil, "run-1")
if key != "" {
t.Fatalf("key = %q, want empty (keyless) for a cron fire with no claimed tick", key)
}
}
// Manual and webhook fires claim no slot, so two fires of one routine are
// two distinct occurrences by design: each RoutineRun.ID mints its own key.
func TestBuildRoutineIdempotencyKey_ManualAndWebhook_UseRoutineRunID(t *testing.T) {
for _, source := range []string{"manual", "webhook"} {
key := buildRoutineIdempotencyKey(source, "routine-1", "", "", nil, "run-1")
want := "routine:routine-1:run:run-1"
if key != want {
t.Errorf("source=%q key = %q, want %q", source, key, want)
}
}
// Two distinct manual fires -> two distinct keys (the collision the
// old unix-minute key format could reach).
first := buildRoutineIdempotencyKey("manual", "routine-1", "", "", nil, "run-a")
second := buildRoutineIdempotencyKey("manual", "routine-1", "", "", nil, "run-b")
if first == second {
t.Fatalf("two distinct manual fires must mint distinct keys, both got %q", first)
}
}
// An unrecognised RoutineRun.Source names no occurrence this table covers
// and goes keyless with cause=unresolved, the same direction as a cron
// fire with no claimed tick.
func TestBuildRoutineIdempotencyKey_UnrecognisedSourceGoesKeyless(t *testing.T) {
key := buildRoutineIdempotencyKey("some_future_source", "routine-1", "", "", nil, "run-1")
if key != "" {
t.Fatalf("key = %q, want empty (keyless) for an unrecognised source", key)
}
}
// An explicit request key (a webhook delivery header, say) always wins,
// even over a claimed cron tick, so a caller-supplied idempotency key never
// collides with the source-derived identity scheme.
func TestBuildRoutineIdempotencyKey_ExplicitKeyTakesPriority(t *testing.T) {
tick := time.Date(2026, 5, 10, 12, 0, 0, 0, time.UTC)
key := buildRoutineIdempotencyKey(shared.RoutineSourceCron, "routine-1", "trigger-1", "delivery-abc", &tick, "run-1")
want := fmt.Sprintf("routine:%s:%s:%s", "routine-1", shared.RoutineSourceCron, "delivery-abc")
if key != want {
t.Fatalf("key = %q, want %q (explicit key must take priority over claimedTick)", key, want)
}
// Two webhook deliveries with distinct explicit keys mint distinct keys.
first := buildRoutineIdempotencyKey("webhook", "routine-1", "", "delivery-1", nil, "run-a")
second := buildRoutineIdempotencyKey("webhook", "routine-1", "", "delivery-2", nil, "run-a")
if first == second {
t.Fatalf("two distinct explicit keys must mint distinct keys, both got %q", first)
}
}
@@ -15,6 +15,7 @@ import (
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/service"
"github.com/kandev/kandev/internal/office/shared"
runsservice "github.com/kandev/kandev/internal/runs/service"
taskservice "github.com/kandev/kandev/internal/task/service"
)
@@ -445,6 +446,11 @@ func (s *RoutineService) processCronTrigger(ctx context.Context, trigger *Routin
zap.String("trigger_id", trigger.ID), zap.Error(err))
return err
}
// The claimed tick: UpdateTriggerNextRun below advances the trigger
// row's next_run_at to the next slot, so trigger.NextRunAt is captured
// here, before that write, and carried through to the key builder
// rather than re-read afterward.
claimedTick := trigger.NextRunAt
if err := s.repo.UpdateTriggerNextRun(ctx, trigger.ID, &advanceTo); err != nil {
s.logger.Warn("update trigger next_run_at failed",
zap.String("trigger_id", trigger.ID), zap.Error(err))
@@ -458,7 +464,7 @@ func (s *RoutineService) processCronTrigger(ctx context.Context, trigger *Routin
// runCount-1 as missed_ticks ("you missed N since the last fire").
missedForPayload = runCount - 1
}
_, err = s.DispatchRoutineRunWithMissed(ctx, routine, trigger, shared.RoutineSourceCron, nil, missedForPayload)
_, err = s.DispatchRoutineRunWithMissed(ctx, routine, trigger, shared.RoutineSourceCron, nil, missedForPayload, claimedTick)
return err
}
@@ -530,7 +536,7 @@ func (s *RoutineService) DispatchRoutineRun(
source string,
provided map[string]string,
) (*RoutineRun, error) {
return s.dispatchRoutineRun(ctx, routine, trigger, source, provided, "", 0)
return s.dispatchRoutineRun(ctx, routine, trigger, source, provided, "", 0, nil)
}
// DispatchRoutineRunWithIdempotencyKey dispatches a fire with an explicit
@@ -544,7 +550,7 @@ func (s *RoutineService) DispatchRoutineRunWithIdempotencyKey(
provided map[string]string,
idempotencyKey string,
) (*RoutineRun, error) {
return s.dispatchRoutineRun(ctx, routine, trigger, source, provided, idempotencyKey, 0)
return s.dispatchRoutineRun(ctx, routine, trigger, source, provided, idempotencyKey, 0, nil)
}
// DispatchRoutineRunWithMissed is the cron-tick entry point that
@@ -554,6 +560,12 @@ func (s *RoutineService) DispatchRoutineRunWithIdempotencyKey(
// learns "you missed N-1 ticks" via wakeup.RoutinePayload.MissedTicks.
// Manual fires (UI / API) call DispatchRoutineRun directly with no
// missed-tick attribution.
//
// claimedTick is the scheduled tick processCronTrigger claimed off the
// trigger row before advancing it — the dedup key's occurrence identity
// for a cron fire. It travels as a parameter rather than being re-read
// from the trigger row, which by dispatch time already names the next
// slot.
func (s *RoutineService) DispatchRoutineRunWithMissed(
ctx context.Context,
routine *Routine,
@@ -561,8 +573,9 @@ func (s *RoutineService) DispatchRoutineRunWithMissed(
source string,
provided map[string]string,
missedTicks int,
claimedTick *time.Time,
) (*RoutineRun, error) {
return s.dispatchRoutineRun(ctx, routine, trigger, source, provided, "", missedTicks)
return s.dispatchRoutineRun(ctx, routine, trigger, source, provided, "", missedTicks, claimedTick)
}
func (s *RoutineService) dispatchRoutineRun(
@@ -573,6 +586,7 @@ func (s *RoutineService) dispatchRoutineRun(
provided map[string]string,
idempotencyKey string,
missedTicks int,
claimedTick *time.Time,
) (*RoutineRun, error) {
now := time.Now().UTC()
defaults := parseDeclaredDefaults(routine.Variables)
@@ -640,7 +654,7 @@ func (s *RoutineService) dispatchRoutineRun(
return run, nil
}
if err := s.materialiseRoutineRun(ctx, routine, run, tmpl, title, description, vars, source, idempotencyKey, missedTicks); err != nil {
if err := s.materialiseRoutineRun(ctx, routine, run, tmpl, title, description, vars, source, idempotencyKey, missedTicks, claimedTick); err != nil {
return run, err
}
// A nil error only means materialiseRoutineRun's terminal write
@@ -683,11 +697,12 @@ func (s *RoutineService) materialiseRoutineRun(
source string,
idempotencyKey string,
missedTicks int,
claimedTick *time.Time,
) error {
if tmpl.Title != "" && s.workflowEnsurer != nil && s.taskCreator != nil {
return s.materialiseHeavyRoutineRun(ctx, routine, run, title, description)
}
return s.materialiseLightweightRoutineRun(ctx, routine, run, vars, source, idempotencyKey, missedTicks)
return s.materialiseLightweightRoutineRun(ctx, routine, run, vars, source, idempotencyKey, missedTicks, claimedTick)
}
// materialiseHeavyRoutineRun creates a real task in the routine system
@@ -746,11 +761,12 @@ func (s *RoutineService) materialiseLightweightRoutineRun(
source string,
idempotencyKey string,
missedTicks int,
claimedTick *time.Time,
) error {
if s.wakeup == nil || routine.AssigneeAgentProfileID == "" {
return s.finalizeLightweightRun(ctx, run, models.RoutineRunStatusDone)
}
idemKey := buildRoutineIdempotencyKey(routine.ID, run.TriggerID, source, run.ID, idempotencyKey, run.StartedAt)
idemKey := buildRoutineIdempotencyKey(source, routine.ID, run.TriggerID, idempotencyKey, claimedTick, run.ID)
payloadStr, _ := marshalRoutinePayload(routine.ID, vars, missedTicks)
req := &WakeupRequest{
ID: uuid.New().String(),
@@ -805,27 +821,37 @@ func (s *RoutineService) finalizeLightweightRun(
}
// buildRoutineIdempotencyKey composes the source-level dedup key for a
// routine fire. Cron fires use the trigger and minute bucket. Manual and
// webhook fires use a unique run identity unless the caller supplies an
// explicit request key, so distinct event deliveries never collide.
// routine fire. An explicit request key (a webhook delivery header, say)
// always wins, so distinct event deliveries never collide. Otherwise the
// occurrence identity depends on the source: a cron fire claims a scheduled
// slot off the trigger row (ClaimTrigger's compare-and-swap means exactly
// one RoutineRun exists per slot, so the slot - not the run - is the
// occurrence, and a catch-up collapsing several missed ticks into one fire
// is still one occurrence). A manual or webhook fire claims no slot, so
// RoutineRun.ID is its only durable distinguishing identity, and two such
// fires are two distinct occurrences by design.
//
// A cron source with no claimed tick, or a source this table does not
// recognise, has no occurrence identity to name and goes keyless.
func buildRoutineIdempotencyKey(
routineID, triggerID, source, runID, explicitKey string, startedAt *time.Time,
source, routineID, triggerID, explicitKey string, claimedTick *time.Time, routineRunID string,
) string {
if explicitKey != "" {
return fmt.Sprintf("routine:%s:%s:%s", routineID, source, explicitKey)
}
if source != shared.RoutineSourceCron {
return fmt.Sprintf("routine:%s:%s:%s", routineID, source, runID)
switch source {
case shared.RoutineSourceCron:
if claimedTick == nil {
runsservice.ReportKeylessEnqueue(shared.RoutineDispatchReason(source), runsservice.KeylessCauseUnresolved, "cron_no_claimed_tick")
return ""
}
return fmt.Sprintf("routine:%s:%s:tick:%d", routineID, triggerID, claimedTick.Unix())
case "manual", "webhook":
return fmt.Sprintf("routine:%s:run:%s", routineID, routineRunID)
default:
runsservice.ReportKeylessEnqueue(shared.RoutineDispatchReason(source), runsservice.KeylessCauseUnresolved, "unrecognised_routine_source")
return ""
}
now := time.Now().UTC()
if startedAt != nil {
now = *startedAt
}
minute := now.Unix() / 60
if triggerID == "" {
return fmt.Sprintf("routine:%s:%s:%d", routineID, source, minute)
}
return fmt.Sprintf("routine:%s:%s:%d", routineID, triggerID, minute)
}
// marshalRoutinePayload renders the wakeup-request payload for a
@@ -10,6 +10,7 @@ import (
"github.com/google/uuid"
"github.com/kandev/kandev/internal/office/models"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// CommentWriter is the comment mutation dependency used by runtime actions.
@@ -254,7 +255,7 @@ type ApprovalRequester interface {
// RunSpawner is the run queue dependency used by runtime actions.
type RunSpawner interface {
QueueRun(ctx context.Context, agentInstanceID, reason, payload, idempotencyKey string) error
QueueRun(ctx context.Context, agentInstanceID, reason, payload, idempotencyKey string) (runsservice.QueueOutcome, error)
}
// AgentModifier is the agent update dependency used by runtime actions.
@@ -565,7 +566,22 @@ type SpawnAgentRunInput struct {
IdempotencyKey string `json:"idempotency_key"`
}
// maxSpawnAgentRunReasonLength bounds SpawnAgentRunInput.Reason. Reason is
// agent-supplied and reaches office_run_dedup_total /
// office_run_dedup_keyless_total as an expvar.Map label; those maps never
// evict, so an unbounded Reason would let a caller grow them without limit.
const maxSpawnAgentRunReasonLength = 100
// SpawnAgentRun queues a run for an agent in the same workspace.
//
// A non-empty agent-supplied key is prefixed with the calling run's id
// (agent:<callerRunID>:<key>) so a retry of the same run reuses the run id
// and still dedupes, while a later run gets a different prefix and is not
// suppressed. With no caller run id the request enqueues keyless
// (cause=unresolved) rather than risk colliding across runs. An empty key is
// NOT prefixed: the agent expressed no dedup intent (cause=by_design), and
// prefixing it would collapse every no-dedup-intent call inside one run onto
// a single key, suppressing every call after the first.
func (a *Actions) SpawnAgentRun(
ctx context.Context,
runCtx RunContext,
@@ -577,6 +593,9 @@ func (a *Actions) SpawnAgentRun(
if a.deps.Runs == nil || a.deps.AgentModifier == nil {
return fmt.Errorf("%w: runs", ErrRuntimeDependencyMissing)
}
if len(input.Reason) > maxSpawnAgentRunReasonLength {
return ErrReasonTooLong
}
target, err := a.deps.AgentModifier.GetAgentInstance(ctx, input.AgentID)
if err != nil {
return err
@@ -592,7 +611,17 @@ func (a *Actions) SpawnAgentRun(
if err != nil {
return err
}
return a.deps.Runs.QueueRun(ctx, target.ID, input.Reason, string(payload), input.IdempotencyKey)
key := ""
switch {
case input.IdempotencyKey == "":
runsservice.ReportKeylessEnqueue(input.Reason, runsservice.KeylessCauseByDesign, "")
case runCtx.RunID != "":
key = fmt.Sprintf("agent:%s:%s", runCtx.RunID, input.IdempotencyKey)
default:
runsservice.ReportKeylessEnqueue(input.Reason, runsservice.KeylessCauseUnresolved, "no_caller_run")
}
_, err = a.deps.Runs.QueueRun(ctx, target.ID, input.Reason, string(payload), key)
return err
}
// ModifyAgentInput contains agent fields an authorized runtime may update.
@@ -10,6 +10,7 @@ import (
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/shared"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
func TestCapabilitiesMarshalProjectCapabilityKeys(t *testing.T) {
@@ -1221,14 +1222,14 @@ type spawnRunCall struct {
func (r *recordingRunSpawner) QueueRun(
_ context.Context,
agentInstanceID, reason, payload, idempotencyKey string,
) error {
) (runsservice.QueueOutcome, error) {
r.calls = append(r.calls, spawnRunCall{
AgentID: agentInstanceID,
Reason: reason,
Payload: payload,
IdempotencyKey: idempotencyKey,
})
return nil
return runsservice.QueueOutcomeQueued, nil
}
type recordingAgentModifier struct {
@@ -20,4 +20,10 @@ var (
// either) in a workspace that has at least one project to choose from.
// Caller-correctable: the caller should retry with an explicit project_id.
ErrProjectRequired = fmt.Errorf("project_id is required")
// ErrReasonTooLong is returned when SpawnAgentRunInput.Reason exceeds
// maxSpawnAgentRunReasonLength. Reason is agent-supplied and becomes a
// label on the process-global office_run_dedup_total /
// office_run_dedup_keyless_total expvar maps, which never evict entries;
// an unbounded value lets a caller grow those maps without limit.
ErrReasonTooLong = fmt.Errorf("reason exceeds max length of %d characters", maxSpawnAgentRunReasonLength)
)
@@ -478,7 +478,7 @@ func (h *Handler) respondRuntimeError(
targetID string,
err error,
) {
if errors.Is(err, errTaskTitleRequired) || errors.Is(err, ErrProjectRequired) {
if errors.Is(err, errTaskTitleRequired) || errors.Is(err, ErrProjectRequired) || errors.Is(err, ErrReasonTooLong) {
h.appendDeniedRunEvent(c.Request.Context(), runCtx, action, targetType, targetID, err)
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
@@ -0,0 +1,217 @@
package runtime
import (
"context"
"errors"
"expvar"
"strings"
"testing"
"github.com/kandev/kandev/internal/office/models"
)
// spawnAgentRunKeylessCounterHasLabel reports whether the process-global
// office_run_dedup_keyless_total expvar map carries an entry for the given
// reason and cause. Mirrors internal/runs/service's own counterHasLabel test
// helper — that one lives in a different package (service_test) and cannot
// be imported here.
func spawnAgentRunKeylessCounterHasLabel(t *testing.T, reason, cause string) bool {
t.Helper()
v := expvar.Get("office_run_dedup_keyless_total")
if v == nil {
t.Fatalf("expvar map office_run_dedup_keyless_total not registered")
}
m, ok := v.(*expvar.Map)
if !ok {
t.Fatalf("office_run_dedup_keyless_total is not a *expvar.Map")
}
found := false
m.Do(func(kv expvar.KeyValue) {
if strings.Contains(kv.Key, "reason="+reason) && strings.Contains(kv.Key, "cause="+cause) {
found = true
}
})
return found
}
// TestActionsSpawnAgentRun_PrefixesKeyWithCallerRunID is the AC-001.7
// regression: a non-empty agent-supplied key is prefixed with the calling
// run's id (agent:<callerRunID>:<key>) before reaching QueueRun, so a retry
// of the same run reuses the run id and still dedupes, while a later run
// (different RunID) produces a different key and is not suppressed. Before
// this test, the only coverage touching SpawnAgentRun's key logic was the
// unrelated cross-workspace denial test — the prefix itself was unpinned.
func TestActionsSpawnAgentRun_PrefixesKeyWithCallerRunID(t *testing.T) {
agents := &recordingAgentModifier{
agents: map[string]*models.AgentInstance{
"agent-2": {ID: "agent-2", WorkspaceID: "ws-1"},
},
}
runs := &recordingRunSpawner{}
actions := NewActions(ActionDependencies{Runs: runs, AgentModifier: agents})
runCtx := RunContext{
WorkspaceID: "ws-1",
RunID: "run-caller-1",
Capabilities: Capabilities{
CanSpawnAgentRun: true,
},
}
if err := actions.SpawnAgentRun(context.Background(), runCtx, SpawnAgentRunInput{
AgentID: "agent-2",
Reason: "custom_reason",
IdempotencyKey: "custom-key",
}); err != nil {
t.Fatalf("SpawnAgentRun: %v", err)
}
if len(runs.calls) != 1 {
t.Fatalf("run spawner calls = %d, want 1", len(runs.calls))
}
want := "agent:run-caller-1:custom-key"
if got := runs.calls[0].IdempotencyKey; got != want {
t.Fatalf("idempotency key = %q, want %q", got, want)
}
}
// TestActionsSpawnAgentRun_DifferentCallerRun_ProducesDifferentKey proves
// the prefix's other half: the SAME agent-supplied key from a DIFFERENT
// calling run does not collide, because the caller run id varies the key.
func TestActionsSpawnAgentRun_DifferentCallerRun_ProducesDifferentKey(t *testing.T) {
agents := &recordingAgentModifier{
agents: map[string]*models.AgentInstance{
"agent-2": {ID: "agent-2", WorkspaceID: "ws-1"},
},
}
runs := &recordingRunSpawner{}
actions := NewActions(ActionDependencies{Runs: runs, AgentModifier: agents})
caps := Capabilities{CanSpawnAgentRun: true}
for _, callerRunID := range []string{"run-a", "run-b"} {
runCtx := RunContext{WorkspaceID: "ws-1", RunID: callerRunID, Capabilities: caps}
if err := actions.SpawnAgentRun(context.Background(), runCtx, SpawnAgentRunInput{
AgentID: "agent-2",
Reason: "custom_reason",
IdempotencyKey: "same-key",
}); err != nil {
t.Fatalf("SpawnAgentRun (caller %s): %v", callerRunID, err)
}
}
if len(runs.calls) != 2 {
t.Fatalf("run spawner calls = %d, want 2", len(runs.calls))
}
if runs.calls[0].IdempotencyKey == runs.calls[1].IdempotencyKey {
t.Fatalf("two different caller runs produced the same key %q", runs.calls[0].IdempotencyKey)
}
}
// TestActionsSpawnAgentRun_EmptyKey_NotPrefixed_ByDesign proves the third
// branch: the agent expressed no dedup intent (empty IdempotencyKey), so the
// call enqueues keyless without a prefix — prefixing an empty key would
// collapse every no-dedup-intent call inside one run onto a single key,
// suppressing every call after the first.
func TestActionsSpawnAgentRun_EmptyKey_NotPrefixed_ByDesign(t *testing.T) {
agents := &recordingAgentModifier{
agents: map[string]*models.AgentInstance{
"agent-2": {ID: "agent-2", WorkspaceID: "ws-1"},
},
}
runs := &recordingRunSpawner{}
actions := NewActions(ActionDependencies{Runs: runs, AgentModifier: agents})
runCtx := RunContext{
WorkspaceID: "ws-1",
RunID: "run-caller-2",
Capabilities: Capabilities{CanSpawnAgentRun: true},
}
if err := actions.SpawnAgentRun(context.Background(), runCtx, SpawnAgentRunInput{
AgentID: "agent-2",
Reason: "test_spawn_empty_key",
}); err != nil {
t.Fatalf("SpawnAgentRun: %v", err)
}
if len(runs.calls) != 1 {
t.Fatalf("run spawner calls = %d, want 1", len(runs.calls))
}
if got := runs.calls[0].IdempotencyKey; got != "" {
t.Fatalf("idempotency key = %q, want empty (keyless by design)", got)
}
// Reason isn't in runs/service's bounded metricReasons allowlist, so it
// buckets to "custom" on the label rather than surviving verbatim.
if !spawnAgentRunKeylessCounterHasLabel(t, "custom", "by_design") {
t.Fatal("expected office_run_dedup_keyless_total to carry a custom/by_design entry")
}
}
// TestActionsSpawnAgentRun_NoCallerRunID_EnqueuesKeylessUnresolved proves
// the fourth branch: a non-empty agent-supplied key with no caller run id to
// prefix it enqueues keyless (cause=unresolved) rather than risk colliding
// across unrelated runs.
func TestActionsSpawnAgentRun_NoCallerRunID_EnqueuesKeylessUnresolved(t *testing.T) {
agents := &recordingAgentModifier{
agents: map[string]*models.AgentInstance{
"agent-2": {ID: "agent-2", WorkspaceID: "ws-1"},
},
}
runs := &recordingRunSpawner{}
actions := NewActions(ActionDependencies{Runs: runs, AgentModifier: agents})
runCtx := RunContext{
WorkspaceID: "ws-1",
RunID: "",
Capabilities: Capabilities{CanSpawnAgentRun: true},
}
if err := actions.SpawnAgentRun(context.Background(), runCtx, SpawnAgentRunInput{
AgentID: "agent-2",
Reason: "test_spawn_no_caller_run",
IdempotencyKey: "custom-key",
}); err != nil {
t.Fatalf("SpawnAgentRun: %v", err)
}
if len(runs.calls) != 1 {
t.Fatalf("run spawner calls = %d, want 1", len(runs.calls))
}
if got := runs.calls[0].IdempotencyKey; got != "" {
t.Fatalf("idempotency key = %q, want empty (keyless, no caller run to prefix with)", got)
}
// Reason isn't in runs/service's bounded metricReasons allowlist, so it
// buckets to "custom" on the label rather than surviving verbatim.
if !spawnAgentRunKeylessCounterHasLabel(t, "custom", "unresolved") {
t.Fatal("expected office_run_dedup_keyless_total to carry a custom/unresolved entry")
}
}
// TestActionsSpawnAgentRun_ReasonTooLong_Rejected proves Reason is bounded
// before it can reach office_run_dedup_total /
// office_run_dedup_keyless_total as an expvar.Map label: those maps never
// evict entries, so an unvalidated agent-supplied Reason would let a caller
// grow them without limit. An over-length Reason is rejected with
// ErrReasonTooLong and never reaches the run spawner.
func TestActionsSpawnAgentRun_ReasonTooLong_Rejected(t *testing.T) {
agents := &recordingAgentModifier{
agents: map[string]*models.AgentInstance{
"agent-2": {ID: "agent-2", WorkspaceID: "ws-1"},
},
}
runs := &recordingRunSpawner{}
actions := NewActions(ActionDependencies{Runs: runs, AgentModifier: agents})
runCtx := RunContext{
WorkspaceID: "ws-1",
RunID: "run-caller-3",
Capabilities: Capabilities{CanSpawnAgentRun: true},
}
err := actions.SpawnAgentRun(context.Background(), runCtx, SpawnAgentRunInput{
AgentID: "agent-2",
Reason: strings.Repeat("a", maxSpawnAgentRunReasonLength+1),
})
if !errors.Is(err, ErrReasonTooLong) {
t.Fatalf("SpawnAgentRun error = %v, want ErrReasonTooLong", err)
}
if len(runs.calls) != 0 {
t.Fatalf("run spawner calls = %d, want 0 (rejected before enqueue)", len(runs.calls))
}
}
@@ -45,7 +45,7 @@ func (a *DashboardApprovalAdapter) QueueApprovalRuns(
DecisionComment: w.DecisionComment,
IdempotencyKey: w.IdempotencyKey,
}
if err := a.scheduler.QueueRunCtx(ctx, w.AgentID, c); err != nil {
if _, err := a.scheduler.QueueRunCtx(ctx, w.AgentID, c); err != nil {
a.scheduler.logger.Warn("approval run failed: " + err.Error())
}
}
@@ -75,12 +75,13 @@ func (a *DashboardReactivityAdapter) ApplyTaskMutation(
// two packages.
func convertChangeToMutation(c dashboard.TaskReactivityChange) TaskMutation {
out := TaskMutation{
NewStatus: c.NewStatus,
NewAssigneeID: c.NewAssigneeID,
ReopenIntent: c.ReopenIntent,
ResumeIntent: c.ResumeIntent,
ActorID: c.ActorID,
ActorType: c.ActorType,
NewStatus: c.NewStatus,
NewAssigneeID: c.NewAssigneeID,
AssignmentGeneration: c.AssignmentGeneration,
ReopenIntent: c.ReopenIntent,
ResumeIntent: c.ResumeIntent,
ActorID: c.ActorID,
ActorType: c.ActorType,
}
if c.Comment != nil {
out.Comment = &MutationComment{
@@ -0,0 +1,72 @@
package scheduler
import (
"context"
"expvar"
"testing"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// windowedDedupCounterValue reads the current value of one
// office_run_dedup_total{reason,kind=windowed,queue=runs} label, or 0 if the
// label has never been reported. Callers snapshot before/after and assert
// the exact delta.
func windowedDedupCounterValue(t *testing.T, reason string) int64 {
t.Helper()
v := expvar.Get("office_run_dedup_total")
if v == nil {
t.Fatalf("expvar map office_run_dedup_total not registered")
}
m, ok := v.(*expvar.Map)
if !ok {
t.Fatalf("office_run_dedup_total is not a *expvar.Map")
}
iv := m.Get("reason=" + reason + ";kind=windowed;queue=runs")
if iv == nil {
return 0
}
i, ok := iv.(*expvar.Int)
if !ok {
t.Fatalf("counter value for reason=%s;kind=windowed;queue=runs is not *expvar.Int", reason)
}
return i.Value()
}
// TestSchedulerService_QueueRun_ReportsWindowedDedupOutcome closes a
// phantom-green gap on the office-owned queue implementation the design
// names as least observable (run-dedup-generation-02.md#test-strategy):
// no test asserted SchedulerService.QueueRun actually calls
// ReportWindowedDedup, so deleting that call would leave every existing
// test green while AC-OFFICE-RUN-DEDUP-004.1 silently went unmet on the
// path a reassignment travels.
func TestSchedulerService_QueueRun_ReportsWindowedDedupOutcome(t *testing.T) {
repo := newReactivityTestRepo(t)
ss := newChildrenCompletedTestScheduler(t, repo)
createChildrenCompletedAgent(t, repo, "agent-queue-run-outcome")
ctx := context.Background()
key := "idem-key-scheduler-outcome"
first, err := ss.QueueRun(ctx, "agent-queue-run-outcome", RunReasonTaskAssigned, "{}", key)
if err != nil {
t.Fatalf("first enqueue: %v", err)
}
if first != runsservice.QueueOutcomeQueued {
t.Fatalf("first outcome = %v, want QueueOutcomeQueued", first)
}
before := windowedDedupCounterValue(t, RunReasonTaskAssigned)
second, err := ss.QueueRun(ctx, "agent-queue-run-outcome", RunReasonTaskAssigned, "{}", key)
if err != nil {
t.Fatalf("second enqueue: %v", err)
}
if second != runsservice.QueueOutcomeDeduped {
t.Fatalf("second outcome = %v, want QueueOutcomeDeduped", second)
}
after := windowedDedupCounterValue(t, RunReasonTaskAssigned)
if after != before+1 {
t.Fatalf("office_run_dedup_total{%s,windowed,runs} = %d, want %d", RunReasonTaskAssigned, after, before+1)
}
}
@@ -10,6 +10,9 @@ import (
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/repository/sqlite"
"github.com/kandev/kandev/internal/runs/commentkeys"
"github.com/kandev/kandev/internal/runs/dedupkeys"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// Canonical lowercase status values used inside the pipeline. Backend
@@ -46,10 +49,17 @@ type TaskMutation struct {
// What's changing.
NewStatus *string // nil = unchanged
NewAssigneeID *string
NewPriority *string
Comment *MutationComment // user/agent comment if this mutation includes one
ReopenIntent bool // explicit reopen=true (or status moves done|cancelled → todo|in_progress)
ResumeIntent bool // explicit resume=true (validated upstream to require comment)
// AssignmentGeneration is the value the assigning transaction committed,
// carried here rather than re-read (a producer reading the task's
// current generation after the fact can observe a later occurrence's
// value — see docs/specs/office/system-design/run-dedup-generation-01.md
// #carrying-the-generation). Nil means the caller could not supply one;
// reactToAssigneeChange then enqueues keyless rather than guess.
AssignmentGeneration *int64
NewPriority *string
Comment *MutationComment // user/agent comment if this mutation includes one
ReopenIntent bool // explicit reopen=true (or status moves done|cancelled → todo|in_progress)
ResumeIntent bool // explicit resume=true (validated upstream to require comment)
// Who is acting.
ActorID string
@@ -115,13 +125,17 @@ func (ss *SchedulerService) ApplyTaskMutation(
return
}
seen[key] = struct{}{}
if err := ss.QueueRunCtx(ctx, agentID, c); err != nil {
outcome, err := ss.QueueRunCtx(ctx, agentID, c)
if err != nil {
ss.logger.Error("reactivity run failed",
zap.String("agent", agentID),
zap.String("reason", c.Reason),
zap.Error(err))
return
}
if outcome != runsservice.QueueOutcomeQueued {
return
}
res.Runs = append(res.Runs, QueuedRunSummary{
AgentID: agentID, Reason: c.Reason, TaskID: c.TaskID,
})
@@ -133,7 +147,12 @@ func (ss *SchedulerService) ApplyTaskMutation(
}
// --- Assignee handoff ---
if change.NewAssigneeID != nil && *change.NewAssigneeID != task.AssigneeAgentProfileID {
// Fires on every non-nil NewAssigneeID, including a repeat assignment to
// the agent that already holds the seat: that is a real occurrence (the
// operator is asking for the work again), not a no-op, so this no longer
// gates on the two ids differing. reactToAssigneeChange itself guards the
// session interrupt.
if change.NewAssigneeID != nil {
ss.reactToAssigneeChange(task, *change.NewAssigneeID, change, queue, res)
}
@@ -180,7 +199,13 @@ func (ss *SchedulerService) reactToStatusChange(
ss.cascadeReviewRequested(ctx, task, change, queue)
case prev == statusBlocked && next != statusBlocked:
// Unblocked — wake assignee with task_unblocked.
// Unblocked — no durable occurrence row to key on: keyless by design.
// Reported only when there's an assignee to enqueue for — queue()
// silently drops an empty agent id, so reporting unconditionally
// would count an enqueue that never happened.
if task.AssigneeAgentProfileID != "" {
runsservice.ReportKeylessEnqueue(RunReasonTaskUnblocked, runsservice.KeylessCauseByDesign, "")
}
queue(task.AssigneeAgentProfileID, RunContext{
Reason: RunReasonTaskUnblocked,
TaskID: task.ID,
@@ -190,7 +215,7 @@ func (ss *SchedulerService) reactToStatusChange(
})
case (prev == statusDone || prev == statusCancelled) && (next == statusTodo || next == statusInProgress):
// Reopen — different reason if a comment was attached.
// Reopen — different reason (and dedup identity) if a comment was attached.
reason := RunReasonTaskReopened
commentID := ""
if change.Comment != nil {
@@ -201,13 +226,23 @@ func (ss *SchedulerService) reactToStatusChange(
// Explicit resume always uses the comment-flavoured reason.
reason = RunReasonTaskReopenedComment
}
var key string
if reason == RunReasonTaskReopenedComment {
key = fmt.Sprintf("%s:%s:%s", RunReasonTaskReopenedComment, commentID, task.AssigneeAgentProfileID)
} else if task.AssigneeAgentProfileID != "" {
// Silent reopen (no comment) — no durable occurrence row: keyless
// by design. Reported only when there's an assignee to enqueue
// for, matching the task_unblocked case above.
runsservice.ReportKeylessEnqueue(RunReasonTaskReopened, runsservice.KeylessCauseByDesign, "")
}
queue(task.AssigneeAgentProfileID, RunContext{
Reason: reason,
TaskID: task.ID,
WorkspaceID: task.WorkspaceID,
ActorID: change.ActorID,
ActorType: change.ActorType,
CommentID: commentID,
Reason: reason,
TaskID: task.ID,
WorkspaceID: task.WorkspaceID,
ActorID: change.ActorID,
ActorType: change.ActorType,
CommentID: commentID,
IdempotencyKey: key,
})
}
}
@@ -222,6 +257,13 @@ func (ss *SchedulerService) reactToStatusChange(
//
// The previous assignee is NOT separately notified — interrupting their
// run is the signal that they're no longer in charge.
//
// ApplyTaskMutation now calls this for every non-nil NewAssigneeID,
// including a repeat assignment to the agent that already holds the seat.
// The interrupt must NOT fire for that case — it would hard-cancel the
// agent's own in-flight run — so unlike the removed caller-side equality
// gate, this comparison stays local to the interrupt decision and does not
// also guard the wake.
func (ss *SchedulerService) reactToAssigneeChange(
task *TaskSnapshot,
newAssigneeID string,
@@ -229,25 +271,36 @@ func (ss *SchedulerService) reactToAssigneeChange(
queue func(string, RunContext),
res *ApplyTaskMutationResult,
) {
// Cancel the prior assignee's session if there was one. We re-use
// InterruptSessionID — status→cancelled also sets it; either reason
// for hard-cancelling produces the same downstream call.
if task.AssigneeAgentProfileID != "" && res.InterruptSessionID == "" {
if task.AssigneeAgentProfileID != "" && newAssigneeID != task.AssigneeAgentProfileID {
res.InterruptSessionID = task.ID
}
// Wake the new assignee.
if newAssigneeID == "" {
// Unassignment: the interrupt above (if any) already fired; queue's
// own empty-agent-id guard would catch this too, but there is no key
// to build or keyless cause to report for a run that will never be
// attempted.
return
}
commentID := ""
if change.Comment != nil {
commentID = change.Comment.ID
}
var key string
if change.AssignmentGeneration != nil {
key = dedupkeys.AssignmentKey(task.ID, newAssigneeID, *change.AssignmentGeneration)
} else {
runsservice.ReportKeylessEnqueue(RunReasonTaskAssigned, runsservice.KeylessCauseUnresolved, "nil_mutation_generation")
}
queue(newAssigneeID, RunContext{
Reason: RunReasonTaskAssigned,
TaskID: task.ID,
WorkspaceID: task.WorkspaceID,
ActorID: change.ActorID,
ActorType: change.ActorType,
CommentID: commentID,
Reason: RunReasonTaskAssigned,
TaskID: task.ID,
WorkspaceID: task.WorkspaceID,
ActorID: change.ActorID,
ActorType: change.ActorType,
CommentID: commentID,
IdempotencyKey: key,
})
}
@@ -262,13 +315,15 @@ func (ss *SchedulerService) reactToComment(
// Assignee wake — skip if self-comment or task is closed.
if !comment.SkipAssigneeWake && !selfComment && !closed {
key := commentkeys.TaskComment(comment.ID) + ":" + task.AssigneeAgentProfileID
queue(task.AssigneeAgentProfileID, RunContext{
Reason: RunReasonTaskComment,
TaskID: task.ID,
WorkspaceID: task.WorkspaceID,
ActorID: comment.AuthorID,
ActorType: comment.AuthorType,
CommentID: comment.ID,
Reason: RunReasonTaskComment,
TaskID: task.ID,
WorkspaceID: task.WorkspaceID,
ActorID: comment.AuthorID,
ActorType: comment.AuthorType,
CommentID: comment.ID,
IdempotencyKey: key,
})
}
@@ -286,13 +341,15 @@ func (ss *SchedulerService) reactToComment(
if agentID == comment.AuthorID {
continue
}
key := fmt.Sprintf("%s:%s:%s", RunReasonTaskMentioned, comment.ID, agentID)
queue(agentID, RunContext{
Reason: RunReasonTaskMentioned,
TaskID: task.ID,
WorkspaceID: task.WorkspaceID,
ActorID: comment.AuthorID,
ActorType: comment.AuthorType,
CommentID: comment.ID,
Reason: RunReasonTaskMentioned,
TaskID: task.ID,
WorkspaceID: task.WorkspaceID,
ActorID: comment.AuthorID,
ActorType: comment.AuthorType,
CommentID: comment.ID,
IdempotencyKey: key,
})
}
}
@@ -320,6 +377,7 @@ func (ss *SchedulerService) cascadeReviewRequested(
zap.String("task_id", task.ID), zap.Error(err))
return
}
reported := map[string]struct{}{}
for _, p := range parts {
if p.AgentProfileID == "" {
continue
@@ -327,6 +385,14 @@ func (ss *SchedulerService) cascadeReviewRequested(
if p.Role != models.ParticipantRoleReviewer && p.Role != models.ParticipantRoleApprover {
continue
}
// No durable occurrence row to key on: keyless by design, reported
// once per distinct agent rather than per role — an agent seated as
// both reviewer and approver is one recipient, and queue()'s own
// seen-map dedup only attempts one enqueue for it.
if _, dup := reported[p.AgentProfileID]; !dup {
reported[p.AgentProfileID] = struct{}{}
runsservice.ReportKeylessEnqueue(RunReasonTaskReviewRequested, runsservice.KeylessCauseByDesign, "")
}
queue(p.AgentProfileID, RunContext{
Reason: RunReasonTaskReviewRequested,
TaskID: task.ID,
@@ -352,19 +418,34 @@ func (ss *SchedulerService) cascadeBlockersResolved(
}
for _, blockedID := range blockedTaskIDs {
// Verify all OTHER blockers are also resolved.
ready, err := ss.allBlockersResolvedExcept(ctx, blockedID, task.ID)
if err != nil || !ready {
ready, blockers, err := ss.allBlockersResolvedExcept(ctx, blockedID, task.ID)
if err != nil {
ss.logger.Error("check blockers resolved failed",
zap.String("task_id", blockedID), zap.Error(err))
continue
}
if !ready {
continue
}
assignee, err := ss.repo.GetTaskAssignee(ctx, blockedID)
if err != nil || assignee == "" {
continue
}
blockerIDs := make([]string, 0, len(blockers))
for _, b := range blockers {
blockerIDs = append(blockerIDs, b.BlockerTaskID)
}
if len(blockerIDs) == 0 {
continue
}
key := fmt.Sprintf("%s:%s:%s:%s",
RunReasonTaskBlockersResolved, blockedID, assignee, dedupkeys.BlockerDigest(blockerIDs))
queue(assignee, RunContext{
Reason: RunReasonTaskBlockersResolved,
TaskID: blockedID,
WorkspaceID: task.WorkspaceID,
ResolvedBlockerTaskID: task.ID,
IdempotencyKey: key,
})
}
}
@@ -443,13 +524,17 @@ func childrenCompletedIdempotencyKey(parentID, agentID string, children []sqlite
}
// allBlockersResolvedExcept returns true if every blocker on `taskID`
// other than `excludeBlockerID` is in a terminal step.
// other than `excludeBlockerID` is in a terminal step, alongside the full
// blocker-task-id set it read to decide — the caller digests that same
// slice for the wake's dedup key rather than re-reading it (AC-001.9
// applied to a set: a second read could observe a different set than the
// one this readiness decision was actually made against).
func (ss *SchedulerService) allBlockersResolvedExcept(
ctx context.Context, taskID, excludeBlockerID string,
) (bool, error) {
) (bool, []*models.TaskBlocker, error) {
blockers, err := ss.repo.ListTaskBlockers(ctx, taskID)
if err != nil {
return false, err
return false, nil, err
}
for _, b := range blockers {
if b.BlockerTaskID == excludeBlockerID {
@@ -457,13 +542,13 @@ func (ss *SchedulerService) allBlockersResolvedExcept(
}
done, err := ss.repo.IsTaskInTerminalStep(ctx, b.BlockerTaskID)
if err != nil {
return false, err
return false, nil, err
}
if !done {
return false, nil
return false, blockers, nil
}
}
return true, nil
return true, blockers, nil
}
// normalisedStatus maps both backend uppercase task states (TODO,
@@ -0,0 +1,138 @@
package scheduler
import (
"context"
"testing"
)
// TestApplyTaskMutation_SameAgentRepeatAssignment_QueuesWithoutInterrupt
// pins the reactivity gate relaxation this feature made: ApplyTaskMutation
// now calls reactToAssigneeChange for EVERY non-nil NewAssigneeID, including
// a repeat assignment to the agent that already holds the seat, because that
// is a real occurrence (the operator asking for the work again) rather than
// a no-op. Before this feature the caller-side equality gate suppressed it
// entirely. The relocated interrupt-comparison guard must still hold: a
// same-agent repeat must NOT hard-cancel the agent's own in-flight session.
// Before this test, ApplyTaskMutation/reactToStatusChange/
// reactToAssigneeChange were at 0.0% coverage.
func TestApplyTaskMutation_SameAgentRepeatAssignment_QueuesWithoutInterrupt(t *testing.T) {
repo := newReactivityTestRepo(t)
ss := newChildrenCompletedTestScheduler(t, repo)
createChildrenCompletedAgent(t, repo, "agent-repeat")
task := &TaskSnapshot{
ID: "task-repeat-assign",
WorkspaceID: "ws-1",
AssigneeAgentProfileID: "agent-repeat", // already holds the seat
}
newAssignee := "agent-repeat"
gen := int64(1)
change := TaskMutation{
NewAssigneeID: &newAssignee,
AssignmentGeneration: &gen,
ActorID: "user-1",
ActorType: "user",
}
res, err := ss.ApplyTaskMutation(context.Background(), task, change)
if err != nil {
t.Fatalf("ApplyTaskMutation: %v", err)
}
if res.InterruptSessionID != "" {
t.Fatalf("same-agent repeat assignment set InterruptSessionID = %q, want empty (must not cancel its own run)", res.InterruptSessionID)
}
found := false
for _, r := range res.Runs {
if r.AgentID == "agent-repeat" && r.Reason == RunReasonTaskAssigned {
found = true
}
}
if !found {
t.Fatalf("expected a task_assigned run queued for agent-repeat, got %+v", res.Runs)
}
}
// TestApplyTaskMutation_DifferentAgentReassignment_InterruptsPreviousAssignee
// is the positive counterpart: reassigning to a DIFFERENT agent must still
// hard-cancel the previous assignee's session — the interrupt guard's
// comparison (moved inside reactToAssigneeChange by this feature) must not
// have accidentally suppressed the case it was already handling.
func TestApplyTaskMutation_DifferentAgentReassignment_InterruptsPreviousAssignee(t *testing.T) {
repo := newReactivityTestRepo(t)
ss := newChildrenCompletedTestScheduler(t, repo)
createChildrenCompletedAgent(t, repo, "agent-old")
createChildrenCompletedAgent(t, repo, "agent-new")
task := &TaskSnapshot{
ID: "task-reassign",
WorkspaceID: "ws-1",
AssigneeAgentProfileID: "agent-old",
}
newAssignee := "agent-new"
gen := int64(1)
change := TaskMutation{
NewAssigneeID: &newAssignee,
AssignmentGeneration: &gen,
ActorID: "user-1",
ActorType: "user",
}
res, err := ss.ApplyTaskMutation(context.Background(), task, change)
if err != nil {
t.Fatalf("ApplyTaskMutation: %v", err)
}
if res.InterruptSessionID != task.ID {
t.Fatalf("InterruptSessionID = %q, want %q (previous assignee's session must be cancelled)", res.InterruptSessionID, task.ID)
}
found := false
for _, r := range res.Runs {
if r.AgentID == "agent-new" && r.Reason == RunReasonTaskAssigned {
found = true
}
}
if !found {
t.Fatalf("expected a task_assigned run queued for agent-new, got %+v", res.Runs)
}
}
// TestApplyTaskMutation_StatusChangeAndCommentDispatch drives
// ApplyTaskMutation's NewStatus and Comment branches together (an unblock
// plus an attached comment), covering the dispatcher itself rather than only
// the reactToStatusChange/reactToComment helpers it delegates to.
func TestApplyTaskMutation_StatusChangeAndCommentDispatch(t *testing.T) {
repo := newReactivityTestRepo(t)
ss := newChildrenCompletedTestScheduler(t, repo)
createChildrenCompletedAgent(t, repo, "agent-dispatch")
task := &TaskSnapshot{
ID: "task-dispatch",
WorkspaceID: "ws-1",
State: "BLOCKED",
AssigneeAgentProfileID: "agent-dispatch",
}
newStatus := "todo"
change := TaskMutation{
NewStatus: &newStatus,
Comment: &MutationComment{ID: "comment-dispatch", AuthorType: "user", AuthorID: "user-1"},
ActorID: "user-1",
ActorType: "user",
}
res, err := ss.ApplyTaskMutation(context.Background(), task, change)
if err != nil {
t.Fatalf("ApplyTaskMutation: %v", err)
}
reasons := map[string]bool{}
for _, r := range res.Runs {
reasons[r.Reason] = true
}
if !reasons[RunReasonTaskUnblocked] {
t.Fatalf("expected %s among dispatched runs, got %+v", RunReasonTaskUnblocked, res.Runs)
}
if !reasons[RunReasonTaskComment] {
t.Fatalf("expected %s among dispatched runs, got %+v", RunReasonTaskComment, res.Runs)
}
}
@@ -0,0 +1,117 @@
package scheduler
import (
"context"
"fmt"
"testing"
officesqlite "github.com/kandev/kandev/internal/office/repository/sqlite"
"github.com/kandev/kandev/internal/runs/dedupkeys"
)
// insertBlockerRelationship records that taskID is blocked by
// blockerTaskID, and gives blockerTaskID the given (already-terminal or
// not) state.
func insertBlockerRelationship(
t *testing.T, repo *officesqlite.Repository, taskID, blockerTaskID, blockerState string,
) {
t.Helper()
ctx := context.Background()
if _, err := repo.ExecRaw(ctx,
`INSERT INTO tasks (id, workspace_id, state) VALUES (?, 'ws-1', ?)`,
blockerTaskID, blockerState,
); err != nil {
t.Fatalf("insert blocker task %s: %v", blockerTaskID, err)
}
if _, err := repo.ExecRaw(ctx,
`INSERT INTO task_blockers (task_id, blocker_task_id, created_at) VALUES (?, ?, CURRENT_TIMESTAMP)`,
taskID, blockerTaskID,
); err != nil {
t.Fatalf("insert task_blockers row (%s blocked by %s): %v", taskID, blockerTaskID, err)
}
}
// TestCascadeBlockersResolved_QueuesRunWithSharedDigestKey is the
// regression test for both blocker-resolution producers converging on the
// same key shape (AC-OFFICE-RUN-DEDUP-002.1, SR-59's resolution: only the
// digest need be byte-identical, not the whole key/opID — see
// docs/specs/office/system-design/run-dedup-generation-03.md#the-shared-key-builders).
// Before this test, office/scheduler.cascadeBlockersResolved and
// allBlockersResolvedExcept had zero coverage: nothing exercised the
// blocked task's wake at all, let alone the specific
// dedupkeys.BlockerDigest call.
func TestCascadeBlockersResolved_QueuesRunWithSharedDigestKey(t *testing.T) {
repo := newReactivityTestRepo(t)
ss := newChildrenCompletedTestScheduler(t, repo)
createChildrenCompletedAgent(t, repo, "agent-1")
queue := newChildrenCompletedQueue(t, ss)
ctx := context.Background()
// blocked-1 is blocked by blocker-a (already done) and blocker-b (the
// one resolving now). setupChildrenCompletedParent gives blocked-1 a
// runner participant row so GetTaskAssignee resolves it to agent-1 —
// reused here even though it's named for the children-completed tests,
// because it does exactly what a blocked task's assignee wiring needs.
setupChildrenCompletedParent(t, ss, "blocked-1", "agent-1")
insertBlockerRelationship(t, repo, "blocked-1", "blocker-a", "COMPLETED")
insertBlockerRelationship(t, repo, "blocked-1", "blocker-b", "IN_PROGRESS")
// blocker-b finishes: it's the second and last outstanding blocker.
if _, err := repo.ExecRaw(ctx, `UPDATE tasks SET state = 'COMPLETED' WHERE id = 'blocker-b'`); err != nil {
t.Fatalf("complete blocker-b: %v", err)
}
ss.cascadeBlockersResolved(ctx, &TaskSnapshot{ID: "blocker-b", WorkspaceID: "ws-1"}, queue)
runs, err := repo.ListRuns(ctx, "ws-1")
if err != nil {
t.Fatalf("list runs: %v", err)
}
var got *string
for _, run := range runs {
if run.Reason == RunReasonTaskBlockersResolved && run.AgentProfileID == "agent-1" {
got = run.IdempotencyKey
break
}
}
if got == nil {
t.Fatalf("no %s run persisted for agent-1: runs=%#v", RunReasonTaskBlockersResolved, runs)
}
// The key's digest segment must be exactly what the OTHER blocker
// producer (office/service.resolveAndWakeIfUnblocked) would derive
// for the identical blocker set, in either read order — that's the
// structural convergence AC-002.1 requires.
digestAscending := dedupkeys.BlockerDigest([]string{"blocker-a", "blocker-b"})
digestDescending := dedupkeys.BlockerDigest([]string{"blocker-b", "blocker-a"})
if digestAscending != digestDescending {
t.Fatalf("BlockerDigest is not order-independent: %q vs %q", digestAscending, digestDescending)
}
want := fmt.Sprintf("%s:blocked-1:agent-1:%s", RunReasonTaskBlockersResolved, digestAscending)
if *got != want {
t.Fatalf("idempotency key = %q, want %q", *got, want)
}
}
// TestCascadeBlockersResolved_StillBlocked_DoesNotQueue proves the
// negative: cascadeBlockersResolved must not wake the blocked task while
// another blocker is still outstanding.
func TestCascadeBlockersResolved_StillBlocked_DoesNotQueue(t *testing.T) {
repo := newReactivityTestRepo(t)
ss := newChildrenCompletedTestScheduler(t, repo)
createChildrenCompletedAgent(t, repo, "agent-1")
queue := newChildrenCompletedQueue(t, ss)
ctx := context.Background()
setupChildrenCompletedParent(t, ss, "blocked-2", "agent-1")
insertBlockerRelationship(t, repo, "blocked-2", "blocker-c", "IN_PROGRESS") // never finishes
insertBlockerRelationship(t, repo, "blocked-2", "blocker-d", "IN_PROGRESS")
if _, err := repo.ExecRaw(ctx, `UPDATE tasks SET state = 'COMPLETED' WHERE id = 'blocker-d'`); err != nil {
t.Fatalf("complete blocker-d: %v", err)
}
ss.cascadeBlockersResolved(ctx, &TaskSnapshot{ID: "blocker-d", WorkspaceID: "ws-1"}, queue)
if got := runsCountForReason(t, ss, RunReasonTaskBlockersResolved); got != 0 {
t.Fatalf("persisted runs = %d, want 0 (blocker-c is still unresolved)", got)
}
}
@@ -50,7 +50,7 @@ func createChildrenCompletedAgent(t *testing.T, repo *officesqlite.Repository, i
func newChildrenCompletedQueue(t *testing.T, ss *SchedulerService) func(string, RunContext) {
t.Helper()
return func(agentID string, c RunContext) {
if err := ss.QueueRunCtx(context.Background(), agentID, c); err != nil {
if _, err := ss.QueueRunCtx(context.Background(), agentID, c); err != nil {
t.Fatalf("QueueRunCtx: %v", err)
}
}
@@ -0,0 +1,372 @@
package scheduler
import (
"context"
"expvar"
"testing"
officesqlite "github.com/kandev/kandev/internal/office/repository/sqlite"
"github.com/kandev/kandev/internal/runs/commentkeys"
"github.com/kandev/kandev/internal/runs/dedupkeys"
)
// newAuditScheduler wires a real repo + real *service.Service (reused from
// reactivity_children_completed_test.go's newChildrenCompletedTestScheduler)
// so every producer under audit — including the two (task_blockers_resolved,
// task_review_requested) that read participants/blockers off the repo, and
// task_mentioned, which resolves mentions via ss.svc.ListAgentInstances —
// can run unmodified.
func newAuditScheduler(t *testing.T) (*SchedulerService, *officesqlite.Repository) {
t.Helper()
repo := newReactivityTestRepo(t)
return newChildrenCompletedTestScheduler(t, repo), repo
}
// requireOneCallKey finds the single recorded queue call for (reason, agentID)
// and returns its IdempotencyKey, failing the test if no such call exists.
func requireOneCallKey(t *testing.T, calls []recordedQueueCall, reason, agentID string) string {
t.Helper()
for _, c := range calls {
if c.agentID == agentID && c.ctx.Reason == reason {
return c.ctx.IdempotencyKey
}
}
t.Fatalf("no %s call recorded for agent %q: calls=%#v", reason, agentID, calls)
return ""
}
// requireOneCallKeyless is requireOneCallKey's keyless counterpart: it
// asserts the matching call carries NO idempotency key, rather than
// asserting a value.
func requireOneCallKeyless(t *testing.T, calls []recordedQueueCall, reason, agentID string) {
t.Helper()
for _, c := range calls {
if c.agentID == agentID && c.ctx.Reason == reason {
if c.ctx.IdempotencyKey != "" {
t.Fatalf("%s call for agent %q carries key %q, want keyless", reason, agentID, c.ctx.IdempotencyKey)
}
return
}
}
t.Fatalf("no %s call recorded for agent %q: calls=%#v", reason, agentID, calls)
}
// auditKeylessCounterValue returns the current value of the process-global
// office_run_dedup_keyless_total expvar map (internal/runs/service) for the
// exact "reason=<reason>;cause=<cause>" label, or 0 if that label has never
// been incremented. Duplicated locally rather than imported: the counter map
// is unexported and the sibling helper in internal/runs/service/dedup_test.go
// lives in a different package. Callers snapshot before and after a producer
// call and assert the delta, rather than merely that the label exists — a
// mere-existence check would pass even if this specific call site failed to
// report, as long as some other test already set that label.
func auditKeylessCounterValue(t *testing.T, reason, cause string) int64 {
t.Helper()
v := expvar.Get("office_run_dedup_keyless_total")
if v == nil {
t.Fatalf("expvar map office_run_dedup_keyless_total not registered")
}
m, ok := v.(*expvar.Map)
if !ok {
t.Fatalf("office_run_dedup_keyless_total is not a *expvar.Map")
}
iv := m.Get("reason=" + reason + ";cause=" + cause)
if iv == nil {
return 0
}
i, ok := iv.(*expvar.Int)
if !ok {
t.Fatalf("counter value for reason=%s;cause=%s is not *expvar.Int", reason, cause)
}
return i.Value()
}
// TestReactivityProducerKeyAudit is the Part 2-required "producer-audit
// key-format table test" (docs/specs/office/system-design/
// run-dedup-generation-02.md#test-strategy): a table over every reactivity
// producer named in the "Generation sources per producer" table
// (…-01.md#generation-sources-per-producer), asserting each one derives the
// spec's exact key shape, or reports the spec's exact keyless cause when the
// reason has no durable occurrence to key on. Review round 1 found 6 of this
// file's 8 run reasons silently doing neither — this is the design's own
// named enforcement mechanism whose absence is why that gap shipped
// undetected, and what stops the next reason added to reactivity.go from
// reintroducing it silently.
func TestReactivityProducerKeyAudit(t *testing.T) {
t.Run("task_assigned", func(t *testing.T) {
ss, _ := newAuditScheduler(t)
var calls []recordedQueueCall
queue := func(agentID string, c RunContext) {
calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
}
task := &TaskSnapshot{ID: "task-audit-assigned", WorkspaceID: "ws-1"}
gen := int64(3)
res := &ApplyTaskMutationResult{}
ss.reactToAssigneeChange(task, "agent-assigned", TaskMutation{
AssignmentGeneration: &gen, ActorID: "user-1", ActorType: "user",
}, queue, res)
key := requireOneCallKey(t, calls, RunReasonTaskAssigned, "agent-assigned")
if want := dedupkeys.AssignmentKey(task.ID, "agent-assigned", gen); key != want {
t.Fatalf("task_assigned key = %q, want %q", key, want)
}
})
t.Run("task_blockers_resolved", func(t *testing.T) {
ss, repo := newAuditScheduler(t)
setupChildrenCompletedParent(t, ss, "blocked-audit", "agent-blocked")
insertBlockerRelationship(t, repo, "blocked-audit", "blocker-audit-a", "COMPLETED")
insertBlockerRelationship(t, repo, "blocked-audit", "blocker-audit-b", "COMPLETED")
var calls []recordedQueueCall
queue := func(agentID string, c RunContext) {
calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
}
ss.cascadeBlockersResolved(context.Background(),
&TaskSnapshot{ID: "blocker-audit-b", WorkspaceID: "ws-1"}, queue)
key := requireOneCallKey(t, calls, RunReasonTaskBlockersResolved, "agent-blocked")
digest := dedupkeys.BlockerDigest([]string{"blocker-audit-a", "blocker-audit-b"})
want := RunReasonTaskBlockersResolved + ":blocked-audit:agent-blocked:" + digest
if key != want {
t.Fatalf("task_blockers_resolved key = %q, want %q", key, want)
}
})
t.Run("task_comment", func(t *testing.T) {
ss, _ := newAuditScheduler(t)
task := &TaskSnapshot{ID: "task-audit-comment", WorkspaceID: "ws-1", AssigneeAgentProfileID: "agent-assignee"}
comment := &MutationComment{ID: "comment-audit-1", Body: "status update", AuthorType: "user", AuthorID: "user-1"}
var calls []recordedQueueCall
queue := func(agentID string, c RunContext) {
calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
}
ss.reactToComment(context.Background(), task, comment, queue)
key := requireOneCallKey(t, calls, RunReasonTaskComment, "agent-assignee")
want := commentkeys.TaskComment("comment-audit-1") + ":agent-assignee"
if key != want {
t.Fatalf("task_comment key = %q, want %q", key, want)
}
})
t.Run("task_mentioned", func(t *testing.T) {
ss, repo := newAuditScheduler(t)
createChildrenCompletedAgent(t, repo, "agent-mentioned")
task := &TaskSnapshot{ID: "task-audit-mentioned", WorkspaceID: "ws-1"}
comment := &MutationComment{
ID: "comment-audit-2", AuthorType: "user", AuthorID: "user-1",
// The mention token regex allows internal spaces (for multi-word
// agent names), so it greedily consumes everything after the "@"
// up to end of string or a disallowed character. Keeping the
// mention last with nothing trailing bounds the capture to
// exactly the agent name.
Body: "cc @agent-mentioned",
// Isolate the mention path from the assignee-wake path (task has
// no assignee here, but SkipAssigneeWake documents the intent).
SkipAssigneeWake: true,
}
var calls []recordedQueueCall
queue := func(agentID string, c RunContext) {
calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
}
ss.reactToComment(context.Background(), task, comment, queue)
key := requireOneCallKey(t, calls, RunReasonTaskMentioned, "agent-mentioned")
want := RunReasonTaskMentioned + ":comment-audit-2:agent-mentioned"
if key != want {
t.Fatalf("task_mentioned key = %q, want %q", key, want)
}
})
t.Run("task_reopened_via_comment", func(t *testing.T) {
ss, _ := newAuditScheduler(t)
task := &TaskSnapshot{ID: "task-audit-reopen-comment", WorkspaceID: "ws-1", State: "DONE", AssigneeAgentProfileID: "agent-reopen"}
res := &ApplyTaskMutationResult{}
var calls []recordedQueueCall
queue := func(agentID string, c RunContext) {
calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
}
change := TaskMutation{
Comment: &MutationComment{ID: "comment-audit-3", AuthorType: "user", AuthorID: "user-1"},
ActorID: "user-1",
ActorType: "user",
}
ss.reactToStatusChange(context.Background(), task, "todo", change, queue, res)
key := requireOneCallKey(t, calls, RunReasonTaskReopenedComment, "agent-reopen")
want := RunReasonTaskReopenedComment + ":comment-audit-3:agent-reopen"
if key != want {
t.Fatalf("task_reopened_via_comment key = %q, want %q", key, want)
}
})
t.Run("task_unblocked", func(t *testing.T) {
ss, _ := newAuditScheduler(t)
task := &TaskSnapshot{ID: "task-audit-unblocked", WorkspaceID: "ws-1", State: "BLOCKED", AssigneeAgentProfileID: "agent-unblocked"}
res := &ApplyTaskMutationResult{}
var calls []recordedQueueCall
queue := func(agentID string, c RunContext) {
calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
}
before := auditKeylessCounterValue(t, RunReasonTaskUnblocked, "by_design")
change := TaskMutation{ActorID: "user-1", ActorType: "user"}
ss.reactToStatusChange(context.Background(), task, "todo", change, queue, res)
requireOneCallKeyless(t, calls, RunReasonTaskUnblocked, "agent-unblocked")
if got, want := auditKeylessCounterValue(t, RunReasonTaskUnblocked, "by_design"), before+1; got != want {
t.Fatalf("office_run_dedup_keyless_total{task_unblocked,by_design} = %d, want %d", got, want)
}
})
t.Run("task_unblocked_no_assignee_does_not_report", func(t *testing.T) {
// Regression test for Review round 2 Finding 3: an unassigned task
// must not increment the keyless counter for an enqueue that never
// happens. Goes through ApplyTaskMutation (not reactToStatusChange
// directly) so the real queue closure's empty-agent-id guard
// (reactivity.go's `if agentID == "" { return }`) is exercised —
// that guard is exactly what silently swallowed the enqueue while
// FINDING 3's ReportKeylessEnqueue call still fired unconditionally.
ss, _ := newAuditScheduler(t)
task := &TaskSnapshot{ID: "task-audit-unblocked-noassignee", WorkspaceID: "ws-1", State: "BLOCKED"}
before := auditKeylessCounterValue(t, RunReasonTaskUnblocked, "by_design")
newStatus := "todo"
change := TaskMutation{NewStatus: &newStatus, ActorID: "user-1", ActorType: "user"}
res, err := ss.ApplyTaskMutation(context.Background(), task, change)
if err != nil {
t.Fatalf("ApplyTaskMutation: %v", err)
}
if len(res.Runs) != 0 {
t.Fatalf("expected no queued runs for an unassigned task_unblocked, got %#v", res.Runs)
}
if got := auditKeylessCounterValue(t, RunReasonTaskUnblocked, "by_design"); got != before {
t.Fatalf("office_run_dedup_keyless_total{task_unblocked,by_design} = %d, want unchanged %d (no enqueue attempted)", got, before)
}
})
t.Run("task_reopened", func(t *testing.T) {
ss, _ := newAuditScheduler(t)
task := &TaskSnapshot{ID: "task-audit-reopen-silent", WorkspaceID: "ws-1", State: "CANCELLED", AssigneeAgentProfileID: "agent-silent-reopen"}
res := &ApplyTaskMutationResult{}
var calls []recordedQueueCall
queue := func(agentID string, c RunContext) {
calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
}
before := auditKeylessCounterValue(t, RunReasonTaskReopened, "by_design")
change := TaskMutation{ActorID: "user-1", ActorType: "user"}
ss.reactToStatusChange(context.Background(), task, "in_progress", change, queue, res)
requireOneCallKeyless(t, calls, RunReasonTaskReopened, "agent-silent-reopen")
if got, want := auditKeylessCounterValue(t, RunReasonTaskReopened, "by_design"), before+1; got != want {
t.Fatalf("office_run_dedup_keyless_total{task_reopened,by_design} = %d, want %d", got, want)
}
})
t.Run("task_reopened_no_assignee_does_not_report", func(t *testing.T) {
// Regression test for Review round 2 Finding 3, silent-reopen branch.
// Goes through ApplyTaskMutation for the same reason as the
// task_unblocked case above: the empty-agent-id guard lives in the
// real queue closure, not in reactToStatusChange itself.
ss, _ := newAuditScheduler(t)
task := &TaskSnapshot{ID: "task-audit-reopen-silent-noassignee", WorkspaceID: "ws-1", State: "CANCELLED"}
before := auditKeylessCounterValue(t, RunReasonTaskReopened, "by_design")
newStatus := "in_progress"
change := TaskMutation{NewStatus: &newStatus, ActorID: "user-1", ActorType: "user"}
res, err := ss.ApplyTaskMutation(context.Background(), task, change)
if err != nil {
t.Fatalf("ApplyTaskMutation: %v", err)
}
if len(res.Runs) != 0 {
t.Fatalf("expected no queued runs for an unassigned silent reopen, got %#v", res.Runs)
}
if got := auditKeylessCounterValue(t, RunReasonTaskReopened, "by_design"); got != before {
t.Fatalf("office_run_dedup_keyless_total{task_reopened,by_design} = %d, want unchanged %d (no enqueue attempted)", got, before)
}
})
t.Run("task_review_requested", func(t *testing.T) {
ss, repo := newAuditScheduler(t)
ctx := context.Background()
if _, err := repo.ExecRaw(ctx, `
INSERT INTO tasks (id, workspace_id, workflow_step_id) VALUES ('task-audit-review', 'ws-1', 'step-audit')
`); err != nil {
t.Fatalf("insert task: %v", err)
}
if _, err := repo.ExecRaw(ctx, `
INSERT INTO workflow_step_participants
(id, step_id, task_id, role, agent_profile_id, decision_required, position)
VALUES ('p-audit-reviewer', 'step-audit', 'task-audit-review', 'reviewer', 'agent-reviewer-audit', 1, 0)
`); err != nil {
t.Fatalf("insert participant: %v", err)
}
task := &TaskSnapshot{ID: "task-audit-review", WorkspaceID: "ws-1"}
var calls []recordedQueueCall
queue := func(agentID string, c RunContext) {
calls = append(calls, recordedQueueCall{agentID: agentID, ctx: c})
}
before := auditKeylessCounterValue(t, RunReasonTaskReviewRequested, "by_design")
ss.cascadeReviewRequested(ctx, task, TaskMutation{ActorID: "user-1", ActorType: "user"}, queue)
requireOneCallKeyless(t, calls, RunReasonTaskReviewRequested, "agent-reviewer-audit")
if got, want := auditKeylessCounterValue(t, RunReasonTaskReviewRequested, "by_design"), before+1; got != want {
t.Fatalf("office_run_dedup_keyless_total{task_review_requested,by_design} = %d, want %d", got, want)
}
})
t.Run("task_review_requested_dual_role_recipient_counts_once", func(t *testing.T) {
// Regression test for Review round 3 Finding 4: an agent seated as
// both reviewer and approver on the same task is one recipient, not
// two. Driven through the real ApplyTaskMutation (not a bare
// closure) so the real queue closure's seen-map dedup — which
// silently absorbs the second queue() call for the same agent —
// is what proves the counter must match the number of enqueue
// attempts actually made, not the number of participant rows read.
ss, repo := newAuditScheduler(t)
ctx := context.Background()
createChildrenCompletedAgent(t, repo, "agent-dual-role")
if _, err := repo.ExecRaw(ctx, `
INSERT INTO tasks (id, workspace_id, workflow_step_id) VALUES ('task-audit-review-dual', 'ws-1', 'step-audit')
`); err != nil {
t.Fatalf("insert task: %v", err)
}
if _, err := repo.ExecRaw(ctx, `
INSERT INTO workflow_step_participants
(id, step_id, task_id, role, agent_profile_id, decision_required, position)
VALUES ('p-audit-dual-reviewer', 'step-audit', 'task-audit-review-dual', 'reviewer', 'agent-dual-role', 1, 0)
`); err != nil {
t.Fatalf("insert reviewer participant: %v", err)
}
if _, err := repo.ExecRaw(ctx, `
INSERT INTO workflow_step_participants
(id, step_id, task_id, role, agent_profile_id, decision_required, position)
VALUES ('p-audit-dual-approver', 'step-audit', 'task-audit-review-dual', 'approver', 'agent-dual-role', 1, 1)
`); err != nil {
t.Fatalf("insert approver participant: %v", err)
}
task := &TaskSnapshot{ID: "task-audit-review-dual", WorkspaceID: "ws-1", State: statusTodo}
before := auditKeylessCounterValue(t, RunReasonTaskReviewRequested, "by_design")
newStatus := statusInReview
change := TaskMutation{NewStatus: &newStatus, ActorID: "user-1", ActorType: "user"}
res, err := ss.ApplyTaskMutation(ctx, task, change)
if err != nil {
t.Fatalf("ApplyTaskMutation: %v", err)
}
queued := 0
for _, r := range res.Runs {
if r.Reason == RunReasonTaskReviewRequested && r.AgentID == "agent-dual-role" {
queued++
}
}
if queued != 1 {
t.Fatalf("expected exactly 1 queued task_review_requested run for the dual-role agent, got %d (%#v)", queued, res.Runs)
}
if got, want := auditKeylessCounterValue(t, RunReasonTaskReviewRequested, "by_design"), before+1; got != want {
t.Fatalf("office_run_dedup_keyless_total{task_review_requested,by_design} = %d, want %d (one recipient, not one row per role)", got, want)
}
})
}
+32 -32
View File
@@ -19,6 +19,7 @@ import (
"github.com/kandev/kandev/internal/office/routing"
"github.com/kandev/kandev/internal/office/service"
"github.com/kandev/kandev/internal/office/shared"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// ErrRoutingNotSupported is returned by TaskStarter.StartTaskWithRoute
@@ -107,13 +108,13 @@ type RunContext struct {
// task_changes_requested has the context inline.
DecisionComment string `json:"decision_comment,omitempty"`
// IdempotencyKey, when non-empty, overrides the default
// "{reason}:{taskID}:{agentID}" key QueueRunCtx mints. The default
// key is permanently unique per (reason, task, agent) — fine for
// reasons that fire at most once per task, wrong for a reason that
// can legitimately recur (e.g. task_children_completed across
// repeated delegation waves). Callers that recur build their own
// key that changes with the thing that makes each occurrence
// IdempotencyKey is the dedup identity QueueRunCtx passes through
// verbatim. Empty means no dedup — the run enqueues keyless — not
// "derive one for me": QueueRunCtx no longer synthesises a
// "{reason}:{taskID}:{agentID}" default, which was permanently unique
// per (reason, task, agent) and silently swallowed every later
// legitimate occurrence for the same triple. Callers that want dedup
// build a key that changes with the thing that makes each occurrence
// distinct. Excluded from the JSON payload: it must never change
// encodeRunContext's output shape, which CoalesceRun compares for
// equality and taskIDFromPayload parses.
@@ -248,32 +249,30 @@ func (ss *SchedulerService) SetProjectSkillDirResolver(fn func(agentTypeID strin
func (ss *SchedulerService) QueueRun(
ctx context.Context,
agentInstanceID, reason, payload, idempotencyKey string,
) error {
) (runsservice.QueueOutcome, error) {
if err := ss.guardAgentStatus(ctx, agentInstanceID); err != nil {
return err
return runsservice.QueueOutcomeNone, err
}
if idempotencyKey != "" {
dup, err := ss.repo.CheckIdempotencyKey(ctx, idempotencyKey, IdempotencyWindowHours)
if err != nil {
return fmt.Errorf("idempotency check: %w", err)
return runsservice.QueueOutcomeNone, fmt.Errorf("idempotency check: %w", err)
}
if dup {
ss.logger.Debug("run skipped (idempotent)",
zap.String("key", idempotencyKey))
return nil
return runsservice.ReportWindowedDedup(runsservice.QueueSourceRuns, reason, idempotencyKey), nil
}
}
coalesced, err := ss.repo.CoalesceRun(ctx, agentInstanceID, reason, CoalesceWindowSeconds, payload)
if err != nil {
return fmt.Errorf("coalesce check: %w", err)
return runsservice.QueueOutcomeNone, fmt.Errorf("coalesce check: %w", err)
}
if coalesced {
ss.logger.Debug("run coalesced",
zap.String("agent", agentInstanceID),
zap.String("reason", reason))
return nil
return runsservice.QueueOutcomeCoalesced, nil
}
var idemKeyPtr *string
@@ -290,36 +289,37 @@ func (ss *SchedulerService) QueueRun(
IdempotencyKey: idemKeyPtr,
RequestedAt: time.Now().UTC(),
}
if err := ss.repo.CreateRun(ctx, req); err != nil {
return fmt.Errorf("enqueue run: %w", err)
insertErr := ss.repo.CreateRun(ctx, req)
outcome, err := runsservice.ReportInsertResult(runsservice.QueueSourceRuns, reason, idempotencyKey, agentInstanceID, insertErr)
if err != nil {
return runsservice.QueueOutcomeNone, fmt.Errorf("enqueue run: %w", err)
}
if outcome == runsservice.QueueOutcomeDeduped {
return outcome, nil
}
ss.logger.Info("run queued",
zap.String("id", req.ID),
zap.String("agent", agentInstanceID),
zap.String("reason", reason))
return nil
return runsservice.QueueOutcomeQueued, nil
}
// QueueRunCtx is the typed variant of QueueRun that takes a
// structured RunContext. The context is JSON-encoded into the
// payload column so the agent runtime can deserialise it. The
// idempotency key is c.IdempotencyKey when the caller set one;
// otherwise it defaults to "{reason}:{taskID}:{agentID}" so the same
// agent never gets two runs for the same task+reason within the
// idempotency window.
// QueueRunCtx is the typed variant of QueueRun that takes a structured
// RunContext. The context is JSON-encoded into the payload column so the
// agent runtime can deserialise it. The idempotency key is c.IdempotencyKey
// verbatim — an empty key enqueues with no dedup identity rather than
// falling back to a "{reason}:{taskID}:{agentID}" default that would be
// permanently unique per (reason, task, agent) and silently swallow every
// later legitimate occurrence for the same triple.
func (ss *SchedulerService) QueueRunCtx(
ctx context.Context, agentInstanceID string, c RunContext,
) error {
) (runsservice.QueueOutcome, error) {
payload, err := encodeRunContext(c)
if err != nil {
return fmt.Errorf("encode run context: %w", err)
return runsservice.QueueOutcomeNone, fmt.Errorf("encode run context: %w", err)
}
idempotencyKey := c.IdempotencyKey
if idempotencyKey == "" {
idempotencyKey = fmt.Sprintf("%s:%s:%s", c.Reason, c.TaskID, agentInstanceID)
}
return ss.QueueRun(ctx, agentInstanceID, c.Reason, payload, idempotencyKey)
return ss.QueueRun(ctx, agentInstanceID, c.Reason, payload, c.IdempotencyKey)
}
func encodeRunContext(c RunContext) (string, error) {
@@ -40,7 +40,7 @@ func launchedWorkingAgent(
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, project_id, title, created_at, updated_at)
VALUES (?, 'ws-1', ?, 'Working status task', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
taskID, project.ID)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, ""); err != nil {
t.Fatalf("queue run: %v", err)
}
@@ -54,6 +54,7 @@ func newTestService(t *testing.T, overrides ...service.ServiceOptions) *service.
state TEXT NOT NULL DEFAULT 'TODO',
title TEXT DEFAULT '',
assignee_user_id TEXT NOT NULL DEFAULT '',
assignment_generation INTEGER NOT NULL DEFAULT 0,
description TEXT DEFAULT '',
identifier TEXT DEFAULT '',
workflow_id TEXT DEFAULT '',
@@ -0,0 +1,192 @@
package service
import (
"context"
"sync"
"testing"
"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/models"
"github.com/kandev/kandev/internal/office/repository/sqlite"
"github.com/kandev/kandev/internal/runs/dedupkeys"
"github.com/kandev/kandev/internal/workflow/engine"
)
// newBlockersConvergenceTestRepo builds a real office repository (real
// migrations, so task_blockers is the production schema) plus the minimal
// hand-rolled tasks table resolveAndWakeIfUnblocked reads state from —
// tasks is owned by the task package's schema, which office's migrations
// only reference by foreign key.
func newBlockersConvergenceTestRepo(t *testing.T) *sqlite.Repository {
t.Helper()
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)
}
ctx := context.Background()
if _, err := repo.ExecRaw(ctx, `
CREATE TABLE IF NOT EXISTS tasks (
id TEXT PRIMARY KEY,
workspace_id TEXT DEFAULT '',
state TEXT DEFAULT ''
)
`); err != nil {
t.Fatalf("create tasks table: %v", err)
}
return repo
}
// convergenceDispatcher records every HandleTrigger call so the test can
// assert the exact operation id resolveAndWakeIfUnblocked derived.
type convergenceDispatcher struct {
mu sync.Mutex
calls []convergenceCall
}
type convergenceCall struct {
taskID string
trigger engine.Trigger
payload any
opID string
}
func (d *convergenceDispatcher) HandleTrigger(
_ context.Context, taskID string, trigger engine.Trigger, payload any, opID string,
) error {
d.mu.Lock()
defer d.mu.Unlock()
d.calls = append(d.calls, convergenceCall{taskID, trigger, payload, opID})
return nil
}
func newConvergenceTestService(t *testing.T, repo *sqlite.Repository, dispatcher *convergenceDispatcher) *Service {
t.Helper()
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "console"})
if err != nil {
t.Fatalf("logger: %v", err)
}
s := &Service{repo: repo, logger: log}
s.SetWorkflowEngineDispatcher(dispatcher)
return s
}
// TestResolveAndWakeIfUnblocked_DerivesSameDigestAsCascadeBlockersResolved
// is the AC-OFFICE-RUN-DEDUP-002.1 convergence test for the second blocker
// producer. office/scheduler.cascadeBlockersResolved (the other half of the
// same AC) already has a regression test
// (reactivity_blockers_resolved_test.go), but that test never exercises
// this package's resolveAndWakeIfUnblocked — it only proves
// dedupkeys.BlockerDigest is order-independent in isolation. Review round 2
// found the AC's own explicit test requirement ("both producers derive a
// byte-identical digest, driven through the shared builder from both
// packages") unproven by any executing test. This test drives
// resolveAndWakeIfUnblocked directly and asserts its persisted operation id
// carries the exact digest office/scheduler's producer would derive for the
// identical blocker set.
func TestResolveAndWakeIfUnblocked_DerivesSameDigestAsCascadeBlockersResolved(t *testing.T) {
repo := newBlockersConvergenceTestRepo(t)
ctx := context.Background()
for _, row := range []struct{ id, state string }{
{"blocked-1", "IN_PROGRESS"},
{"blocker-conv-a", "COMPLETED"},
{"blocker-conv-b", "COMPLETED"},
} {
if _, err := repo.ExecRaw(ctx,
`INSERT INTO tasks (id, workspace_id, state) VALUES (?, 'ws-1', ?)`,
row.id, row.state,
); err != nil {
t.Fatalf("insert task %s: %v", row.id, err)
}
}
for _, blockerID := range []string{"blocker-conv-a", "blocker-conv-b"} {
if err := repo.CreateTaskBlocker(ctx, &models.TaskBlocker{
TaskID: "blocked-1", BlockerTaskID: blockerID,
}); err != nil {
t.Fatalf("create blocker relationship for %s: %v", blockerID, err)
}
}
dispatcher := &convergenceDispatcher{}
s := newConvergenceTestService(t, repo, dispatcher)
if err := s.resolveAndWakeIfUnblocked(ctx, "blocked-1", "blocker-conv-b"); err != nil {
t.Fatalf("resolveAndWakeIfUnblocked: %v", err)
}
dispatcher.mu.Lock()
calls := append([]convergenceCall{}, dispatcher.calls...)
dispatcher.mu.Unlock()
if len(calls) != 1 {
t.Fatalf("HandleTrigger calls = %d, want 1: %#v", len(calls), calls)
}
call := calls[0]
if call.taskID != "blocked-1" {
t.Fatalf("taskID = %q, want blocked-1", call.taskID)
}
if call.trigger != engine.TriggerOnBlockerResolved {
t.Fatalf("trigger = %q, want %q", call.trigger, engine.TriggerOnBlockerResolved)
}
// The digest office/scheduler.cascadeBlockersResolved would derive for
// the identical blocker set — proving the two independently-implemented
// producers converge on the same occurrence identity, per AC-002.1.
digest := dedupkeys.BlockerDigest([]string{"blocker-conv-a", "blocker-conv-b"})
want := "blockers_resolved:blocked-1:" + digest
if call.opID != want {
t.Fatalf("operation id = %q, want %q", call.opID, want)
}
}
// TestResolveAndWakeIfUnblocked_StillBlocked_DoesNotDispatch proves the
// negative: a task with another outstanding blocker must not wake.
func TestResolveAndWakeIfUnblocked_StillBlocked_DoesNotDispatch(t *testing.T) {
repo := newBlockersConvergenceTestRepo(t)
ctx := context.Background()
for _, row := range []struct{ id, state string }{
{"blocked-2", "IN_PROGRESS"},
{"blocker-conv-c", "IN_PROGRESS"}, // never finishes
{"blocker-conv-d", "COMPLETED"},
} {
if _, err := repo.ExecRaw(ctx,
`INSERT INTO tasks (id, workspace_id, state) VALUES (?, 'ws-1', ?)`,
row.id, row.state,
); err != nil {
t.Fatalf("insert task %s: %v", row.id, err)
}
}
for _, blockerID := range []string{"blocker-conv-c", "blocker-conv-d"} {
if err := repo.CreateTaskBlocker(ctx, &models.TaskBlocker{
TaskID: "blocked-2", BlockerTaskID: blockerID,
}); err != nil {
t.Fatalf("create blocker relationship for %s: %v", blockerID, err)
}
}
dispatcher := &convergenceDispatcher{}
s := newConvergenceTestService(t, repo, dispatcher)
if err := s.resolveAndWakeIfUnblocked(ctx, "blocked-2", "blocker-conv-d"); err != nil {
t.Fatalf("resolveAndWakeIfUnblocked: %v", err)
}
dispatcher.mu.Lock()
defer dispatcher.mu.Unlock()
if len(dispatcher.calls) != 0 {
t.Fatalf("expected no HandleTrigger calls while blocker-conv-c is still outstanding, got %#v", dispatcher.calls)
}
}
@@ -101,7 +101,7 @@ func TestAdmitRun_NoEvaluatorWired_UnattendedCancels(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -128,7 +128,7 @@ func TestAdmitRun_NoEvaluatorWired_AttendedLaunches(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
insertTestTask(t, svc, "task-no-evaluator-attended", "ws-1")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-no-evaluator-attended"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -163,7 +163,7 @@ func TestAdmitRun_EvaluatorFault_DefersThenFailsWithoutEscalation(t *testing.T)
t.Fatalf("create agent: %v", err)
}
insertTestTask(t, svc, "task-evaluator-fault", "ws-1")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-evaluator-fault"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -229,7 +229,7 @@ func TestAdmitRun_UnevaluatedPolicy_NamesPolicyInDeferral(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
insertTestTask(t, svc, "task-unevaluated-policy", "ws-1")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-unevaluated-policy"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -279,7 +279,7 @@ func TestFinishPolicyBlock_ClearsStaleAgentWorkingStatus(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -308,7 +308,7 @@ func TestCancelBudgetRun_ClearsStaleAgentWorkingStatus(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -345,7 +345,7 @@ func TestFailRunNoEscalation_ClearsStaleAgentWorkingStatus(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -388,7 +388,7 @@ func TestProcessRun_MissingAgentReleasesPriorCheckout(t *testing.T) {
INSERT INTO tasks (id, workspace_id, title, created_at, updated_at)
VALUES ('task-routed-missing', 'ws-1', 'Routed task', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-routed-missing"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -445,7 +445,7 @@ func TestProcessRun_WorkspaceLookupErrorReleasesPriorCheckout(t *testing.T) {
INSERT INTO tasks (id, workspace_id, title, created_at, updated_at)
VALUES ('task-routed-lookup-error', 'ws-1', 'Routed task', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-routed-lookup-error"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -496,7 +496,7 @@ func TestAdmitRun_RepeatedDeferral_AttemptIncrementsPerActivity(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
insertTestTask(t, svc, "task-attempt-increments", "ws-1")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-attempt-increments"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -536,7 +536,7 @@ func TestAdmitRun_DefaultCeiling_BlocksUnattendedRunWithZeroPolicies(t *testing.
t.Fatalf("create agent: %v", err)
}
insertTestCostEvent(t, svc, agent.ID, "task-default-ceiling", int64(600_000))
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -621,7 +621,7 @@ func TestAdmitRun_PricingDegradedBlock_UnmeasurableOutcome(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -652,7 +652,7 @@ func TestAdmitRun_DefaultCeiling_ExemptsAttendedRun(t *testing.T) {
}
insertTestCostEvent(t, svc, agent.ID, "task-default-ceiling-attended", int64(600_000))
insertTestTask(t, svc, "task-default-ceiling-attended-run", "ws-1")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-default-ceiling-attended-run"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -711,7 +711,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -732,7 +732,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -752,7 +752,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -775,7 +775,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
service.RunSchedulerTick(svc, ctx)
@@ -794,7 +794,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -825,7 +825,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -847,7 +847,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -870,7 +870,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -891,7 +891,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -918,7 +918,7 @@ func TestBudgetAdmission_ActionsAreDistinguishable(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
service.RunSchedulerTick(svc, ctx)
@@ -47,7 +47,7 @@ func TestAdmitRun_MetricBlockedByLimit(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -78,7 +78,7 @@ func TestAdmitRun_MetricBlockedPricingDegraded(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -104,7 +104,7 @@ func TestAdmitRun_MetricDeferredEvaluatorFault(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
insertTestTask(t, svc, "task-metric-deferred-fault", "ws-1")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-metric-deferred-fault"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -126,7 +126,7 @@ func TestAdmitRun_MetricBlockedAbsentEvaluator(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -156,7 +156,7 @@ func TestAdmitRun_MetricDeferredWorkspaceLookup(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -192,7 +192,7 @@ func TestProcessRun_AgentRowDeleted_Cancels(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -239,7 +239,7 @@ func TestAdmitRun_MetricCancelledNoWorkspace(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -271,7 +271,7 @@ func TestAdmitRun_MetricCancelledStaleDeferral(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -302,7 +302,7 @@ func TestAdmitRun_MetricAdmittedDefault(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -335,7 +335,7 @@ func TestAdmitRun_MetricAdmittedDegradedWindow_ViaDefault(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -369,7 +369,7 @@ func TestAdmitRun_MetricAdmittedDegradedWindow_ViaStoredPolicyBypassingGate5(t *
t.Fatalf("create agent: %v", err)
}
insertTestTask(t, svc, "task-metric-degraded-attended", "ws-1")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-metric-degraded-attended"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -54,7 +54,7 @@ func (s *Service) SetupChannel(ctx context.Context, channel *models.Channel) err
// the runner row is keyed against an empty step_id; the office
// repo's runner projection still resolves it because it joins
// step_id = task.workflow_step_id (also empty).
if err := s.repo.UpdateTaskAssignee(ctx, taskID, channel.AgentProfileID); err != nil {
if _, err := s.repo.UpdateTaskAssignee(ctx, taskID, channel.AgentProfileID); err != nil {
_ = s.repo.DeleteChannel(ctx, channel.ID)
return fmt.Errorf("set channel task assignee: %w", err)
}
@@ -26,7 +26,7 @@ func TestLoadContinuationSummary_AgentScope_RoundTripsThroughRealWriterAndReader
createTestAgent(t, svc, "ws-1", "agent-scope-a")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "agent-scope-a", service.RunReasonTaskAssigned, "{}", "continuation-agent-scope",
); err != nil {
t.Fatalf("queue run: %v", err)
@@ -147,7 +147,7 @@ func TestLoadContinuationSummary_ScopeSurvivesCoalesceAfterClaim(t *testing.T) {
createTestAgent(t, svc, "ws-1", "agent-scope-coalesce")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "agent-scope-coalesce", service.RunReasonTaskAssigned, "{}", "continuation-scope-coalesce",
); err != nil {
t.Fatalf("queue run: %v", err)
@@ -21,6 +21,8 @@ import (
"github.com/kandev/kandev/internal/office/repository/sqlite"
"github.com/kandev/kandev/internal/office/shared"
"github.com/kandev/kandev/internal/runs/commentkeys"
"github.com/kandev/kandev/internal/runs/dedupkeys"
runsservice "github.com/kandev/kandev/internal/runs/service"
"github.com/kandev/kandev/internal/workflow/engine"
)
@@ -107,12 +109,16 @@ type TaskMovedData struct {
SessionID string `json:"session_id"`
}
// TaskUpdatedData represents the payload of a task.updated event.
// TaskUpdatedData represents the payload of a task.created / task.updated
// event. AssignmentGeneration is carried on the extra map by
// publishTaskEventWithExtra's caller at task-creation time (never re-read);
// nil means the publishing event predates this field or is not a creation.
type TaskUpdatedData struct {
TaskID string `json:"task_id"`
WorkspaceID string `json:"workspace_id"`
AssigneeAgentProfileID string `json:"assignee_agent_profile_id"`
Title string `json:"title"`
AssignmentGeneration *int64 `json:"assignment_generation"`
}
// CommentPostedData represents a comment event payload.
@@ -994,22 +1000,39 @@ func (s *Service) handleTaskCreated(ctx context.Context, event *bus.Event) error
if err != nil {
return nil
}
return s.queueTaskAssignedRun(ctx, data.TaskID, data.AssigneeAgentProfileID, true)
return s.queueTaskAssignedRun(ctx, data.TaskID, data.AssigneeAgentProfileID, data.AssignmentGeneration, true)
}
// handleTaskUpdated fires a task_assigned run when an agent is assigned.
// handleTaskUpdated fires a task_assigned run when an agent is assigned. No
// production update path can put a new agent into the runner seat (see
// office/repository/sqlite/tasks.go's UpdateTaskAssignee vs the four inert
// syncRunnerInTx writers), so this stays subscribed as a redelivery and
// defensive path rather than a live assignment occurrence.
func (s *Service) handleTaskUpdated(ctx context.Context, event *bus.Event) error {
data, err := decodeEventData[TaskUpdatedData](event)
if err != nil {
return nil
}
return s.queueTaskAssignedRun(ctx, data.TaskID, data.AssigneeAgentProfileID, false)
return s.queueTaskAssignedRun(ctx, data.TaskID, data.AssigneeAgentProfileID, data.AssignmentGeneration, false)
}
// queueTaskAssignedRun fires a task_assigned run for the given occurrence.
//
// When fallbackToStoredRunner is set and the event carried no assignee, the
// agent is recovered through a fresh post-commit GetTaskExecutionFields read
// (task.created's payload can omit it) while the generation, if any, still
// describes the payload's own occurrence — not necessarily this freshly-read
// agent's. Task event publication is asynchronous (a per-task FIFO drainer),
// so a reassignment can commit between the event being built and this
// handler running, and the two halves would then name different occurrences.
// Forcing keyless whenever the fallback actually recovers an agent this way
// avoids mis-keying at the cost of a possible duplicate wake, which
// AC-003.2 already accepts.
func (s *Service) queueTaskAssignedRun(
ctx context.Context,
taskID string,
agentProfileID string,
assignmentGeneration *int64,
fallbackToStoredRunner bool,
) error {
if taskID == "" {
@@ -1025,15 +1048,23 @@ func (s *Service) queueTaskAssignedRun(
if fields == nil || !fields.IsFromOffice {
return nil
}
fellBackToStoredRunner := false
if agentProfileID == "" && fallbackToStoredRunner {
agentProfileID = fields.AssigneeAgentProfileID
fellBackToStoredRunner = agentProfileID != ""
}
if agentProfileID == "" {
return nil
}
payload := mustJSON(map[string]string{"task_id": taskID})
key := fmt.Sprintf("task_assigned:%s:%s", taskID, agentProfileID)
return s.QueueRun(ctx, agentProfileID, RunReasonTaskAssigned, payload, key)
var key string
if fellBackToStoredRunner || assignmentGeneration == nil {
runsservice.ReportKeylessEnqueue(RunReasonTaskAssigned, runsservice.KeylessCauseUnresolved, "event_missing_generation")
} else {
key = dedupkeys.AssignmentKey(taskID, agentProfileID, *assignmentGeneration)
}
_, err = s.QueueRun(ctx, agentProfileID, RunReasonTaskAssigned, payload, key)
return err
}
// handleTaskMoved keeps the legacy named-step activity fallback and queues
@@ -1120,7 +1151,9 @@ func (s *Service) resolveAndWakeIfUnblocked(ctx context.Context, blockedTaskID,
if err != nil {
return err
}
blockerIDs := make([]string, 0, len(blockers))
for _, b := range blockers {
blockerIDs = append(blockerIDs, b.BlockerTaskID)
if b.BlockerTaskID == resolvedBlockerID {
continue
}
@@ -1129,7 +1162,10 @@ func (s *Service) resolveAndWakeIfUnblocked(ctx context.Context, blockedTaskID,
return err // still blocked
}
}
key := fmt.Sprintf("blockers_resolved:%s", blockedTaskID)
if len(blockerIDs) == 0 {
return nil
}
key := fmt.Sprintf("blockers_resolved:%s:%s", blockedTaskID, dedupkeys.BlockerDigest(blockerIDs))
return s.dispatchEngineTrigger(ctx, blockedTaskID, engine.TriggerOnBlockerResolved,
engine.OnBlockerResolvedPayload{
ResolvedBlockerIDs: []string{resolvedBlockerID},
@@ -39,7 +39,7 @@ func TestHandleAgentCompleted_WarnsWhenReviewDecisionMissing(t *testing.T) {
taskID := createOfficeTask(t, svc, "ws-1", "reviewer-1")
payload := `{"task_id":"` + taskID + `","stage_type":"review","workflow_step_id":"step-1"}`
if err := svc.QueueRun(ctx, "reviewer-1", service.RunReasonTaskAssigned, payload, "review-no-decision"); err != nil {
if _, err := svc.QueueRun(ctx, "reviewer-1", service.RunReasonTaskAssigned, payload, "review-no-decision"); err != nil {
t.Fatalf("queue run: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -91,7 +91,7 @@ func TestHandleAgentCompleted_WarnsOnLegacyReviewStartedReason(t *testing.T) {
taskID := createOfficeTask(t, svc, "ws-1", "reviewer-1")
payload := `{"task_id":"` + taskID + `","workflow_step_id":"step-1","agent_profile_id":"reviewer-1"}`
if err := svc.QueueRun(ctx, "reviewer-1", "review_started", payload, "review-started-no-decision"); err != nil {
if _, err := svc.QueueRun(ctx, "reviewer-1", "review_started", payload, "review-started-no-decision"); err != nil {
t.Fatalf("queue run: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -139,7 +139,7 @@ func TestHandleAgentCompleted_WarnsOnStageIDKeyedPayload(t *testing.T) {
taskID := createOfficeTask(t, svc, "ws-1", "reviewer-1")
payload := `{"task_id":"` + taskID + `","stage_type":"approval","stage_id":"step-1"}`
if err := svc.QueueRun(ctx, "reviewer-1", service.RunReasonTaskAssigned, payload, "approval-stage-id-no-decision"); err != nil {
if _, err := svc.QueueRun(ctx, "reviewer-1", service.RunReasonTaskAssigned, payload, "approval-stage-id-no-decision"); err != nil {
t.Fatalf("queue run: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -179,7 +179,7 @@ func TestHandleAgentCompleted_WarnsOnLegacyApprovalStartedReason(t *testing.T) {
createTestAgent(t, svc, "ws-1", "approver-1")
taskID := createOfficeTask(t, svc, "ws-1", "approver-1")
payload := `{"task_id":"` + taskID + `","workflow_step_id":"step-approval","agent_profile_id":"approver-1"}`
if err := svc.QueueRun(ctx, "approver-1", "approval_started", payload, "approval-started-no-decision"); err != nil {
if _, err := svc.QueueRun(ctx, "approver-1", "approval_started", payload, "approval-started-no-decision"); err != nil {
t.Fatalf("queue run: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -219,7 +219,7 @@ func TestHandleAgentCompleted_WarnsOnTaskReviewRequested(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO workflow_steps (id, stage_type) VALUES (?, ?)`, "step-review-requested", "approval")
payload := `{"task_id":"` + taskID + `","role":"approver"}`
if err := svc.QueueRun(ctx, "approver-1", service.RunReasonTaskReviewRequested, payload, "task-review-requested-no-decision"); err != nil {
if _, err := svc.QueueRun(ctx, "approver-1", service.RunReasonTaskReviewRequested, payload, "task-review-requested-no-decision"); err != nil {
t.Fatalf("queue run: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -259,7 +259,7 @@ func TestHandleAgentCompleted_UsesAuthoritativeStageTypeWhenPayloadOmitsIt(t *te
svc.ExecSQL(t, `INSERT INTO workflow_steps (id, stage_type) VALUES (?, ?)`, "step-authoritative-decision", "approval")
payload := `{"task_id":"` + taskID + `","workflow_step_id":"step-authoritative-decision"}`
if err := svc.QueueRun(ctx, "approver-1", service.RunReasonTaskAssigned, payload, "authoritative-decision-no-decision"); err != nil {
if _, err := svc.QueueRun(ctx, "approver-1", service.RunReasonTaskAssigned, payload, "authoritative-decision-no-decision"); err != nil {
t.Fatalf("queue run: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -301,7 +301,7 @@ func TestHandleAgentCompleted_NoWarnWhenReviewDecisionRecorded(t *testing.T) {
taskID := createOfficeTask(t, svc, "ws-1", "reviewer-1")
payload := `{"task_id":"` + taskID + `","stage_type":"review","workflow_step_id":"step-1"}`
if err := svc.QueueRun(ctx, "reviewer-1", service.RunReasonTaskAssigned, payload, "review-with-decision"); err != nil {
if _, err := svc.QueueRun(ctx, "reviewer-1", service.RunReasonTaskAssigned, payload, "review-with-decision"); err != nil {
t.Fatalf("queue run: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -347,7 +347,7 @@ func TestHandleAgentCompleted_NoWarnForWorkStage(t *testing.T) {
taskID := createOfficeTask(t, svc, "ws-1", "builder-1")
payload := `{"task_id":"` + taskID + `","stage_type":"work","workflow_step_id":"step-1"}`
if err := svc.QueueRun(ctx, "builder-1", service.RunReasonTaskAssigned, payload, "work-stage"); err != nil {
if _, err := svc.QueueRun(ctx, "builder-1", service.RunReasonTaskAssigned, payload, "work-stage"); err != nil {
t.Fatalf("queue run: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -322,7 +322,7 @@ func queueTaskAssignedRunForAgentFailedTests(
) *models.Run {
t.Helper()
ctx := context.Background()
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, agentID, service.RunReasonTaskAssigned, `{"task_id":"`+taskID+`"}`, "",
); err != nil {
t.Fatalf("queue run: %v", err)
@@ -565,7 +565,7 @@ func TestEngineDispatcher_PathBEscalation_DoesNotFireAgentErrorTrigger(t *testin
}
createTestAgent(t, svc, "ws-1", "worker-pathb")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-pathb", service.RunReasonTaskAssigned, `{"task_id":"t1"}`, "",
); err != nil {
t.Fatalf("queue: %v", err)
@@ -70,7 +70,7 @@ func TestHandleAgentCompleted_RecordsFinalAgentMessageAsOutputSummary(t *testing
createTestAgent(t, svc, "ws-1", "worker-1")
taskID := createOfficeTask(t, svc, "ws-1", "worker-1")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "run-output-init",
); err != nil {
@@ -127,7 +127,7 @@ func TestHandleAgentCompleted_TruncatesOutputSummaryAt500Chars(t *testing.T) {
createTestAgent(t, svc, "ws-1", "worker-1")
taskID := createOfficeTask(t, svc, "ws-1", "worker-1")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "run-output-truncate",
); err != nil {
@@ -175,7 +175,7 @@ func TestHandleAgentCompleted_NoAgentMessageStaysBestEffort(t *testing.T) {
createTestAgent(t, svc, "ws-1", "worker-1")
taskID := createOfficeTask(t, svc, "ws-1", "worker-1")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "run-output-none",
); err != nil {
@@ -216,7 +216,7 @@ func TestHandleTasklessAgentCompleted_RecordsOutputSummaryAndKeepsContinuationSu
createTestAgent(t, svc, "ws-1", "worker-taskless")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-taskless", service.RunReasonTaskAssigned, "{}", "run-output-taskless",
); err != nil {
t.Fatalf("queue run: %v", err)
@@ -268,7 +268,7 @@ func TestHandleAgentCompleted_LastAgentMessageWinsOverLaterUserMessage(t *testin
createTestAgent(t, svc, "ws-1", "worker-1")
taskID := createOfficeTask(t, svc, "ws-1", "worker-1")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "run-output-ordering",
); err != nil {
@@ -318,7 +318,7 @@ func TestHandleAgentCompleted_SecondRunOnReusedSessionStaysEmpty(t *testing.T) {
// Run 1: queue, claim, produce an agent message, complete. The
// session (sess-1) is reused for run 2 below, mirroring how office
// task-bound sessions persist across turns.
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "run-output-reuse-1",
); err != nil {
@@ -359,7 +359,7 @@ func TestHandleAgentCompleted_SecondRunOnReusedSessionStaysEmpty(t *testing.T) {
// Run 2: reuses sess-1 but produces no new agent message before
// completing (e.g. a turn that only made tool calls, or was cut
// short). No new task_session_messages row is inserted here.
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskComment,
`{"task_id":"`+taskID+`"}`, "run-output-reuse-2",
); err != nil {
@@ -390,7 +390,7 @@ func TestHandleAgentCompleted_DoesNotClearExistingSummaryWithoutMessage(t *testi
ctx := context.Background()
createTestAgent(t, svc, "ws-1", "worker-1")
taskID := createOfficeTask(t, svc, "ws-1", "worker-1")
if err := svc.QueueRun(ctx, "worker-1", service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, "worker-1", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "run-output-preserve"); err != nil {
t.Fatalf("queue run: %v", err)
}
@@ -423,7 +423,7 @@ func TestHandleAgentCompleted_UsesTurnScopedMessageForSharedSession(t *testing.T
ctx := context.Background()
createTestAgent(t, svc, "ws-1", "worker-1")
taskID := createOfficeTask(t, svc, "ws-1", "worker-1")
if err := svc.QueueRun(ctx, "worker-1", service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, "worker-1", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "run-output-turn"); err != nil {
t.Fatalf("queue run: %v", err)
}
@@ -18,6 +18,7 @@ import (
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/service"
"github.com/kandev/kandev/internal/office/shared"
"github.com/kandev/kandev/internal/runs/dedupkeys"
"github.com/kandev/kandev/internal/workflow/engine"
)
@@ -42,7 +43,8 @@ func (d *queueRunDispatcher) HandleTrigger(
case engine.TriggerOnComment:
p, _ := payload.(engine.OnCommentPayload)
body, _ := json.Marshal(map[string]string{"task_id": taskID, "comment_id": p.CommentID})
return d.svc.QueueRun(ctx, assignee, service.RunReasonTaskComment, string(body), opID)
_, err := d.svc.QueueRun(ctx, assignee, service.RunReasonTaskComment, string(body), opID)
return err
case engine.TriggerOnBlockerResolved:
p, _ := payload.(engine.OnBlockerResolvedPayload)
var resolved string
@@ -50,10 +52,12 @@ func (d *queueRunDispatcher) HandleTrigger(
resolved = p.ResolvedBlockerIDs[0]
}
body, _ := json.Marshal(map[string]string{"task_id": taskID, "resolved_blocker_id": resolved})
return d.svc.QueueRun(ctx, assignee, service.RunReasonTaskBlockersResolved, string(body), opID)
_, err := d.svc.QueueRun(ctx, assignee, service.RunReasonTaskBlockersResolved, string(body), opID)
return err
case engine.TriggerOnChildrenCompleted:
body, _ := json.Marshal(map[string]string{"task_id": taskID})
return d.svc.QueueRun(ctx, assignee, service.RunReasonTaskChildrenCompleted, string(body), opID)
_, err := d.svc.QueueRun(ctx, assignee, service.RunReasonTaskChildrenCompleted, string(body), opID)
return err
case engine.TriggerOnApprovalResolved:
p, _ := payload.(engine.OnApprovalResolvedPayload)
body, _ := json.Marshal(map[string]string{
@@ -61,7 +65,8 @@ func (d *queueRunDispatcher) HandleTrigger(
"status": p.Status,
"decision_note": p.Note,
})
return d.svc.QueueRun(ctx, assignee, service.RunReasonApprovalResolved, string(body), opID)
_, err := d.svc.QueueRun(ctx, assignee, service.RunReasonApprovalResolved, string(body), opID)
return err
}
return nil
}
@@ -289,6 +294,64 @@ func TestTaskCreated_WakesAssigneeFromStoredRunner(t *testing.T) {
t.Fatalf("expected task_assigned run for worker-created, got %#v", runs)
}
func TestTaskCreated_WithAssignmentIdentity_UsesGenerationKey(t *testing.T) {
svc, eb := newTestServiceWithBus(t)
ctx := context.Background()
createTestAgent(t, svc, "ws-1", "worker-created-keyed")
insertTestTask(t, svc, "task-created-keyed", "ws-1")
svc.ExecSQL(t, `UPDATE tasks SET project_id = 'office-project', assignment_generation = 1 WHERE id = ?`, "task-created-keyed")
setTestTaskAssignee(t, svc, "task-created-keyed", "worker-created-keyed")
event := bus.NewEvent(events.TaskCreated, "task-service", map[string]any{
"task_id": "task-created-keyed",
"assignee_agent_profile_id": "worker-created-keyed",
"assignment_generation": int64(1),
})
if err := eb.Publish(ctx, events.TaskCreated, event); err != nil {
t.Fatalf("publish task created event: %v", err)
}
runs, err := svc.ListRuns(ctx, "ws-1")
if err != nil {
t.Fatalf("list creation runs: %v", err)
}
wantKey := dedupkeys.AssignmentKey("task-created-keyed", "worker-created-keyed", 1)
var creationRunCount int
for _, run := range runs {
if run.AgentProfileID != "worker-created-keyed" || run.Reason != service.RunReasonTaskAssigned {
continue
}
creationRunCount++
if run.IdempotencyKey == nil || *run.IdempotencyKey != wantKey {
t.Fatalf("creation run key = %v, want %q", run.IdempotencyKey, wantKey)
}
}
if creationRunCount != 1 {
t.Fatalf("creation event queued %d runs, want 1", creationRunCount)
}
// A replay after the first run is claimed must still use the same durable
// key and avoid inserting a second run.
svc.ExecSQL(t, `UPDATE runs SET status = 'completed' WHERE reason = ? AND idempotency_key = ?`, service.RunReasonTaskAssigned, wantKey)
if err := eb.Publish(ctx, events.TaskCreated, event); err != nil {
t.Fatalf("replay task created event: %v", err)
}
allRuns, err := svc.ListRuns(ctx, "ws-1")
if err != nil {
t.Fatalf("list replay runs: %v", err)
}
var totalTaskAssigned int
for _, run := range allRuns {
if run.AgentProfileID == "worker-created-keyed" && run.Reason == service.RunReasonTaskAssigned {
totalTaskAssigned++
}
}
if totalTaskAssigned != 1 {
t.Fatalf("replayed creation event queued %d runs, want 1", totalTaskAssigned)
}
}
func TestTaskCreated_NoopWhenNoStoredRunner(t *testing.T) {
svc, eb := newTestServiceWithBus(t)
ctx := context.Background()
@@ -488,6 +488,9 @@ func (s *Service) autoPauseAgent(
return nil
}
// requeueRunForTask is a manual resume: the failed run's own id is the
// occurrence identity (falling back to the task id if it is somehow empty),
// so a duplicate "Mark fixed" click dedupes instead of double-queuing.
func (s *Service) requeueRunForTask(
ctx context.Context, agentID, taskID, failedRunID string,
) error {
@@ -497,7 +500,8 @@ func (s *Service) requeueRunForTask(
identity = taskID
}
key := fmt.Sprintf("%s:%s:%s", RunReasonManualResumeAfterFailure, agentID, identity)
return s.QueueRun(ctx, agentID, RunReasonManualResumeAfterFailure, payload, key)
_, err := s.QueueRun(ctx, agentID, RunReasonManualResumeAfterFailure, payload, key)
return err
}
func (s *Service) publishRunFailed(
@@ -26,7 +26,7 @@ func queueAndReadRun(
ctx := context.Background()
payload := mustMarshalJSON(map[string]string{"task_id": taskID})
idem := agentID + ":" + taskID
if err := svc.QueueRun(ctx, agentID, service.RunReasonTaskAssigned, payload, idem); err != nil {
if _, err := svc.QueueRun(ctx, agentID, service.RunReasonTaskAssigned, payload, idem); err != nil {
t.Fatalf("queue run: %v", err)
}
rows, err := svc.ListRuns(ctx, "ws-1")
@@ -255,7 +255,7 @@ func TestMarkAgentPausedFixed_RecoversAgent(t *testing.T) {
// Proves the guardAgentStatus rejection is gone: QueueRun must
// succeed now that the agent is actually idle.
if err := svc.QueueRun(ctx, "agent-recover", service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, "agent-recover", service.RunReasonTaskAssigned,
mustMarshalJSON(map[string]string{"task_id": "agent-recover-task-a"}),
"agent-recover:post-fix"); err != nil {
t.Fatalf("expected QueueRun to succeed after mark fixed, got: %v", err)
@@ -227,7 +227,12 @@ func (s *Service) queueCEOAgentError(
"run_id": run.ID,
"error": errMsg,
})
_ = s.QueueRun(ctx, ceos[0].ID, RunReasonAgentError, payload, "")
// The failed run's own id makes this occurrence identity: one escalation
// per failed run per CEO, but a later run by the same agent that also
// fails escalates again instead of being silently swallowed by a
// permanently-unique-per-pair key.
key := fmt.Sprintf("agent_error:%s:%s", run.ID, ceos[0].ID)
_, _ = s.QueueRun(ctx, ceos[0].ID, RunReasonAgentError, payload, key)
}
// retryDelayWithJitter returns the base delay for a given retry index
@@ -36,7 +36,7 @@ func TestHandleRunFailure_CEOOwnFailure_DoesNotSelfEscalate(t *testing.T) {
}
for i := 0; i < 3; i++ {
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, ceo.ID, service.RunReasonAgentError, `{}`, "",
); err != nil {
t.Fatalf("queue ceo run %d: %v", i, err)
@@ -226,7 +226,7 @@ func TestHandleRunFailure_RateLimitParsed_UsesResetTime(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k1"); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k1"); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -275,7 +275,7 @@ func TestHandleRunFailure_RateLimitNoParseable_UsesBackoff(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k2"); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k2"); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
@@ -325,7 +325,7 @@ func TestHandleRunFailure_NonRateLimit_UsesBackoff(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k3"); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k3"); err != nil {
t.Fatalf("queue: %v", err)
}
run, err := svc.ClaimNextRun(ctx)
+16 -14
View File
@@ -85,18 +85,17 @@ const IdempotencyWindowHours = 24
func (s *Service) QueueRun(
ctx context.Context,
agentInstanceID, reason, payload, idempotencyKey string,
) error {
) (runsservice.QueueOutcome, error) {
if err := s.guardAgentStatus(ctx, agentInstanceID); err != nil {
return err
return runsservice.QueueOutcomeNone, err
}
if s.runsService != nil {
_, err := s.runsService.QueueRun(ctx, runsservice.QueueRunRequest{
return s.runsService.QueueRun(ctx, runsservice.QueueRunRequest{
Reason: reason,
IdempotencyKey: idempotencyKey,
Payload: payloadWithAgent(payload, agentInstanceID),
})
return err
}
return s.queueRunInline(ctx, agentInstanceID, reason, payload, idempotencyKey)
}
@@ -107,28 +106,26 @@ func (s *Service) QueueRun(
func (s *Service) queueRunInline(
ctx context.Context,
agentInstanceID, reason, payload, idempotencyKey string,
) error {
) (runsservice.QueueOutcome, error) {
if idempotencyKey != "" {
dup, err := s.repo.CheckIdempotencyKey(ctx, idempotencyKey, IdempotencyWindowHours)
if err != nil {
return fmt.Errorf("idempotency check: %w", err)
return runsservice.QueueOutcomeNone, fmt.Errorf("idempotency check: %w", err)
}
if dup {
s.logger.Debug("run skipped (idempotent)",
zap.String("key", idempotencyKey))
return nil
return runsservice.ReportWindowedDedup(runsservice.QueueSourceRuns, reason, idempotencyKey), nil
}
}
coalesced, err := s.repo.CoalesceRun(ctx, agentInstanceID, reason, CoalesceWindowSeconds, payload)
if err != nil {
return fmt.Errorf("coalesce check: %w", err)
return runsservice.QueueOutcomeNone, fmt.Errorf("coalesce check: %w", err)
}
if coalesced {
s.logger.Debug("run coalesced",
zap.String("agent", agentInstanceID),
zap.String("reason", reason))
return nil
return runsservice.QueueOutcomeCoalesced, nil
}
var idemKeyPtr *string
@@ -145,8 +142,13 @@ func (s *Service) queueRunInline(
IdempotencyKey: idemKeyPtr,
RequestedAt: time.Now().UTC(),
}
if err := s.repo.CreateRun(ctx, req); err != nil {
return fmt.Errorf("enqueue run: %w", err)
insertErr := s.repo.CreateRun(ctx, req)
outcome, err := runsservice.ReportInsertResult(runsservice.QueueSourceRuns, reason, idempotencyKey, agentInstanceID, insertErr)
if err != nil {
return runsservice.QueueOutcomeNone, fmt.Errorf("enqueue run: %w", err)
}
if outcome == runsservice.QueueOutcomeDeduped {
return outcome, nil
}
s.logger.Info("run queued",
@@ -155,7 +157,7 @@ func (s *Service) queueRunInline(
zap.String("reason", reason))
s.publishRunQueued(ctx, req, idempotencyKey)
return nil
return runsservice.QueueOutcomeQueued, nil
}
// payloadWithAgent decodes the JSON payload string and adds the
@@ -30,7 +30,7 @@ func TestRunLifecycle_StepCompleteEventsEmitted(t *testing.T) {
// Queue + claim a run for the task so handleAgentTurnMessageSaved
// resolves a runID via GetClaimedRunByTaskID.
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "lifecycle-init",
); err != nil {
@@ -102,7 +102,7 @@ func TestRunLifecycle_StepCompleteEventsEmitted(t *testing.T) {
// no longer find a claimed run. To exercise the persistence path
// directly, we re-queue + claim once more and seed an activity
// row tagged with the run id.
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskComment,
`{"task_id":"`+taskID+`"}`, "lifecycle-second",
); err != nil {
@@ -131,7 +131,7 @@ func TestRunLifecycle_ErrorEventEmitted(t *testing.T) {
createTestAgent(t, svc, "ws-1", "worker-1")
taskID := createOfficeTask(t, svc, "ws-1", "worker-1")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "lifecycle-error",
); err != nil {
@@ -249,7 +249,7 @@ func TestQueueRun_PublishesOfficeRunQueued(t *testing.T) {
const idemKey = "task_comment:cm-123"
payload := `{"task_id":"` + taskID + `","comment_id":"cm-123"}`
if err := svc.QueueRun(ctx, "worker-1", service.RunReasonTaskComment, payload, idemKey); err != nil {
if _, err := svc.QueueRun(ctx, "worker-1", service.RunReasonTaskComment, payload, idemKey); err != nil {
t.Fatalf("queue run: %v", err)
}
@@ -293,7 +293,7 @@ func TestFinishRun_PublishesOfficeRunProcessed(t *testing.T) {
createTestAgent(t, svc, "ws-1", "worker-1")
taskID := createOfficeTask(t, svc, "ws-1", "worker-1")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskComment,
`{"task_id":"`+taskID+`","comment_id":"cm-1"}`, "task_comment:cm-1",
); err != nil {
@@ -353,7 +353,7 @@ func TestFinishRun_PublishesOfficeRunProcessedForSourceCommentTask(t *testing.T)
sourceTaskID := "source-task"
insertTestTask(t, svc, sourceTaskID, "ws-1")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskComment,
`{"task_id":"`+targetTaskID+`","source_task_id":"`+sourceTaskID+`","comment_id":"cm-source"}`,
"task_comment:cm-source:target",
@@ -406,7 +406,7 @@ func TestFailRun_PublishesOfficeRunProcessedFailed(t *testing.T) {
createTestAgent(t, svc, "ws-1", "worker-1")
taskID := createOfficeTask(t, svc, "ws-1", "worker-1")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-1", service.RunReasonTaskComment,
`{"task_id":"`+taskID+`","comment_id":"cm-2"}`, "task_comment:cm-2",
); err != nil {
@@ -2,12 +2,39 @@ package service_test
import (
"context"
"expvar"
"testing"
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/service"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// windowedDedupCounterValue reads the current value of one
// office_run_dedup_total{reason,kind=windowed,queue=runs} label, or 0 if the
// label has never been reported. Callers snapshot before/after and assert
// the exact delta.
func windowedDedupCounterValue(t *testing.T, reason string) int64 {
t.Helper()
v := expvar.Get("office_run_dedup_total")
if v == nil {
t.Fatalf("expvar map office_run_dedup_total not registered")
}
m, ok := v.(*expvar.Map)
if !ok {
t.Fatalf("office_run_dedup_total is not a *expvar.Map")
}
iv := m.Get("reason=" + reason + ";kind=windowed;queue=runs")
if iv == nil {
return 0
}
i, ok := iv.(*expvar.Int)
if !ok {
t.Fatalf("counter value for reason=%s;kind=windowed;queue=runs is not *expvar.Int", reason)
}
return i.Value()
}
func TestQueueRun_Basic(t *testing.T) {
svc := newTestService(t)
ctx := context.Background()
@@ -17,7 +44,7 @@ func TestQueueRun_Basic(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, "key-1")
_, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, "key-1")
if err != nil {
t.Fatalf("queue run: %v", err)
}
@@ -47,11 +74,11 @@ func TestQueueRun_Idempotency(t *testing.T) {
}
key := "idem-key-1"
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", key); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", key); err != nil {
t.Fatalf("first enqueue: %v", err)
}
// Second enqueue with same key should be silently dropped.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", key); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", key); err != nil {
t.Fatalf("second enqueue: %v", err)
}
@@ -61,6 +88,46 @@ func TestQueueRun_Idempotency(t *testing.T) {
}
}
// TestQueueRun_Idempotency_ReportsWindowedDedupOutcome closes a phantom-green
// gap: TestQueueRun_Idempotency above only asserted the resulting row count,
// so deleting queueRunInline's ReportWindowedDedup call would leave it green
// while AC-OFFICE-RUN-DEDUP-004.1's observability contract silently went
// unmet on this queue implementation. Asserts the outcome value and the
// office_run_dedup_total counter directly.
func TestQueueRun_Idempotency_ReportsWindowedDedupOutcome(t *testing.T) {
svc := newTestService(t)
ctx := context.Background()
agent := makeAgent("worker-1", models.AgentRoleWorker)
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
key := "idem-key-outcome"
first, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", key)
if err != nil {
t.Fatalf("first enqueue: %v", err)
}
if first != runsservice.QueueOutcomeQueued {
t.Fatalf("first outcome = %v, want QueueOutcomeQueued", first)
}
before := windowedDedupCounterValue(t, service.RunReasonTaskAssigned)
second, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", key)
if err != nil {
t.Fatalf("second enqueue: %v", err)
}
if second != runsservice.QueueOutcomeDeduped {
t.Fatalf("second outcome = %v, want QueueOutcomeDeduped", second)
}
after := windowedDedupCounterValue(t, service.RunReasonTaskAssigned)
if after != before+1 {
t.Fatalf("office_run_dedup_total{%s,windowed,runs} = %d, want %d", service.RunReasonTaskAssigned, after, before+1)
}
}
func TestQueueRun_SkipsPausedAgent(t *testing.T) {
svc := newTestService(t)
ctx := context.Background()
@@ -74,7 +141,7 @@ func TestQueueRun_SkipsPausedAgent(t *testing.T) {
t.Fatalf("pause agent: %v", err)
}
err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "")
_, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "")
if err == nil {
t.Fatal("expected error for paused agent")
}
@@ -97,7 +164,7 @@ func TestQueueRun_SkipsStoppedAgent(t *testing.T) {
t.Fatalf("stop agent: %v", err)
}
err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "")
_, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "")
if err == nil {
t.Fatal("expected error for stopped agent")
}
@@ -113,10 +180,10 @@ func TestQueueRun_Coalesce(t *testing.T) {
}
// Two runs with the same agent + reason within coalesce window should merge.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskComment, `{"task_id":"t1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskComment, `{"task_id":"t1"}`, ""); err != nil {
t.Fatalf("first: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskComment, `{"task_id":"t1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskComment, `{"task_id":"t1"}`, ""); err != nil {
t.Fatalf("second: %v", err)
}
@@ -42,7 +42,7 @@ func TestSchedulerTick_ContendedCheckoutRequeuesRun(t *testing.T) {
t.Fatalf("seed checkout: ok=%v err=%v", ok, err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-contended-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -35,7 +35,7 @@ func TestSchedulerTick_CheckoutErrorRetriesInsteadOfFalseFinish(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-checkout-db-error-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -55,7 +55,7 @@ func TestSchedulerTick_InactiveAgentRunDoesNotStealCheckout(t *testing.T) {
// Queue while idle (QueueRun itself refuses to queue for a paused
// agent), then go paused before the tick runs - reproducing the race
// where an agent is deactivated between queueing and processing.
if err := svc.QueueRun(ctx, inactive.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, inactive.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-inactive-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -125,7 +125,7 @@ func TestSchedulerTick_SameAgentPreCheckoutRunDoesNotStealOwnLiveCheckout(t *tes
// queued while idle, then the agent goes paused before the tick picks
// it up - reproducing the race where the holder is deactivated after
// a second run was already queued behind its own live run.
if err := svc.QueueRun(ctx, holder.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, holder.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-same-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -63,7 +63,7 @@ func TestSchedulerTick_AgentCompletedReleasesTaskCheckout(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-checkout-release-1', 'ws-1', 'Build API', 'Implement endpoint', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-checkout-release-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -151,7 +151,7 @@ func TestSchedulerTick_AgentFailedReleasesTaskCheckout(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-checkout-fail-1', 'ws-1', 'Build API', 'Implement endpoint', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-checkout-fail-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -228,7 +228,7 @@ func TestSchedulerTick_TasklessAgentCompletedStampsRuntime(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
claimed, err := svc.ClaimNextRun(ctx)
@@ -29,7 +29,7 @@ func TestCooldown_RecentFinish_GuardAllows(t *testing.T) {
}
// Queue a run.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -69,7 +69,7 @@ func TestCooldown_PastCooldown_ClaimedNormally(t *testing.T) {
}
// Queue a run.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -97,7 +97,7 @@ func TestRetry_FailedRun_RetriedWithBackoff(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -156,7 +156,7 @@ func TestRetry_FifthFailure_MarkedFailed(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -402,7 +402,7 @@ func TestIdleSkip_HeartbeatNoTasks_Skipped(t *testing.T) {
}
// Worker defaults to skip_idle_runs=true, no tasks assigned.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -457,7 +457,7 @@ func TestIdleSkip_HeartbeatWithActionableTasks_Proceeds(t *testing.T) {
// Assign an IN_PROGRESS task to this agent.
insertActionableTask(t, svc, "task-inprog-1", agent.ID, "IN_PROGRESS")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -499,7 +499,7 @@ func TestIdleSkip_HeartbeatSkipDisabled_Proceeds(t *testing.T) {
`UPDATE agent_profiles SET skip_idle_runs = 0 WHERE id = ?`, agent.ID)
// No tasks assigned.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -535,7 +535,7 @@ func TestIdleSkip_NonHeartbeatRun_NotSkipped(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, created_at, updated_at)
VALUES ('task-event-1', 'ws-1', 'Event Task', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-event-1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-event-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -569,7 +569,7 @@ func TestIdleSkip_CEODefaultFalse_NotSkipped(t *testing.T) {
}
// CEO has no tasks, but skip_idle_runs defaults to false.
if err := svc.QueueRun(ctx, ceo.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, ceo.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -109,7 +109,7 @@ func TestSchedulerIntegration_RoutingReceivesBuiltPromptAndEnv(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, priority, created_at, updated_at)
VALUES ('task-routing-1', 'ws-1', 'ROUTING_PROMPT_SENTINEL_TITLE',
'Implement endpoint', 'medium', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-routing-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -171,7 +171,7 @@ func TestSchedulerIntegration_SeatActionFlowsToPromptAndLaunch(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, workflow_step_id, title, description, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
"task-decision", "ws-1", "step-decision", "Decision task", "Review the change")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-decision"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -263,7 +263,7 @@ func TestSchedulerIntegration_NonSeatRunDoesNotReceiveDecisionSkill(t *testing.T
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, workflow_step_id, title, description, created_at, updated_at)
VALUES (?, ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
"task-non-seat", "ws-1", "step-non-seat", "Non-seat task", "Review the change")
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-non-seat"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -317,7 +317,7 @@ func TestSchedulerIntegration_RoutingFallThrough_FallsBackToLegacy(t *testing.T)
}
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-ft-1', 'ws-1', 'Fall-through Task', 'desc', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-ft-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -361,7 +361,7 @@ func TestSchedulerIntegration_RoutingParked_LeavesAgentIdle(t *testing.T) {
}
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-parked-1', 'ws-1', 'Parked Task', 'desc', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-parked-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -402,7 +402,7 @@ func TestSchedulerIntegration_RoutingDispatchError_LeavesAgentIdle(t *testing.T)
}
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-routing-err-1', 'ws-1', 'Routing Error Task', 'desc', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-routing-err-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -20,7 +20,7 @@ func TestSchedulerIntegration_TickProcessesRun(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
t.Fatalf("queue run: %v", err)
}
@@ -72,7 +72,7 @@ func TestSchedulerIntegration_CancelsRunForMovedWorkflowStep(t *testing.T) {
// The run was queued while the task was on step-old. A later workflow
// move must prevent the scheduler from launching that stale step.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-moved-step","workflow_step_id":"step-old"}`, ""); err != nil {
t.Fatalf("queue run: %v", err)
}
@@ -150,7 +150,7 @@ func TestSchedulerIntegration_ResolvesExecutorFromTaskProject(t *testing.T) {
VALUES ('task-project-exec', 'ws-1', ?, 'Project executor task', 'desc',
CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`, project.ID)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-project-exec"}`, ""); err != nil {
t.Fatalf("queue run: %v", err)
}
@@ -205,7 +205,7 @@ func TestSchedulerIntegration_PausedAgentSkipped(t *testing.T) {
}
// Queue while agent is active.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -244,10 +244,10 @@ func TestSchedulerIntegration_AtCapacityStaysQueued(t *testing.T) {
}
// Queue two runs.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, "k1"); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, "k1"); err != nil {
t.Fatalf("queue first: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskComment, `{"task_id":"t2"}`, "k2"); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskComment, `{"task_id":"t2"}`, "k2"); err != nil {
t.Fatalf("queue second: %v", err)
}
@@ -330,7 +330,7 @@ func TestSchedulerIntegration_PromptBuiltCorrectly(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, tt.reason, tt.payload, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, tt.reason, tt.payload, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -7,6 +7,7 @@ import (
"go.uber.org/zap"
"github.com/kandev/kandev/internal/common/logger"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// maxRecoveryPerTick caps the number of unstarted tasks recovered in one tick.
@@ -69,7 +70,8 @@ func (si *SchedulerIntegration) recoverUnstartedTasks(ctx context.Context, log *
zap.String("agent_profile_id", t.AssigneeAgentProfileID))
payload := mustJSON(map[string]string{"task_id": t.ID})
if err := si.svc.QueueRun(ctx, t.AssigneeAgentProfileID,
runsservice.ReportKeylessEnqueue(RunReasonTaskAssigned, runsservice.KeylessCauseByDesign, "")
if _, err := si.svc.QueueRun(ctx, t.AssigneeAgentProfileID,
RunReasonTaskAssigned, payload, ""); err != nil {
if ctx.Err() != nil {
return
@@ -64,7 +64,7 @@ func TestSchedulerOutcome_AgentInactive_WritesOutcome(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"t1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -106,7 +106,7 @@ func TestSchedulerOutcome_IdleSkipped_WritesOutcome(t *testing.T) {
}
// Worker defaults to skip_idle_runs=true, no tasks assigned.
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonHeartbeat, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -134,7 +134,7 @@ func TestSchedulerOutcome_TaskTreeHeld_WritesOutcome(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-tree-outcome"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -184,7 +184,7 @@ func TestSchedulerOutcome_CheckoutError_RetriesInsteadOfFinishing(t *testing.T)
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-checkout-error"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -250,7 +250,7 @@ func TestSchedulerOutcome_CheckoutUnavailable_RetriesInsteadOfFinishing(t *testi
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-checkout-unavail"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -286,7 +286,7 @@ func TestSchedulerOutcome_BudgetBlocked_WritesOutcome(t *testing.T) {
insertTestTask(t, svc, "task-budget-outcome", "ws-1")
insertTestCostEvent(t, svc, agent.ID, "task-budget-outcome", int64(600))
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-budget-outcome"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -312,7 +312,7 @@ func TestSchedulerOutcome_NoTaskStarter_FailsRun(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-no-launch"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -353,7 +353,7 @@ func TestSchedulerOutcome_Processed_WritesOutcome(t *testing.T) {
createTestAgent(t, svc, "ws-1", "worker-processed")
taskID := createOfficeTask(t, svc, "ws-1", "worker-processed")
if err := svc.QueueRun(
if _, err := svc.QueueRun(
ctx, "worker-processed", service.RunReasonTaskAssigned,
`{"task_id":"`+taskID+`"}`, "processed-outcome",
); err != nil {
@@ -30,7 +30,7 @@ func TestClaimNextRun_ClaimsQueued(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -96,7 +96,7 @@ func TestFinishAndFailRun(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k1"); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k1"); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -132,7 +132,7 @@ func TestFailRun_WritesFailedStatusAndNullOutcome(t *testing.T) {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k1"); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k1"); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -178,7 +178,7 @@ func TestTransitionRunTerminal_FirstWriterWinsOnStatusAndOutcome(t *testing.T) {
if err := svc.CreateAgentInstance(ctx, agent); err != nil {
t.Fatalf("create agent: %v", err)
}
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k1"); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, "{}", "k1"); err != nil {
t.Fatalf("queue: %v", err)
}
req, _ := svc.ClaimNextRun(ctx)
@@ -45,7 +45,7 @@ func TestSchedulerTick_TasklessRunFailsInsteadOfFinishing(t *testing.T) {
// Mirrors wakeup/dispatcher.go's createFreshRun: reason from the
// routine trigger, payload literally "{}" (no task_id).
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -196,7 +196,7 @@ func TestSchedulerTick_TaskBoundRunStillLaunches(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-wo35-1', 'ws-1', 'Build API', 'Implement endpoint', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-wo35-1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-wo35-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -256,7 +256,7 @@ func TestSchedulerTick_TasklessRunsDoNotAutoPauseAgent(t *testing.T) {
// mirrors 3 ticks of the pre-installed coordinator heartbeat routine.
const firesAtThreshold = 3
for i := 0; i < firesAtThreshold; i++ {
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue taskless run %d: %v", i, err)
}
service.RunSchedulerTick(svc, ctx)
@@ -281,7 +281,7 @@ func TestSchedulerTick_TasklessRunsDoNotAutoPauseAgent(t *testing.T) {
// failures: a task-bound run queued afterwards must still launch.
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-wo35-pause-1', 'ws-1', 'Build API', 'Implement endpoint', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-wo35-pause-1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-wo35-pause-1"}`, ""); err != nil {
t.Fatalf("queue task-bound run: %v", err)
}
service.RunSchedulerTick(svc, ctx)
@@ -330,7 +330,7 @@ func TestSchedulerTick_TasklessRunFailure_PublishesResolvableWorkspaceEvent(t *t
}
t.Cleanup(func() { _ = sub.Unsubscribe() })
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
service.RunSchedulerTick(svc, ctx)
@@ -391,7 +391,7 @@ func TestSchedulerTick_UnlaunchableRun_PublishesResolvableWorkspaceEvent(t *test
}
t.Cleanup(func() { _ = sub.Unsubscribe() })
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-wo35-unlaunchable"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -446,7 +446,7 @@ func TestSchedulerTick_RepeatTasklessFailures_OnlyFirstStaysInInbox(t *testing.T
const fires = 3
var firstRunID string
for i := 0; i < fires; i++ {
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, ""); err != nil {
t.Fatalf("queue taskless run %d: %v", i, err)
}
service.RunSchedulerTick(svc, ctx)
@@ -512,7 +512,7 @@ func TestSchedulerTick_RepeatTasklessFailures_StayVisiblePerRoutineScope(t *test
queueRoutineFailure := func(scope, idempotencyKey string) string {
t.Helper()
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, idempotencyKey); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonRoutineTrigger, `{}`, idempotencyKey); err != nil {
t.Fatalf("queue routine %s: %v", scope, err)
}
runs, err := svc.ListRuns(ctx, "ws-1")
@@ -0,0 +1,180 @@
package service_test
// This file proves the specific defect run-dedup-generation fixes: a
// task_assigned dedup key that used to be stable for the life of a
// (task, agent) pair (docs/specs/office/requirements/run-dedup-generation.md,
// AC-OFFICE-SCHEDULER-001.7). idx_run_idempotency has no time bound, so a
// stable key collided with the UNIQUE index forever, making a re-assignment
// of the same task to the same agent a silent, permanent no-op. These tests
// exercise the real bus -> handleTaskUpdated -> queueTaskAssignedRun path
// (not just the dedupkeys.AssignmentKey unit) and assert on the persisted
// idempotency_key, which none of the existing task_assigned tests do.
import (
"context"
"encoding/json"
"testing"
"github.com/kandev/kandev/internal/events"
"github.com/kandev/kandev/internal/events/bus"
"github.com/kandev/kandev/internal/office/service"
"github.com/kandev/kandev/internal/runs/dedupkeys"
)
func publishTaskAssigned(
t *testing.T, ctx context.Context, eb bus.EventBus, taskID, agentID string, generation *int64,
) {
t.Helper()
data := map[string]any{
"task_id": taskID,
"assignee_agent_profile_id": agentID,
}
if generation != nil {
data["assignment_generation"] = *generation
}
event := bus.NewEvent(events.TaskUpdated, "test", data)
if err := eb.Publish(ctx, events.TaskUpdated, event); err != nil {
t.Fatalf("publish task updated event: %v", err)
}
}
func gen(n int64) *int64 { return &n }
// taskAssignedRunsFor returns every task_assigned run queued for agentID on
// the given task, in insertion order (ListRuns has no ordering guarantee
// this test relies on beyond "all rows present").
func taskAssignedRunsFor(t *testing.T, svc *service.Service, wsID, taskID, agentID string) []string {
t.Helper()
runs, err := svc.ListRuns(context.Background(), wsID)
if err != nil {
t.Fatalf("list runs: %v", err)
}
var keys []string
for _, run := range runs {
if run.AgentProfileID != agentID || run.Reason != service.RunReasonTaskAssigned {
continue
}
var payload struct {
TaskID string `json:"task_id"`
}
if err := json.Unmarshal([]byte(run.Payload), &payload); err != nil {
t.Fatalf("decode run payload: %v", err)
}
if payload.TaskID != taskID {
continue
}
if run.IdempotencyKey == nil {
keys = append(keys, "")
continue
}
keys = append(keys, *run.IdempotencyKey)
}
return keys
}
func TestTaskAssigned_SameGenerationRedelivery_DedupesForever(t *testing.T) {
svc, eb := newTestServiceWithBus(t)
ctx := context.Background()
createTestAgent(t, svc, "ws-1", "worker-gen")
insertTestTask(t, svc, "task-gen-redeliver", "ws-1")
svc.ExecSQL(t, `UPDATE tasks SET project_id = 'office-project' WHERE id = ?`, "task-gen-redeliver")
publishTaskAssigned(t, ctx, eb, "task-gen-redeliver", "worker-gen", gen(0))
publishTaskAssigned(t, ctx, eb, "task-gen-redeliver", "worker-gen", gen(0))
keys := taskAssignedRunsFor(t, svc, "ws-1", "task-gen-redeliver", "worker-gen")
if len(keys) != 1 {
t.Fatalf("redelivering the same generation queued %d runs, want exactly 1 (deduped): keys=%#v", len(keys), keys)
}
want := dedupkeys.AssignmentKey("task-gen-redeliver", "worker-gen", 0)
if keys[0] != want {
t.Fatalf("idempotency key = %q, want %q", keys[0], want)
}
}
// TestTaskAssigned_GenerationBump_AvoidsPermanentCollision is the direct
// regression test for the reported defect: reassigning the same task to the
// same agent must NOT be a permanent no-op once the durable unique index
// already holds a row for an earlier generation's key. Unlike
// TestTaskAssigned_ReassignmentUsesAgentScopedIdempotency (which reassigns to
// a *different* agent and never sets assignment_generation, so it never
// exercised the keyed path at all), this drives the same agent through two
// generations to prove the key itself changed.
func TestTaskAssigned_GenerationBump_AvoidsPermanentCollision(t *testing.T) {
svc, eb := newTestServiceWithBus(t)
ctx := context.Background()
createTestAgent(t, svc, "ws-1", "worker-regen")
insertTestTask(t, svc, "task-regen", "ws-1")
svc.ExecSQL(t, `UPDATE tasks SET project_id = 'office-project' WHERE id = ?`, "task-regen")
// Occurrence 1: original assignment at generation 0. In production this
// row's idempotency_key (task_assigned:task-regen:worker-regen:0) is
// exactly the durable UNIQUE row that used to collide forever under the
// pre-fix key format (task_assigned:<task>:<agent>, no generation).
publishTaskAssigned(t, ctx, eb, "task-regen", "worker-regen", gen(0))
// The first run is claimed and finishes (agent did the work) long before
// the reassignment — CoalesceRun only merges into a still-'queued' row,
// so this ensures the second occurrence exercises the durable
// idx_run_idempotency INSERT path, not the 5-second coalesce window.
// This is exactly the "more than 24 hours later" gap in the bug report.
svc.ExecSQL(t, `UPDATE runs SET status = 'completed' WHERE status = 'queued'`)
// Simulate a real reassignment: UpdateTaskAssignee bumps the stored
// generation before the task.updated event carrying it is published.
svc.ExecSQL(t, `UPDATE tasks SET assignment_generation = 1 WHERE id = ?`, "task-regen")
publishTaskAssigned(t, ctx, eb, "task-regen", "worker-regen", gen(1))
keys := taskAssignedRunsFor(t, svc, "ws-1", "task-regen", "worker-regen")
if len(keys) != 2 {
t.Fatalf("reassigning the same agent at a new generation queued %d runs, want 2 (no false collision): keys=%#v", len(keys), keys)
}
wantGen0 := dedupkeys.AssignmentKey("task-regen", "worker-regen", 0)
wantGen1 := dedupkeys.AssignmentKey("task-regen", "worker-regen", 1)
matchesInOrder := keys[0] == wantGen0 && keys[1] == wantGen1
matchesReversed := keys[0] == wantGen1 && keys[1] == wantGen0
if !matchesInOrder && !matchesReversed {
t.Fatalf("keys = %#v, want %q and %q (one run per generation)", keys, wantGen0, wantGen1)
}
if keys[0] == keys[1] {
t.Fatalf("both runs share idempotency key %q — this is the reported defect: a reassignment collided with the durable unique index", keys[0])
}
// Redelivering generation 1 again — past the coalesce window and after
// the generation-1 run has itself been claimed — must still dedupe via
// the durable unique index: the fix must not turn every event into a
// fresh key, only a genuine generation change.
svc.ExecSQL(t, `UPDATE runs SET status = 'completed' WHERE status = 'queued'`)
publishTaskAssigned(t, ctx, eb, "task-regen", "worker-regen", gen(1))
keysAfterRedeliver := taskAssignedRunsFor(t, svc, "ws-1", "task-regen", "worker-regen")
if len(keysAfterRedeliver) != 2 {
t.Fatalf("redelivering generation 1 queued a 3rd run: keys=%#v", keysAfterRedeliver)
}
}
// TestTaskAssigned_MissingGeneration_EnqueuesKeyless proves SR-55's
// resolution: an event carrying no assignment_generation must never mint a
// key with a guessed or zero generation — it goes keyless (empty
// idempotency_key) so a possible duplicate wake is accepted (AC-003.2)
// instead of a mis-keyed row that could collide with an unrelated
// occurrence.
func TestTaskAssigned_MissingGeneration_EnqueuesKeyless(t *testing.T) {
svc, eb := newTestServiceWithBus(t)
ctx := context.Background()
createTestAgent(t, svc, "ws-1", "worker-nogen")
insertTestTask(t, svc, "task-nogen", "ws-1")
svc.ExecSQL(t, `UPDATE tasks SET project_id = 'office-project' WHERE id = ?`, "task-nogen")
publishTaskAssigned(t, ctx, eb, "task-nogen", "worker-nogen", nil)
keys := taskAssignedRunsFor(t, svc, "ws-1", "task-nogen", "worker-nogen")
if len(keys) != 1 {
t.Fatalf("got %d runs, want 1: keys=%#v", len(keys), keys)
}
if keys[0] != "" {
t.Fatalf("idempotency key = %q, want empty (keyless enqueue)", keys[0])
}
}
@@ -38,7 +38,10 @@ const participantTypeAgent = "agent"
// by tests and a couple of internal callers; the dashboard's
// permissioned variant is SetTaskAssigneeAsAgent.
func (s *Service) SetTaskAssignee(ctx context.Context, taskID, assigneeID string) error {
return s.repo.UpdateTaskAssignee(ctx, taskID, assigneeID)
// generation discarded — this caller does not carry it to the event bus;
// queueTaskAssignedRun falls through to keyless enqueue (AC-OFFICE-RUN-DEDUP-003).
_, err := s.repo.UpdateTaskAssignee(ctx, taskID, assigneeID)
return err
}
// SetTaskAssigneeAsAgent checks can_assign_tasks for the given caller
@@ -55,5 +58,8 @@ func (s *Service) SetTaskAssigneeAsAgent(ctx context.Context, callerAgentID, tas
return shared.ErrForbidden
}
}
return s.repo.UpdateTaskAssignee(ctx, taskID, assigneeID)
// generation discarded — this caller does not carry it to the event bus;
// queueTaskAssignedRun falls through to keyless enqueue (AC-OFFICE-RUN-DEDUP-003).
_, err := s.repo.UpdateTaskAssignee(ctx, taskID, assigneeID)
return err
}
@@ -99,7 +99,7 @@ func TestSchedulerTick_LaunchesAgent(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, priority, created_at, updated_at)
VALUES ('task-launch-1', 'ws-1', 'Build API', 'Implement endpoint', 'medium', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-launch-1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-launch-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -145,7 +145,7 @@ func TestSchedulerTick_LaunchIncludesRuntimeTokenEnv(t *testing.T) {
}
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-token-1', 'ws-1', 'Build API', 'Implement endpoint', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-token-1","session_id":"sess-token"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-token-1","session_id":"sess-token"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -192,7 +192,7 @@ func TestSchedulerTick_SnapshotsRunSkills(t *testing.T) {
}
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-skill-1', 'ws-1', 'Review API', 'Review endpoint', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-skill-1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-skill-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -239,7 +239,7 @@ func TestSchedulerTick_KeepsRunClaimedUntilAgentCompletes(t *testing.T) {
}
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-life-1', 'ws-1', 'Build API', 'Implement endpoint', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-life-1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-life-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -301,7 +301,7 @@ func TestSchedulerTick_AgentStoppedFinishesRun(t *testing.T) {
}
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, description, created_at, updated_at)
VALUES ('task-stop-1', 'ws-1', 'Stop handler test', 'desc', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-stop-1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-stop-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -386,7 +386,7 @@ func TestSchedulerTick_StartTaskError_TriggersRetry(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, created_at, updated_at)
VALUES ('task-fail-1', 'ws-1', 'Failing Task', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-fail-1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-fail-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -439,7 +439,7 @@ func TestSchedulerTick_NoTaskStarter_FailsRunLoudly(t *testing.T) {
svc.ExecSQL(t, `INSERT INTO tasks (id, workspace_id, title, created_at, updated_at)
VALUES ('task-noop-1', 'ws-1', 'NoOp Task', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-noop-1"}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned, `{"task_id":"task-noop-1"}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -494,7 +494,7 @@ func TestSchedulerTick_NoTaskStarter_UpdatesFailureAccounting(t *testing.T) {
for i := 0; i < threshold; i++ {
idempotencyKey := fmt.Sprintf("task-noop-accounting-%d", i)
if err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
if _, err := svc.QueueRun(ctx, agent.ID, service.RunReasonTaskAssigned,
`{"task_id":"task-noop-accounting"}`, idempotencyKey); err != nil {
t.Fatalf("queue failure %d: %v", i, err)
}
@@ -32,7 +32,7 @@ func TestIdleSkip_RoutineDispatchNoTasks_Skipped(t *testing.T) {
}
// Worker defaults to skip_idle_runs=true, no tasks assigned.
if err := svc.QueueRun(ctx, agent.ID, shared.RunReasonRoutineDispatchCron, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, shared.RunReasonRoutineDispatchCron, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -98,7 +98,7 @@ func TestIdleSkip_RoutineDispatch_CoordinatorNotSkipped(t *testing.T) {
t.Fatalf("CEO role should default to SkipIdleRuns=false")
}
if err := svc.QueueRun(ctx, agent.ID, shared.RunReasonRoutineDispatchCron, `{}`, ""); err != nil {
if _, err := svc.QueueRun(ctx, agent.ID, shared.RunReasonRoutineDispatchCron, `{}`, ""); err != nil {
t.Fatalf("queue: %v", err)
}
@@ -6,6 +6,7 @@ import (
"time"
"github.com/kandev/kandev/internal/office/models"
runsservice "github.com/kandev/kandev/internal/runs/service"
"github.com/kandev/kandev/internal/workflow/engine"
)
@@ -39,8 +40,11 @@ type AgentWriter interface {
// Implemented by the run feature (and transitionally by office/service.Service).
type RunQueuer interface {
// QueueRun enqueues a run for agentInstanceID with the given reason, payload,
// and optional idempotency key (empty string disables deduplication).
QueueRun(ctx context.Context, agentInstanceID, reason, payload, idempotencyKey string) error
// and optional idempotency key (empty string disables deduplication). The
// returned QueueOutcome reports what actually happened (queued / deduped /
// coalesced / none-on-error) so callers that need to distinguish a fresh
// insert from a no-op don't have to infer it from side effects.
QueueRun(ctx context.Context, agentInstanceID, reason, payload, idempotencyKey string) (runsservice.QueueOutcome, error)
}
// WorkflowEngineDispatcher routes typed office task events through the
@@ -22,6 +22,7 @@ import (
"github.com/kandev/kandev/internal/orchestrator/executor"
"github.com/kandev/kandev/internal/orchestrator/messagequeue"
"github.com/kandev/kandev/internal/orchestrator/watcher"
runsservice "github.com/kandev/kandev/internal/runs/service"
"github.com/kandev/kandev/internal/steptelemetry"
"github.com/kandev/kandev/internal/sysprompt"
"github.com/kandev/kandev/internal/task/models"
@@ -2179,13 +2180,16 @@ func (s *Service) queueOfficeAutoStartRun(ctx context.Context, task *models.Task
}
// officeAutoStartIdempotencyKey uses the immutable workflow-step transition
// row as the per-entry component. A legacy event without that field uses the
// task timestamp as a compatibility fallback until the event is republished.
// row as the per-entry component. A zero stepTransitionID means the
// per-occurrence identity is unavailable, so the enqueue goes keyless rather
// than falling back to a time-derived key that would never suppress a
// redelivery.
func officeAutoStartIdempotencyKey(task *models.Task, agentProfileID, stepID string, stepTransitionID int64) string {
entryID := strconv.FormatInt(stepTransitionID, 10)
if stepTransitionID == 0 {
entryID = "legacy:" + task.UpdatedAt.UTC().Format(time.RFC3339Nano)
runsservice.ReportKeylessEnqueue(officeAutoStartRunReason, runsservice.KeylessCauseUnresolved, "zero_step_transition")
return ""
}
entryID := strconv.FormatInt(stepTransitionID, 10)
return fmt.Sprintf("%s:%s:%s:%s:%s",
officeAutoStartRunReason, task.ID, agentProfileID, stepID, entryID)
}
@@ -36,6 +36,20 @@ func TestOfficeAutoStartIdempotencyKey(t *testing.T) {
}
}
// TestOfficeAutoStartIdempotencyKey_ZeroStepTransitionGoesKeyless pins that a
// zero StepTransitionID (no per-occurrence identity available) enqueues with
// no key at all, rather than falling back to a time-derived key that would
// never suppress a genuine redelivery.
func TestOfficeAutoStartIdempotencyKey_ZeroStepTransitionGoesKeyless(t *testing.T) {
task := &models.Task{ID: "t1", UpdatedAt: time.Now().UTC(), WorkflowStepTransitionID: 0}
key := officeAutoStartIdempotencyKey(task, "agent-1", "step2", task.WorkflowStepTransitionID)
if key != "" {
t.Errorf("key = %q, want empty (keyless) for zero StepTransitionID", key)
}
}
// TestAutoStartOfficeTaskLogsQueuedOutcomeAtInfo pins that a real insert
// (QueueOutcomeQueued) is logged as an info-level "queued" message, and the
// log is only emitted after the queue attempt resolves — not asserted
@@ -0,0 +1,38 @@
// Package dedupkeys centralizes the shared dedup-key builders that more than
// one office producer must derive identically for the same occurrence
// (docs/specs/office/system-design/run-dedup-generation-01.md#convergent-producers,
// #the-shared-key-builders). Placed beside internal/runs/commentkeys as a
// sibling package so the office service, the office scheduler and the
// orchestrator can all reach it without an import cycle.
package dedupkeys
import (
"crypto/sha256"
"fmt"
"sort"
"strings"
)
// AssignmentKey builds the canonical task_assigned dedup key. Both
// task_assigned producers (office/service's queueTaskAssignedRun and
// office/scheduler's reactToAssigneeChange) call this rather than
// formatting the string themselves, which is what makes their convergence
// structural instead of coincidental (AC-OFFICE-RUN-DEDUP-002.1).
func AssignmentKey(taskID, agentProfileID string, generation int64) string {
return fmt.Sprintf("task_assigned:%s:%s:%d", taskID, agentProfileID, generation)
}
// BlockerDigest computes the shared digest over a blocker-task-id set. Both
// blocker producers (office/service's resolveAndWakeIfUnblocked and
// office/scheduler's cascadeBlockersResolved) call this so a set observed in
// different read order still converges on the same digest. The encoding is
// binding: sort ascending by byte value (NOT ListTaskBlockers's created_at,
// which is not unique), join with a single comma, then SHA-256 the UTF-8
// bytes and render the full digest as lowercase hex.
func BlockerDigest(blockerTaskIDs []string) string {
sorted := make([]string, len(blockerTaskIDs))
copy(sorted, blockerTaskIDs)
sort.Strings(sorted)
sum := sha256.Sum256([]byte(strings.Join(sorted, ",")))
return fmt.Sprintf("%x", sum)
}
@@ -0,0 +1,43 @@
package dedupkeys
import "testing"
func TestAssignmentKey(t *testing.T) {
got := AssignmentKey("task-1", "agent-1", 3)
want := "task_assigned:task-1:agent-1:3"
if got != want {
t.Fatalf("AssignmentKey() = %q, want %q", got, want)
}
}
func TestAssignmentKey_VariesByGeneration(t *testing.T) {
a := AssignmentKey("task-1", "agent-1", 1)
b := AssignmentKey("task-1", "agent-1", 2)
if a == b {
t.Fatalf("expected distinct keys for distinct generations, got %q for both", a)
}
}
func TestBlockerDigest_OrderIndependent(t *testing.T) {
a := BlockerDigest([]string{"b1", "b2", "b3"})
b := BlockerDigest([]string{"b3", "b1", "b2"})
if a != b {
t.Fatalf("BlockerDigest should be order-independent: %q != %q", a, b)
}
}
func TestBlockerDigest_VariesBySet(t *testing.T) {
a := BlockerDigest([]string{"b1", "b2"})
b := BlockerDigest([]string{"b1", "b2", "b3"})
if a == b {
t.Fatalf("expected distinct digests for distinct sets, got %q for both", a)
}
}
func TestBlockerDigest_DoesNotMutateInput(t *testing.T) {
input := []string{"b3", "b1", "b2"}
_ = BlockerDigest(input)
if input[0] != "b3" || input[1] != "b1" || input[2] != "b2" {
t.Fatalf("BlockerDigest mutated its input slice: %v", input)
}
}
+116
View File
@@ -0,0 +1,116 @@
package service
import (
"go.uber.org/zap"
"github.com/kandev/kandev/internal/common/logger"
runssqlite "github.com/kandev/kandev/internal/runs/repository/sqlite"
)
// QueueOutcomeNone means no enqueue was attempted, or the attempt returned
// an error. It is the zero value, so it is what a widened signature yields
// on any path that returns before deciding an outcome.
//
// It is declared identically in internal/workflow/engine/adapters.go — both
// declarations MUST match, the same invariant QueueOutcome itself already
// carries.
const QueueOutcomeNone QueueOutcome = ""
// QueueSource distinguishes which queue implementation made a dedup
// decision, for the "queue" label on office_run_dedup_total.
type QueueSource string
const (
// QueueSourceRuns identifies a suppression on the runs table (the
// idx_run_idempotency windowed lookup or durable unique index).
QueueSourceRuns QueueSource = "runs"
// QueueSourceWakeup identifies a suppression on the wakeup-request
// table. That table has no windowed lookup, so its kind is always
// "durable".
QueueSourceWakeup QueueSource = "wakeup"
)
// KeylessCause discriminates why a producer enqueued a wake with no dedup
// key: a generation that should have been available and was not
// (KeylessCauseUnresolved), or a producer that never had an occurrence to
// name (KeylessCauseByDesign). See
// docs/specs/office/system-design/run-dedup-generation-01.md#unresolvable-generation.
type KeylessCause string
const (
// KeylessCauseUnresolved means the occurrence has a generation that this
// producer simply was not handed or could not resolve.
KeylessCauseUnresolved KeylessCause = "unresolved"
// KeylessCauseByDesign means the producer never had an occurrence
// identity to name — a status transition with no redelivery path, an
// agent that expressed no dedup intent, and similar.
KeylessCauseByDesign KeylessCause = "by_design"
)
func dedupLogger() *logger.Logger {
return logger.Default().WithFields(zap.String("component", "runs-dedup"))
}
// ReportWindowedDedup records a suppression caught by the recent-duplicate
// lookup (a windowed dedup hit): counts kind="windowed", logs at Info, and
// returns QueueOutcomeDeduped.
func ReportWindowedDedup(q QueueSource, reason, key string) QueueOutcome {
incRunDedup(q, reason, "windowed")
dedupLogger().Info("run deduplicated (windowed)",
zap.String("queue", string(q)),
zap.String("reason", reason),
zap.String("key", key))
return QueueOutcomeDeduped
}
// ReportInsertResult classifies the error from an insert attempt. A unique
// violation on idx_run_idempotency counts kind="durable", logs at Warn with
// key, reason and agent, and returns (QueueOutcomeDeduped, nil). Any other
// non-nil error is returned UNCHANGED with QueueOutcomeNone and moves no
// counter — a disk error is not a dedup decision. A nil error returns
// (QueueOutcomeQueued, nil).
func ReportInsertResult(
q QueueSource, reason, key, agentProfileID string, err error,
) (QueueOutcome, error) {
if err == nil {
return QueueOutcomeQueued, nil
}
if runssqlite.IsIdempotencyKeyUniqueViolation(err) {
return ReportDurableDedup(q, reason, key, agentProfileID), nil
}
return QueueOutcomeNone, err
}
// ReportDurableDedup records a durable conflict the caller has already
// classified (the wakeup path's ErrWakeupIdempotencyConflict, or
// ReportInsertResult's own classification): counts kind="durable", logs at
// Warn, and returns QueueOutcomeDeduped.
func ReportDurableDedup(q QueueSource, reason, key, agentProfileID string) QueueOutcome {
incRunDedup(q, reason, "durable")
dedupLogger().Warn("run deduplicated (durable index)",
zap.String("queue", string(q)),
zap.String("reason", reason),
zap.String("key", key),
zap.String("agent_profile_id", agentProfileID))
return QueueOutcomeDeduped
}
// ReportKeylessEnqueue records a producer's decision to enqueue a wake with
// no dedup key, attributed to reason and discriminated by cause so a
// resolution failure (unresolved) is countable separately from expected,
// by-design keyless traffic. Called by the producer at its own decision
// site, immediately before enqueuing — the queue itself never infers a
// cause from an empty key. detail names what failed to resolve (a fixed
// lowercase snake_case constant, never a formatted message); it is a log
// field only, never a counter label, and is empty for cause=by_design,
// which is counted but never logged.
func ReportKeylessEnqueue(reason string, cause KeylessCause, detail string) {
incRunDedupKeyless(reason, cause)
if cause != KeylessCauseUnresolved {
return
}
dedupLogger().Info("run enqueued with no dedup key",
zap.String("reason", reason),
zap.String("cause", string(cause)),
zap.String("detail", detail))
}
@@ -0,0 +1,140 @@
package service_test
import (
"errors"
"expvar"
"strings"
"testing"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// counterHasLabel reports whether the named expvar.Map has an entry whose
// key contains every given substring. Counters are process-global expvar
// state (no reset hook), so tests assert presence/growth rather than exact
// values that could collide with other tests in the same binary run.
func counterHasLabel(t *testing.T, mapName string, substrs ...string) bool {
t.Helper()
v := expvar.Get(mapName)
if v == nil {
t.Fatalf("expvar map %q not registered", mapName)
}
m, ok := v.(*expvar.Map)
if !ok {
t.Fatalf("expvar %q is not a *expvar.Map", mapName)
}
found := false
m.Do(func(kv expvar.KeyValue) {
matches := true
for _, s := range substrs {
if !strings.Contains(kv.Key, s) {
matches = false
break
}
}
if matches {
found = true
}
})
return found
}
func TestQueueOutcomeNone_IsZeroValue(t *testing.T) {
if runsservice.QueueOutcomeNone != "" {
t.Fatalf("QueueOutcomeNone = %q, want empty string", runsservice.QueueOutcomeNone)
}
var zero runsservice.QueueOutcome
if zero != runsservice.QueueOutcomeNone {
t.Fatalf("zero QueueOutcome %q != QueueOutcomeNone", zero)
}
}
func TestReportWindowedDedup(t *testing.T) {
reason := "agent-supplied-windowed-" + t.Name()
outcome := runsservice.ReportWindowedDedup(runsservice.QueueSourceRuns, reason, "some-key")
if outcome != runsservice.QueueOutcomeDeduped {
t.Fatalf("outcome = %q, want deduped", outcome)
}
if !counterHasLabel(t, "office_run_dedup_total", "reason=custom", "kind=windowed", "queue=runs") {
t.Fatal("expected custom reasons to share the bounded windowed/runs entry")
}
if counterHasLabel(t, "office_run_dedup_total", "reason="+reason) {
t.Fatal("agent-supplied reason must not create its own metric series")
}
}
func TestReportInsertResult_NilError(t *testing.T) {
outcome, err := runsservice.ReportInsertResult(runsservice.QueueSourceRuns, "r", "k", "agent", nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if outcome != runsservice.QueueOutcomeQueued {
t.Fatalf("outcome = %q, want queued", outcome)
}
}
func TestReportInsertResult_UniqueViolation(t *testing.T) {
reason := "agent-supplied-durable-" + t.Name()
violation := errors.New("UNIQUE constraint failed: runs.idempotency_key")
outcome, err := runsservice.ReportInsertResult(runsservice.QueueSourceRuns, reason, "k", "agent-1", violation)
if err != nil {
t.Fatalf("expected nil error on classified conflict, got %v", err)
}
if outcome != runsservice.QueueOutcomeDeduped {
t.Fatalf("outcome = %q, want deduped", outcome)
}
if !counterHasLabel(t, "office_run_dedup_total", "reason=custom", "kind=durable", "queue=runs") {
t.Fatal("expected custom reasons to share the bounded durable/runs entry")
}
if counterHasLabel(t, "office_run_dedup_total", "reason="+reason) {
t.Fatal("agent-supplied reason must not create its own metric series")
}
}
func TestReportInsertResult_OtherError_PassesThrough(t *testing.T) {
reason := "test_other_" + t.Name()
diskErr := errors.New("disk I/O error")
outcome, err := runsservice.ReportInsertResult(runsservice.QueueSourceRuns, reason, "k", "agent-1", diskErr)
if !errors.Is(err, diskErr) {
t.Fatalf("expected the original error to pass through unchanged, got %v", err)
}
if outcome != runsservice.QueueOutcomeNone {
t.Fatalf("outcome = %q, want none", outcome)
}
if counterHasLabel(t, "office_run_dedup_total", "reason="+reason) {
t.Fatal("a non-conflict error must not move the dedup counter")
}
}
func TestReportDurableDedup_Wakeup(t *testing.T) {
reason := "agent-supplied-wakeup-" + t.Name()
outcome := runsservice.ReportDurableDedup(runsservice.QueueSourceWakeup, reason, "k", "agent-1")
if outcome != runsservice.QueueOutcomeDeduped {
t.Fatalf("outcome = %q, want deduped", outcome)
}
if !counterHasLabel(t, "office_run_dedup_total", "reason=custom", "kind=durable", "queue=wakeup") {
t.Fatal("expected custom reasons to share the bounded durable/wakeup entry")
}
if counterHasLabel(t, "office_run_dedup_total", "reason="+reason) {
t.Fatal("agent-supplied reason must not create its own metric series")
}
}
func TestReportKeylessEnqueue_CountsBothCauses(t *testing.T) {
reasonUnresolved := "agent-supplied-keyless-unresolved-" + t.Name()
reasonByDesign := "agent-supplied-keyless-by-design-" + t.Name()
runsservice.ReportKeylessEnqueue(reasonUnresolved, runsservice.KeylessCauseUnresolved, "some_detail")
runsservice.ReportKeylessEnqueue(reasonByDesign, runsservice.KeylessCauseByDesign, "")
if !counterHasLabel(t, "office_run_dedup_keyless_total", "reason=custom", "cause=unresolved") {
t.Fatal("expected custom reasons to share the bounded unresolved entry")
}
if !counterHasLabel(t, "office_run_dedup_keyless_total", "reason=custom", "cause=by_design") {
t.Fatal("expected custom reasons to share the bounded by_design entry")
}
if counterHasLabel(t, "office_run_dedup_keyless_total", "reason="+reasonUnresolved) ||
counterHasLabel(t, "office_run_dedup_keyless_total", "reason="+reasonByDesign) {
t.Fatal("agent-supplied reasons must not create their own keyless metric series")
}
}
@@ -0,0 +1,75 @@
package service
import (
"expvar"
"strings"
)
const metricReasonCustom = "custom"
// metricReasons is the bounded set of run reasons that receive their own
// metric series. Agent-supplied or future reasons use metricReasonCustom so
// process-global expvar maps cannot grow once per distinct input.
var metricReasons = map[string]struct{}{
"task_assigned": {},
"task_comment": {},
"task_blockers_resolved": {},
"task_children_completed": {},
"approval_resolved": {},
"task_review_requested": {},
"task_changes_requested": {},
"routine_trigger": {},
"heartbeat": {},
"budget_alert": {},
"agent_error": {},
"task_unblocked": {},
"task_reopened": {},
"task_reopened_via_comment": {},
"task_mentioned": {},
"stage_pending": {},
"stage_changes_requested": {},
"task_ready_to_close": {},
"routine_dispatch": {},
"routine_dispatch_cron": {},
"routine_dispatch_event": {},
"manual_resume_after_failure": {},
"review_started": {},
"approval_started": {},
"blockers_resolved": {},
"children_completed": {},
}
// expvar maps published at package init, exposed via stdlib's /debug/vars
// handler. Mirrors internal/office/scheduler/metrics_vars.go's "k=v;k=v"
// label model and counters-only rule.
var (
runDedupTotal = expvar.NewMap("office_run_dedup_total")
runDedupKeylessTotal = expvar.NewMap("office_run_dedup_keyless_total")
)
// metricLabel builds a "k1=v1;k2=v2;..." label string for an expvar map key.
func metricLabel(pairs ...string) string {
if len(pairs)%2 != 0 {
return ""
}
parts := make([]string, 0, len(pairs)/2)
for i := 0; i < len(pairs); i += 2 {
parts = append(parts, pairs[i]+"="+pairs[i+1])
}
return strings.Join(parts, ";")
}
func incRunDedup(q QueueSource, reason, kind string) {
runDedupTotal.Add(metricLabel("reason", metricReason(reason), "kind", kind, "queue", string(q)), 1)
}
func incRunDedupKeyless(reason string, cause KeylessCause) {
runDedupKeylessTotal.Add(metricLabel("reason", metricReason(reason), "cause", string(cause)), 1)
}
func metricReason(reason string) string {
if _, ok := metricReasons[reason]; ok {
return reason
}
return metricReasonCustom
}
@@ -0,0 +1,22 @@
package service_test
import (
"testing"
"github.com/kandev/kandev/internal/workflow/engine"
runsservice "github.com/kandev/kandev/internal/runs/service"
)
// TestQueueOutcomeNone_MatchesEngineDeclaration pins the "both MUST match"
// invariant both QueueOutcomeNone doc comments carry: runs/service and
// workflow/engine declare the same string type and zero-value constant
// independently, and a change to one without the other would silently break
// callers comparing an outcome received from one package against a constant
// imported from the other.
func TestQueueOutcomeNone_MatchesEngineDeclaration(t *testing.T) {
if string(runsservice.QueueOutcomeNone) != string(engine.QueueOutcomeNone) {
t.Fatalf("runsservice.QueueOutcomeNone = %q, engine.QueueOutcomeNone = %q; both MUST match",
runsservice.QueueOutcomeNone, engine.QueueOutcomeNone)
}
}
+20 -38
View File
@@ -12,7 +12,6 @@ package service
import (
"context"
"encoding/json"
"errors"
"fmt"
"time"
@@ -27,18 +26,6 @@ import (
runssqlite "github.com/kandev/kandev/internal/runs/repository/sqlite"
)
// errIdempotencyKeyConflict signals that insertRun's CreateRun failed
// because idx_run_idempotency rejected a duplicate idempotency_key —
// distinct from the earlier CheckIdempotencyKey miss, which only looks
// within IdempotencyWindowHours. Two independent producers deriving the
// same operation id for the same event can both pass that fast-path check
// before either commits (or the colliding row can simply be older than the
// window); the unique index is what actually stops the second insert.
// QueueRun treats this as a durable dedupe hit: QueueOutcomeDeduped, not an
// error, so the losing producer's caller does not abort or log a spurious
// failure for what is really a no-op.
var errIdempotencyKeyConflict = errors.New("idempotency key conflict")
// RunQueueAdapter is the interface the workflow engine uses to enqueue
// runs from queue_run actions. Phase 2 final's parallel agent
// declares the same shape inside internal/workflow/engine; both
@@ -180,34 +167,32 @@ func (s *Service) SubscribeSignal() <-chan struct{} { return s.signalCh }
func (s *Service) QueueRun(ctx context.Context, req QueueRunRequest) (QueueOutcome, error) {
agentInstanceID, err := s.resolveAgentInstance(ctx, req)
if err != nil {
return "", err
return QueueOutcomeNone, err
}
if agentInstanceID == "" {
return "", fmt.Errorf("queue run: agent_profile_id is required")
return QueueOutcomeNone, fmt.Errorf("queue run: agent_profile_id is required")
}
if req.IdempotencyKey != "" {
dup, err := s.repo.CheckIdempotencyKey(ctx, req.IdempotencyKey, IdempotencyWindowHours)
if err != nil {
return "", fmt.Errorf("idempotency check: %w", err)
return QueueOutcomeNone, fmt.Errorf("idempotency check: %w", err)
}
if dup {
s.log.Debug("run skipped (idempotent)",
zap.String("key", req.IdempotencyKey))
return QueueOutcomeDeduped, nil
return ReportWindowedDedup(QueueSourceRuns, req.Reason, req.IdempotencyKey), nil
}
}
payloadMap := runPayload(req, agentInstanceID)
payload, err := encodePayload(payloadMap)
if err != nil {
return "", fmt.Errorf("encode payload: %w", err)
return QueueOutcomeNone, fmt.Errorf("encode payload: %w", err)
}
if shouldCoalesceRun(req) {
coalesced, err := s.repo.CoalesceRun(ctx, agentInstanceID, req.Reason, CoalesceWindowSeconds, payload)
if err != nil {
return "", fmt.Errorf("coalesce check: %w", err)
return QueueOutcomeNone, fmt.Errorf("coalesce check: %w", err)
}
if coalesced {
s.log.Debug("run coalesced",
@@ -220,20 +205,17 @@ func (s *Service) QueueRun(ctx context.Context, req QueueRunRequest) (QueueOutco
}
}
row, err := s.insertRun(ctx, agentInstanceID, req, payload)
row, insertErr := s.insertRun(ctx, agentInstanceID, req, payload)
// idx_run_idempotency has no time bound, so a conflict here can come
// from a row older than IdempotencyWindowHours, not just the windowed
// race CheckIdempotencyKey guards against above. ReportInsertResult
// classifies that as a no-op dedupe rather than a hard error.
outcome, err := ReportInsertResult(QueueSourceRuns, req.Reason, req.IdempotencyKey, agentInstanceID, insertErr)
if err != nil {
// idx_run_idempotency has no time bound, so a conflict here can
// come from a row older than IdempotencyWindowHours, not just the
// windowed race CheckIdempotencyKey guards against above. Either
// way the existing row is definitionally the same operation this
// key identifies, so treat it as a no-op dedupe rather than a hard
// error (see errIdempotencyKeyConflict's doc comment).
if errors.Is(err, errIdempotencyKeyConflict) {
s.log.Debug("run skipped (idempotency index race)",
zap.String("key", req.IdempotencyKey))
return QueueOutcomeDeduped, nil
}
return "", err
return QueueOutcomeNone, err
}
if outcome == QueueOutcomeDeduped {
return outcome, nil
}
s.log.Info("run queued",
@@ -247,7 +229,10 @@ func (s *Service) QueueRun(ctx context.Context, req QueueRunRequest) (QueueOutco
}
// insertRun creates the runs row and returns it. Pulled out of
// QueueRun to keep the latter under the funlen budget.
// QueueRun to keep the latter under the funlen budget. The returned error is
// the raw CreateRun error (wrapped for context, never reclassified) — the
// caller runs it through ReportInsertResult, which does its own unique-index
// classification against the original error chain.
func (s *Service) insertRun(
ctx context.Context, agentInstanceID string, req QueueRunRequest, payload string,
) (*models.Run, error) {
@@ -267,9 +252,6 @@ func (s *Service) insertRun(
RequestedAt: time.Now().UTC(),
}
if err := s.repo.CreateRun(ctx, row); err != nil {
if runssqlite.IsIdempotencyKeyUniqueViolation(err) {
return nil, errIdempotencyKeyConflict
}
return nil, fmt.Errorf("enqueue run: %w", err)
}
return row, nil
@@ -116,6 +116,12 @@ func (r *Repository) runMigrations() error {
if err := r.migrateTasksRemoveWorkflowFK(); err != nil {
return err
}
// Must run AFTER migrateTasksRemoveWorkflowFK: that migration recreates
// tasks from an explicit column list. Adding this column beforehand would
// have it silently dropped by the recreate on any database still carrying
// the legacy FK, leaving it absent for the remainder of that boot (the
// same hazard class as the task_sessions.name comment above).
_ = r.migrate.Apply("tasks.assignment_generation", `ALTER TABLE tasks ADD COLUMN assignment_generation INTEGER NOT NULL DEFAULT 0`)
if err := r.dropRetiredSlackIntegration(); err != nil {
return err
}
@@ -390,6 +390,14 @@ func (r *Repository) insertTaskTx(ctx context.Context, tx *sql.Tx, task *models.
if err := upsertRunnerInTx(ctx, tx, r.db.Rebind, task.WorkflowStepID, task.ID, task.AssigneeAgentProfileID); err != nil {
return "", err
}
// A task created already assigned starts at generation 1, the same
// value UpdateTaskAssignee would commit for a first assignment - a
// creation and a following first assignment must not both mint
// generation 1.
if _, err := tx.ExecContext(ctx, r.db.Rebind(
`UPDATE tasks SET assignment_generation = 1 WHERE id = ?`), task.ID); err != nil {
return "", err
}
}
return entryID, nil
}
@@ -0,0 +1,117 @@
package sqlite
import (
"strings"
"testing"
"github.com/jmoiron/sqlx"
_ "github.com/mattn/go-sqlite3"
)
// TestMigrateTasksRemoveWorkflowFK_AssignmentGenerationSurvivesRebuild is the
// replay regression test flagged (but not written) in the run-dedup-generation
// build: migrateTasksRemoveWorkflowFK (base_migrations.go) recreates `tasks`
// from an explicit 19-column list that predates assignment_generation. The
// tasks.assignment_generation ADD COLUMN is sequenced strictly AFTER that
// rebuild in runMigrations() precisely so a database still carrying the
// legacy `FOREIGN KEY (workflow_id)` DDL does not have the new column
// silently dropped for the remainder of that boot (the same hazard class as
// the task_sessions.name comment in the same file). This had zero coverage
// before this test: nothing exercised migrateTasksRemoveWorkflowFK's rebuild
// path at all.
func TestMigrateTasksRemoveWorkflowFK_AssignmentGenerationSurvivesRebuild(t *testing.T) {
db, err := sqlx.Open("sqlite3", ":memory:")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
t.Cleanup(func() { _ = db.Close() })
// Seed a pre-migration `tasks` table: the legacy FK clause that triggers
// the rebuild, the 19 columns migrateTasksRemoveWorkflowFK's SELECT list
// expects, and no assignment_generation column at all — matching a real
// database that predates this feature.
if _, err := db.Exec(`
CREATE TABLE tasks (
id TEXT PRIMARY KEY,
workspace_id TEXT NOT NULL DEFAULT '',
workflow_id TEXT NOT NULL DEFAULT '',
workflow_step_id TEXT NOT NULL DEFAULT '',
title TEXT NOT NULL,
description TEXT DEFAULT '',
state TEXT DEFAULT 'TODO',
priority INTEGER DEFAULT 0,
position INTEGER DEFAULT 0,
wip_admitted INTEGER NOT NULL DEFAULT 1,
queued_for_step_id TEXT NOT NULL DEFAULT '',
queued_at TIMESTAMP,
metadata TEXT DEFAULT '{}',
is_ephemeral INTEGER NOT NULL DEFAULT 0,
parent_id TEXT DEFAULT '',
autopilot_enabled INTEGER NOT NULL DEFAULT 0,
archived_at TIMESTAMP,
created_at TIMESTAMP NOT NULL,
updated_at TIMESTAMP NOT NULL,
FOREIGN KEY (workflow_id) REFERENCES workflows(id)
);
INSERT INTO tasks (id, workspace_id, title, created_at, updated_at)
VALUES ('task-legacy-fk', 'ws-legacy', 'Legacy FK task', CURRENT_TIMESTAMP, CURRENT_TIMESTAMP);
`); err != nil {
t.Fatalf("seed legacy schema: %v", err)
}
if _, err := NewWithDB(db, db, nil); err != nil {
t.Fatalf("init task repo (run migrations): %v", err)
}
// The rebuild must have fired: the legacy FK clause is gone.
var tableSQL string
if err := db.QueryRow(
`SELECT sql FROM sqlite_master WHERE type='table' AND name='tasks'`,
).Scan(&tableSQL); err != nil {
t.Fatalf("read tasks schema: %v", err)
}
if strings.Contains(tableSQL, "FOREIGN KEY (workflow_id)") {
t.Fatalf("tasks table still carries the legacy workflow_id FK after migration: %s", tableSQL)
}
// assignment_generation must be present post-rebuild, not silently
// dropped by the rebuild's pre-feature explicit column list.
rows, err := db.Queryx(`PRAGMA table_info(tasks)`)
if err != nil {
t.Fatalf("pragma table_info: %v", err)
}
found := false
for rows.Next() {
var cid int
var name, ctype string
var notnull, pk int
var dflt *string
if err := rows.Scan(&cid, &name, &ctype, &notnull, &dflt, &pk); err != nil {
_ = rows.Close()
t.Fatalf("scan pragma: %v", err)
}
if name == "assignment_generation" {
found = true
}
}
_ = rows.Close()
if !found {
t.Fatalf("assignment_generation column missing after migrating a legacy FK-bearing tasks table")
}
// The pre-existing row must survive the rebuild, with the new column
// defaulted to 0.
var gotTitle string
var gotGeneration int64
if err := db.QueryRow(
`SELECT title, assignment_generation FROM tasks WHERE id = ?`, "task-legacy-fk",
).Scan(&gotTitle, &gotGeneration); err != nil {
t.Fatalf("query migrated row: %v", err)
}
if gotTitle != "Legacy FK task" {
t.Fatalf("title = %q, want %q (row lost during rebuild)", gotTitle, "Legacy FK task")
}
if gotGeneration != 0 {
t.Fatalf("assignment_generation = %d, want 0 for a pre-existing row", gotGeneration)
}
}
@@ -440,7 +440,11 @@ func (s *Service) finalizeCreatedTask(ctx context.Context, prepared *preparedTas
task.Repositories = repos
}
s.publishTaskEvent(ctx, events.TaskCreated, task, nil)
s.publishTaskEventWithExtra(ctx, events.TaskCreated, task, nil,
map[string]interface{}{
"assignee_agent_profile_id": task.AssigneeAgentProfileID,
"assignment_generation": assignmentGenerationForCreate(task),
})
s.pullTasksFromNewFeederWork(ctx, task.WorkflowID, task.WorkflowStepID)
if refreshed, err := s.tasks.GetTask(ctx, task.ID); err != nil {
s.logger.Warn("failed to refresh task after feeder pull", zap.String("task_id", task.ID), zap.Error(err))
@@ -453,6 +457,17 @@ func (s *Service) finalizeCreatedTask(ctx context.Context, prepared *preparedTas
return CreateTaskResult{Task: task, Outcome: CreateTaskOutcomeCreated}, nil
}
// assignmentGenerationForCreate mirrors insertTaskTx's runner-row guard in
// memory rather than re-reading the row it just wrote: a task created
// already assigned starts at generation 1 (matching the value insertTaskTx
// committed), everything else starts at 0 (never assigned).
func assignmentGenerationForCreate(task *models.Task) int64 {
if task.AssigneeAgentProfileID != "" && task.WorkflowStepID != "" {
return 1
}
return 0
}
func (s *Service) prepareWorkspacePolicyForCreation(ctx context.Context, req *CreateTaskRequest) error {
policy := WorkspacePolicy{}
if req.WorkspacePolicy != nil {
@@ -0,0 +1,125 @@
package service
import (
"context"
"testing"
"github.com/kandev/kandev/internal/task/models"
sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite"
)
// setupOfficeTestWithBus mirrors setupOfficeTest but keeps the MockEventBus
// so a test can inspect the payload of a published task.created event.
func setupOfficeTestWithBus(t *testing.T) (*Service, *MockEventBus, *sqliterepo.Repository) {
t.Helper()
svc, bus, repo := createTestService(t)
ctx := context.Background()
_, err := repo.DB().Exec(`
CREATE TABLE IF NOT EXISTS workflow_steps (
id TEXT PRIMARY KEY,
workflow_id TEXT NOT NULL,
name TEXT NOT NULL,
position INTEGER NOT NULL,
color TEXT DEFAULT '',
prompt TEXT DEFAULT '',
events TEXT DEFAULT '{}',
allow_manual_move INTEGER DEFAULT 1,
is_start_step INTEGER DEFAULT 0,
show_in_command_panel INTEGER DEFAULT 1,
auto_archive_after_hours INTEGER DEFAULT 0,
agent_profile_id TEXT DEFAULT '',
created_at TIMESTAMP NOT NULL,
updated_at TIMESTAMP NOT NULL,
FOREIGN KEY (workflow_id) REFERENCES workflows(id) ON DELETE CASCADE
)`)
if err != nil {
t.Fatalf("create workflow_steps: %v", err)
}
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}); err != nil {
t.Fatalf("create workspace: %v", err)
}
if _, err := repo.EnsureOfficeWorkflow(ctx, "ws-1"); err != nil {
t.Fatalf("EnsureOfficeWorkflow: %v", err)
}
svc.SetStartStepResolver(&dbStepResolver{repo: repo})
svc.SetWorkspacePolicyAttacher(noOpWorkspacePolicyAttacher{})
return svc, bus, repo
}
// AC-OFFICE-RUN-DEDUP-001.9: a task created already assigned publishes
// assignment_generation 1 on its task.created event, matching the value
// insertTaskTx committed - a creation and the first following assignment
// must not both mint generation 1.
func TestCreateTask_Assigned_PublishesAssignmentGenerationOne(t *testing.T) {
svc, bus, repo := setupOfficeTestWithBus(t)
ctx := context.Background()
result, err := svc.CreateTask(ctx, &CreateTaskRequest{
WorkspaceID: "ws-1",
Title: "Agent Task",
Origin: models.TaskOriginAgentCreated,
AssigneeAgentProfileID: "agent-1",
})
if err != nil {
t.Fatalf("CreateTask: %v", err)
}
data := singlePublishedEventData(t, bus)
gen, ok := data["assignment_generation"].(int64)
if !ok || gen != 1 {
t.Fatalf("assignment_generation = %#v, want int64(1)", data["assignment_generation"])
}
if got, ok := data["assignee_agent_profile_id"].(string); !ok || got != "agent-1" {
t.Fatalf("assignee_agent_profile_id = %#v, want agent-1", data["assignee_agent_profile_id"])
}
stored, err := repo.GetTask(ctx, result.Task.ID)
if err != nil {
t.Fatalf("GetTask: %v", err)
}
var persistedGen int64
if err := repo.DB().QueryRowContext(ctx,
`SELECT assignment_generation FROM tasks WHERE id = ?`, stored.ID).Scan(&persistedGen); err != nil {
t.Fatalf("query assignment_generation: %v", err)
}
if persistedGen != 1 {
t.Fatalf("persisted assignment_generation = %d, want 1", persistedGen)
}
}
// AC-OFFICE-RUN-DEDUP-001.9 boundary: a task created UNASSIGNED publishes
// assignment_generation 0, never 1 - the boundary that keeps a creation and a
// first assignment from both minting generation 1.
func TestCreateTask_Unassigned_PublishesAssignmentGenerationZero(t *testing.T) {
svc, bus, repo := setupOfficeTestWithBus(t)
ctx := context.Background()
result, err := svc.CreateTask(ctx, &CreateTaskRequest{
WorkspaceID: "ws-1",
Title: "Unassigned Task",
ProjectID: "proj-1",
})
if err != nil {
t.Fatalf("CreateTask: %v", err)
}
data := singlePublishedEventData(t, bus)
gen, ok := data["assignment_generation"].(int64)
if !ok || gen != 0 {
t.Fatalf("assignment_generation = %#v, want int64(0)", data["assignment_generation"])
}
if got, ok := data["assignee_agent_profile_id"].(string); !ok || got != "" {
t.Fatalf("assignee_agent_profile_id = %#v, want empty", data["assignee_agent_profile_id"])
}
var persistedGen int64
if err := repo.DB().QueryRowContext(ctx,
`SELECT assignment_generation FROM tasks WHERE id = ?`, result.Task.ID).Scan(&persistedGen); err != nil {
t.Fatalf("query assignment_generation: %v", err)
}
if persistedGen != 0 {
t.Fatalf("persisted assignment_generation = %d, want 0", persistedGen)
}
}
@@ -34,6 +34,12 @@ const (
// queued row for the same agent + reason within the coalescing
// window, so nothing new was inserted.
QueueOutcomeCoalesced QueueOutcome = "coalesced"
// QueueOutcomeNone means no enqueue was attempted, or the attempt
// returned an error. It is the zero value, so it is what a widened
// signature yields on any path that returns before deciding an
// outcome. Declared identically in internal/runs/service — both
// declarations MUST match.
QueueOutcomeNone QueueOutcome = ""
)
// QueueRunRequest is the typed payload the engine hands to RunQueueAdapter.