diff --git a/services/core/docker-compose.yml b/services/core/docker-compose.yml index 1d84c24..806cda9 100644 --- a/services/core/docker-compose.yml +++ b/services/core/docker-compose.yml @@ -39,6 +39,7 @@ services: GITO_REMOTE_NAME: ${GITO_REMOTE_NAME:-origin} ROADMAP_CREATION_TODO_STATE_ID: ${ROADMAP_CREATION_TODO_STATE_ID:-} PLANE_TODO_STATE_ID: ${PLANE_TODO_STATE_ID:-} + PLANE_PROJECT_ID: ${PLANE_PROJECT_ID:-} ports: - "${NOMADCODE_CORE_HOST_PORT:-18010}:8080" volumes: diff --git a/services/core/internal/scheduler/jobs_test.go b/services/core/internal/scheduler/jobs_test.go index 16644ce..474a6f2 100644 --- a/services/core/internal/scheduler/jobs_test.go +++ b/services/core/internal/scheduler/jobs_test.go @@ -19,8 +19,10 @@ import ( "github.com/nomadcode/nomadcode-core/internal/notification" "github.com/nomadcode/nomadcode-core/internal/projectsync" "github.com/nomadcode/nomadcode-core/internal/roadmapsync" + "github.com/nomadcode/nomadcode-core/internal/roadmapsyncpipeline" "github.com/nomadcode/nomadcode-core/internal/storage" "github.com/nomadcode/nomadcode-core/internal/workflow" + "github.com/nomadcode/nomadcode-core/internal/workitem" ) func TestRunAgentTaskCompletesFromMessage(t *testing.T) { @@ -1522,3 +1524,68 @@ func TestRunTaskAuthoringFailsWhenIdentityWriterMissing(t *testing.T) { t.Errorf("expected authoring_failure_category=identity_write_failed, got %q", fi.ExtraMetadata["authoring_failure_category"]) } } + +type fakeSyncRunner struct { + result roadmapsyncpipeline.SyncCreationResult + err error +} + +func (f fakeSyncRunner) SyncCreation(_ context.Context, _ roadmapsyncpipeline.SyncCreationInput) (roadmapsyncpipeline.SyncCreationResult, error) { + return f.result, f.err +} + +type fakeCreationTaskFinalizer struct { + called bool + err error +} + +func (f *fakeCreationTaskFinalizer) CompleteTaskByExternalRef(_ context.Context, _, _ string, _ json.RawMessage) (storage.Task, bool, error) { + f.called = true + return storage.Task{}, false, f.err +} + +func TestRoadmapCreationSyncWorkerSkipsFinalizerWhenRefIDEmpty(t *testing.T) { + // When SyncCreation returns SyncActionProjected but args.Ref.ID is empty + // (missing-create case), TaskFinalizer must NOT be called to avoid + // pgx.ErrNoRows causing an unintended River retry loop. + finalizer := &fakeCreationTaskFinalizer{} + worker := &RoadmapCreationSyncWorker{ + Sync: fakeSyncRunner{result: roadmapsyncpipeline.SyncCreationResult{Action: roadmapsyncpipeline.SyncActionProjected}}, + TaskFinalizer: finalizer, + } + job := &river.Job[RoadmapCreationSyncJobArgs]{ + Args: RoadmapCreationSyncJobArgs{ + // Ref.ID is zero-value (empty) — missing-create case + }, + } + if err := worker.Work(context.Background(), job); err != nil { + t.Fatalf("Work returned error: %v", err) + } + if finalizer.called { + t.Error("TaskFinalizer.CompleteTaskByExternalRef must not be called when Ref.ID is empty (missing-create case)") + } +} + +func TestRoadmapCreationSyncWorkerCallsFinalizerWhenRefIDSet(t *testing.T) { + // When SyncCreation returns SyncActionProjected and args.Ref.ID is non-empty + // (reconcile case), TaskFinalizer IS called so workflow tasks can be completed. + finalizer := &fakeCreationTaskFinalizer{} + worker := &RoadmapCreationSyncWorker{ + Sync: fakeSyncRunner{result: roadmapsyncpipeline.SyncCreationResult{Action: roadmapsyncpipeline.SyncActionProjected}}, + TaskFinalizer: finalizer, + } + job := &river.Job[RoadmapCreationSyncJobArgs]{ + Args: RoadmapCreationSyncJobArgs{ + Ref: workitem.Ref{ + Provider: "plane", + ID: "6945a5e7-b2ef-4d48-a037-00fa2b6ab99b", + }, + }, + } + if err := worker.Work(context.Background(), job); err != nil { + t.Fatalf("Work returned error: %v", err) + } + if !finalizer.called { + t.Error("TaskFinalizer.CompleteTaskByExternalRef must be called when Ref.ID is non-empty (reconcile case)") + } +} diff --git a/services/core/internal/scheduler/roadmap_sync_jobs.go b/services/core/internal/scheduler/roadmap_sync_jobs.go index 9890b44..8263d0b 100644 --- a/services/core/internal/scheduler/roadmap_sync_jobs.go +++ b/services/core/internal/scheduler/roadmap_sync_jobs.go @@ -88,7 +88,7 @@ func (w *RoadmapCreationSyncWorker) Work(ctx context.Context, job *river.Job[Roa return err } - if (res.Action == roadmapsyncpipeline.SyncActionProjected || res.Action == roadmapsyncpipeline.SyncActionComplete) && w.TaskFinalizer != nil { + if (res.Action == roadmapsyncpipeline.SyncActionProjected || res.Action == roadmapsyncpipeline.SyncActionComplete) && w.TaskFinalizer != nil && args.Ref.ID != "" { resultMap := map[string]any{ "action": string(res.Action), "reason": res.Reason, diff --git a/services/core/internal/workflow/service.go b/services/core/internal/workflow/service.go index aecfc55..74cda58 100644 --- a/services/core/internal/workflow/service.go +++ b/services/core/internal/workflow/service.go @@ -8,6 +8,7 @@ import ( "strings" "sync" + "github.com/jackc/pgx/v5" "github.com/nomadcode/nomadcode-core/internal/storage" ) @@ -241,6 +242,9 @@ func (s *Service) CompleteTaskByExternalRef(ctx context.Context, provider, id st task, err := s.WithExternalRefLock(ctx, provider, id, func(ctx context.Context) (storage.Task, error) { t, err := s.lifecycle.store.GetTaskByExternalRef(ctx, provider, id) if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return storage.Task{}, nil + } return storage.Task{}, err } if t.Status == string(StatusCompleted) { diff --git a/services/core/internal/workflow/service_test.go b/services/core/internal/workflow/service_test.go index a2fbe86..9737baa 100644 --- a/services/core/internal/workflow/service_test.go +++ b/services/core/internal/workflow/service_test.go @@ -828,3 +828,23 @@ func TestServiceCompleteTaskByExternalRefPromotesAuthoringMetadata(t *testing.T) t.Errorf("expected status_reason 'completed via scan', got %v", meta[MetadataKeyStatusReason]) } } + +func TestCompleteTaskByExternalRefNoRowsIsGracefulNoOp(t *testing.T) { + // When GetTaskByExternalRef returns pgx.ErrNoRows (no workflow task registered + // for the given provider/id), CompleteTaskByExternalRef must return + // (zero, false, nil) so the caller does not treat missing tasks as errors. + store := newFakeTaskStore() // empty store → GetTaskByExternalRef returns pgx.ErrNoRows + svc := NewService(nil, nil, nil) + svc.lifecycle = &Lifecycle{store: store} + + task, transitioned, err := svc.CompleteTaskByExternalRef(context.Background(), "plane", "6945a5e7-b2ef-4d48-a037-00fa2b6ab99b", nil) + if err != nil { + t.Fatalf("expected nil error for missing task, got: %v", err) + } + if transitioned { + t.Error("expected transitioned=false for missing task") + } + if task.ID != "" { + t.Errorf("expected zero task for missing task, got: %+v", task) + } +}