fix(scheduler): skip task finalizer when Ref.ID is empty for missing-create jobs
On the missing-create path, SyncCreation creates a new Plane work item and returns SyncActionProjected before any workflow task has been registered for the new work item UUID. Calling CompleteTaskByExternalRef with an empty Ref.ID caused pgx.ErrNoRows → River retry → reconcile returning not_ready, leaving only develop_matched recorded. Fix 1 (roadmap_sync_jobs.go): guard CompleteTaskByExternalRef with args.Ref.ID != "" so missing-create jobs complete cleanly without retrying. Fix 2 (workflow/service.go): CompleteTaskByExternalRef now returns (zero, false, nil) when GetTaskByExternalRef returns pgx.ErrNoRows, so reconcile jobs that have no registered workflow task also complete cleanly rather than triggering retry loops. Also adds PLANE_PROJECT_ID to docker-compose.yml environment so the missing-create bridge receives non-empty tenant/project inputs at runtime. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
2d3a24d261
commit
1a99b3d04b
5 changed files with 93 additions and 1 deletions
|
|
@ -39,6 +39,7 @@ services:
|
||||||
GITO_REMOTE_NAME: ${GITO_REMOTE_NAME:-origin}
|
GITO_REMOTE_NAME: ${GITO_REMOTE_NAME:-origin}
|
||||||
ROADMAP_CREATION_TODO_STATE_ID: ${ROADMAP_CREATION_TODO_STATE_ID:-}
|
ROADMAP_CREATION_TODO_STATE_ID: ${ROADMAP_CREATION_TODO_STATE_ID:-}
|
||||||
PLANE_TODO_STATE_ID: ${PLANE_TODO_STATE_ID:-}
|
PLANE_TODO_STATE_ID: ${PLANE_TODO_STATE_ID:-}
|
||||||
|
PLANE_PROJECT_ID: ${PLANE_PROJECT_ID:-}
|
||||||
ports:
|
ports:
|
||||||
- "${NOMADCODE_CORE_HOST_PORT:-18010}:8080"
|
- "${NOMADCODE_CORE_HOST_PORT:-18010}:8080"
|
||||||
volumes:
|
volumes:
|
||||||
|
|
|
||||||
|
|
@ -19,8 +19,10 @@ import (
|
||||||
"github.com/nomadcode/nomadcode-core/internal/notification"
|
"github.com/nomadcode/nomadcode-core/internal/notification"
|
||||||
"github.com/nomadcode/nomadcode-core/internal/projectsync"
|
"github.com/nomadcode/nomadcode-core/internal/projectsync"
|
||||||
"github.com/nomadcode/nomadcode-core/internal/roadmapsync"
|
"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/storage"
|
||||||
"github.com/nomadcode/nomadcode-core/internal/workflow"
|
"github.com/nomadcode/nomadcode-core/internal/workflow"
|
||||||
|
"github.com/nomadcode/nomadcode-core/internal/workitem"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestRunAgentTaskCompletesFromMessage(t *testing.T) {
|
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"])
|
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)")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -88,7 +88,7 @@ func (w *RoadmapCreationSyncWorker) Work(ctx context.Context, job *river.Job[Roa
|
||||||
return err
|
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{
|
resultMap := map[string]any{
|
||||||
"action": string(res.Action),
|
"action": string(res.Action),
|
||||||
"reason": res.Reason,
|
"reason": res.Reason,
|
||||||
|
|
|
||||||
|
|
@ -8,6 +8,7 @@ import (
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
|
|
||||||
|
"github.com/jackc/pgx/v5"
|
||||||
"github.com/nomadcode/nomadcode-core/internal/storage"
|
"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) {
|
task, err := s.WithExternalRefLock(ctx, provider, id, func(ctx context.Context) (storage.Task, error) {
|
||||||
t, err := s.lifecycle.store.GetTaskByExternalRef(ctx, provider, id)
|
t, err := s.lifecycle.store.GetTaskByExternalRef(ctx, provider, id)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
if errors.Is(err, pgx.ErrNoRows) {
|
||||||
|
return storage.Task{}, nil
|
||||||
|
}
|
||||||
return storage.Task{}, err
|
return storage.Task{}, err
|
||||||
}
|
}
|
||||||
if t.Status == string(StatusCompleted) {
|
if t.Status == string(StatusCompleted) {
|
||||||
|
|
|
||||||
|
|
@ -828,3 +828,23 @@ func TestServiceCompleteTaskByExternalRefPromotesAuthoringMetadata(t *testing.T)
|
||||||
t.Errorf("expected status_reason 'completed via scan', got %v", meta[MetadataKeyStatusReason])
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue