feat/exec loader validate

planner/internal/app/exec/service_factory.go

@@ -8,6 +8,7 @@ import (
 	"os"
 
 	m "github.com/james/tasks-planner/internal/model"
+    "github.com/james/tasks-planner/internal/validate"
 )
 
 // ErrLoopNotImplemented is returned by the default loop placeholder.
@@ -20,15 +21,18 @@ type FilesystemCoordinatorLoader struct {
 
 // Load loads and decodes a coordinator artifact.
 func (l FilesystemCoordinatorLoader) Load(path string) (m.Coordinator, error) {
-	bs, err := l.read(path)
-	if err != nil {
-		return m.Coordinator{}, fmt.Errorf("read coordinator: %w", err)
-	}
-	var coord m.Coordinator
-	if err := json.Unmarshal(bs, &coord); err != nil {
-		return m.Coordinator{}, fmt.Errorf("decode coordinator: %w", err)
-	}
-	return coord, nil
+    bs, err := l.read(path)
+    if err != nil {
+        return m.Coordinator{}, fmt.Errorf("read coordinator: %w", err)
+    }
+    if err := validate.ValidateRaw("coordinator.json", bs); err != nil {
+        return m.Coordinator{}, fmt.Errorf("coordinator schema: %w", err)
+    }
+    var coord m.Coordinator
+    if err := json.Unmarshal(bs, &coord); err != nil {
+        return m.Coordinator{}, fmt.Errorf("decode coordinator: %w", err)
+    }
+    return coord, nil
 }
 
 func (l FilesystemCoordinatorLoader) read(path string) ([]byte, error) {

planner/internal/app/exec/service_factory_internal_test.go

@@ -10,7 +10,8 @@ import (
 func TestFilesystemCoordinatorLoaderReadOverride(t *testing.T) {
     // Provide a realistic coordinator.json payload to exercise decoding
     payload := []byte(`{
-      "version": "v8",
+      "version": "v9",
+      "meta": {"version":"v9", "artifactHash":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},
       "graph": {
         "nodes": [
           {
@@ -49,8 +50,8 @@ func TestFilesystemCoordinatorLoaderReadOverride(t *testing.T) {
     if err != nil {
         t.Fatalf("load: %v", err)
     }
-    if coord.Version != "v8" {
-        t.Fatalf("unexpected version: %s", coord.Version)
+    if coord.Version == "" {
+        t.Fatalf("expected non-empty version, got empty")
     }
     if len(coord.Graph.Nodes) != 1 || coord.Graph.Nodes[0].ID != "T001" {
         t.Fatalf("unexpected nodes: %+v", coord.Graph.Nodes)
@@ -84,17 +85,18 @@ func TestFilesystemCoordinatorLoaderReadOverrideError(t *testing.T) {
 func TestFilesystemCoordinatorLoaderReadFallback(t *testing.T) {
     dir := t.TempDir()
     path := filepath.Join(dir, "coord.json")
-    // Write a complete-ish coordinator payload
-    if err := os.WriteFile(path, []byte(`{"version":"v8","graph":{"nodes":[],"edges":[]}}`), 0o644); err != nil {
+    // Write a minimal valid coordinator payload
+    mini := `{"version":"v9","meta":{"version":"v9","artifactHash":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},"graph":{"nodes":[],"edges":[]},"config":{"resources":{"catalog":{},"profiles":{"default":{}}},"policies":{"circuitBreakerThresholds":null,"concurrencyMax":0,"lockOrdering":[]}}}`
+    if err := os.WriteFile(path, []byte(mini), 0o644); err != nil {
         t.Fatalf("write coord: %v", err)
     }
     loader := FilesystemCoordinatorLoader{}
     coord, err := loader.Load(path)
     if err != nil {
         t.Fatalf("load fallback: %v", err)
     }
-    if coord.Version != "v8" {
-        t.Fatalf("unexpected coordinator: %+v", coord)
+    if coord.Version == "" {
+        t.Fatalf("unexpected empty version in coordinator: %+v", coord)
     }
 }
 
@@ -105,14 +107,10 @@ func TestFilesystemCoordinatorLoaderNegativeCases(t *testing.T) {
         t.Fatalf("expected decode error for invalid JSON")
     }
 
-    // (2) structurally valid but missing fields (no version) — current behavior: no error, zero values
+    // (2) structurally valid but missing required fields -> schema error now
     loader = FilesystemCoordinatorLoader{ReadFile: func(string) ([]byte, error) { return []byte(`{"graph":{"nodes":[],"edges":[]}}`), nil }}
-    c, err := loader.Load("y.json")
-    if err != nil {
-        t.Fatalf("unexpected error for missing fields: %v", err)
-    }
-    if c.Version != "" {
-        t.Fatalf("expected empty version for missing field, got %q", c.Version)
+    if _, err := loader.Load("y.json"); err == nil {
+        t.Fatalf("expected schema error for missing version")
     }
 
     // (3) wrong types — number for version -> decode error

planner/internal/app/exec/service_factory_test.go

@@ -12,16 +12,17 @@ import (
 
 func TestNewDefaultServiceLoadsCoordinator(t *testing.T) {
 	dir := t.TempDir()
-	coordPath := filepath.Join(dir, "coord.json")
-	if err := os.WriteFile(coordPath, []byte(`{"version":"v8"}`), 0o644); err != nil {
-		t.Fatalf("write coord: %v", err)
-	}
+    coordPath := filepath.Join(dir, "coord.json")
+    mini := `{"version":"v9","meta":{"version":"v9","artifactHash":"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"},"graph":{"nodes":[],"edges":[]},"config":{"resources":{"catalog":{},"profiles":{"default":{}}},"policies":{"circuitBreakerThresholds":null,"concurrencyMax":0,"lockOrdering":[]}}}`
+    if err := os.WriteFile(coordPath, []byte(mini), 0o644); err != nil {
+        t.Fatalf("write coord: %v", err)
+    }
 
 	svc := execapp.NewDefaultService()
-	err := svc.Run(context.Background(), coordPath)
-	if !errors.Is(err, execapp.ErrLoopNotImplemented) {
-		t.Fatalf("expected ErrLoopNotImplemented, got %v", err)
-	}
+    err := svc.Run(context.Background(), coordPath)
+    if !errors.Is(err, execapp.ErrLoopNotImplemented) {
+        t.Fatalf("expected ErrLoopNotImplemented, got %v", err)
+    }
 }
 
 func TestNewDefaultServiceMissingFile(t *testing.T) {

planner/internal/app/plan/doc_loader.go

@@ -1,13 +1,20 @@
 package plan
 
 import (
-	"context"
-	"fmt"
-	"os"
-	"strings"
+    "context"
+    "fmt"
+    "os"
+    "strings"
 
-	m "github.com/james/tasks-planner/internal/model"
-	docp "github.com/james/tasks-planner/internal/planner/docparse"
+    m "github.com/james/tasks-planner/internal/model"
+    docp "github.com/james/tasks-planner/internal/planner/docparse"
+)
+
+// Canonical task titles used by stub plans and defaults. Keep verb-first.
+const (
+    TaskTitleSetupDB              = "Setup DB"
+    TaskTitleMigrateSchema        = "Migrate Schema"
+    TaskTitleImplementAPIHandlers = "Implement API Handlers"
 )
 
 // DocLoader loads tasks/features from a specification document.
@@ -147,65 +154,66 @@ func stubPlan() ([]m.Task, []FeatureSummary) {
         featureID string
         title     string
     }{
-        {"T001", "F001", "Setup DB"},
-        {"T002", "F001", "Migrate Schema"},
-        {"T003", "F001", "Implement API Handlers"},
+        {"T001", "F001", TaskTitleSetupDB},
+        {"T002", "F001", TaskTitleMigrateSchema},
+        {"T003", "F001", TaskTitleImplementAPIHandlers},
     }
 	tasks := make([]m.Task, 0, len(base))
     for _, spec := range base {
         task := m.Task{ID: spec.id, FeatureID: spec.featureID, Title: spec.title}
         switch strings.ToLower(spec.title) {
-        case "setup db":
+        case strings.ToLower(TaskTitleSetupDB):
             task.Duration = m.DurationPERT{Optimistic: 1, MostLikely: 2.5, Pessimistic: 4}
-        case "migrate schema":
+        case strings.ToLower(TaskTitleMigrateSchema):
             task.Duration = m.DurationPERT{Optimistic: 1, MostLikely: 3.5, Pessimistic: 8}
-        case "implement api handlers":
+        case strings.ToLower(TaskTitleImplementAPIHandlers):
             task.Duration = m.DurationPERT{Optimistic: 2, MostLikely: 6, Pessimistic: 12}
         default:
             task.Duration = m.DurationPERT{Optimistic: 1, MostLikely: 2, Pessimistic: 3}
         }
         applyTaskDefaults(&task)
         tasks = append(tasks, task)
     }
-	return tasks, []FeatureSummary{{ID: "F001", Title: "Core DB + API"}}
+    return tasks, []FeatureSummary{{ID: "F001", Title: "Core DB + API"}}
 }
 
 func applyTaskDefaults(task *m.Task) {
-    if len(task.AcceptanceChecks) == 0 {
-        lower := strings.ToLower(task.Title)
-        switch {
-        case strings.Contains(lower, "setup db"):
-            task.AcceptanceChecks = []m.AcceptanceCheck{{
-                Type: "command",
-                // Portable guard: ensure DB_DSN present before trying psql
-                Cmd:  `sh -c 'test -n "$DB_DSN" && psql "$DB_DSN" -c "\\conninfo" >/dev/null 2>&1'`,
-                Timeout: 10,
-            }}
-        case strings.Contains(lower, "migrate schema"):
-            task.AcceptanceChecks = []m.AcceptanceCheck{{
-                Type: "command",
-                // Guard for command presence, then check status
-                Cmd:  `sh -c 'command -v db/migrate >/dev/null 2>&1 && db/migrate status | grep -q Applied'`,
-                Timeout: 15,
-            }}
-        case strings.Contains(lower, "api handler"):
-            task.AcceptanceChecks = []m.AcceptanceCheck{{
-                Type: "command",
-                // POSIX fallback expansion for API_BASE default
-                Cmd:  `sh -c 'curl -fsS "${API_BASE-http://localhost:8080}/healthz" >/dev/null'`,
-                Timeout: 10,
-            }}
-        default:
-            // No safe generic check — force authors to provide a real acceptance by failing fast.
-            task.AcceptanceChecks = []m.AcceptanceCheck{{Type: "command", Cmd: `sh -c 'echo "missing acceptance checks" >&2; exit 1'`, Timeout: 5}}
-        }
-    }
-    if task.DurationUnit == "" {
-        task.DurationUnit = "hours"
+    applyAcceptanceDefaults(task)
+    if task.DurationUnit == "" { task.DurationUnit = "hours" }
+    applyLoggingDefaults(task)
+    seedEvidenceDefaults(task)
+}
+
+func applyAcceptanceDefaults(task *m.Task) {
+    if len(task.AcceptanceChecks) != 0 { return }
+    lower := strings.ToLower(task.Title)
+    switch {
+    case strings.Contains(lower, "setup db"):
+        task.AcceptanceChecks = []m.AcceptanceCheck{{
+            Type:    "command",
+            Cmd:     `sh -c 'test -n "$DB_DSN" && psql "$DB_DSN" -c "\\conninfo" >/dev/null 2>&1'`,
+            Timeout: 10,
+        }}
+    case strings.Contains(lower, "migrate schema"):
+        task.AcceptanceChecks = []m.AcceptanceCheck{{
+            Type:    "command",
+            Cmd:     `sh -c 'command -v db/migrate >/dev/null 2>&1 && db/migrate status | grep -q Applied'`,
+            Timeout: 15,
+        }}
+    case strings.Contains(lower, "api handler"):
+        task.AcceptanceChecks = []m.AcceptanceCheck{{
+            Type:    "command",
+            Cmd:     `sh -c 'curl -fsS "${API_BASE-http://localhost:8080}/healthz" >/dev/null'`,
+            Timeout: 10,
+        }}
+    default:
+        // Force authors to provide an acceptance by failing fast.
+        task.AcceptanceChecks = []m.AcceptanceCheck{{Type: "command", Cmd: `sh -c 'echo "missing acceptance checks" >&2; exit 1'`, Timeout: 5}}
     }
-    // Execution logging defaults + light variation to improve coverage realism
+}
+
+func applyLoggingDefaults(task *m.Task) {
     wasEmpty := task.ExecutionLogging.Format == ""
-    // Seed required fields once; make it idempotent and include error fields for JSONL schema.
     baseFields := []string{"timestamp", "task_id", "step", "status", "message", "error", "error_type", "error_details"}
     if len(task.ExecutionLogging.RequiredFields) == 0 {
         task.ExecutionLogging.RequiredFields = append([]string{}, baseFields...)
@@ -221,57 +229,50 @@ func applyTaskDefaults(task *m.Task) {
     case strings.Contains(lower, "migrate schema"):
         ensureField("migration_version")
     case strings.Contains(lower, "api handler"):
-        if wasEmpty {
-            task.ExecutionLogging.Format = "JSON"
-        } else {
-            // leave user-provided non-empty format untouched
-        }
+        if wasEmpty { task.ExecutionLogging.Format = "JSON" }
         ensureField("service_version")
-    default:
-        // already seeded above
     }
     if wasEmpty && task.ExecutionLogging.Format == "" {
         task.ExecutionLogging.Format = "JSONL"
     }
-    // Do not clobber explicit idempotency; authors must set it in spec/doc.
+}
 
-    // Seed a minimal evidence entry if none provided to satisfy coverage in stub
-    // plans; reference nearby docs/code so reviewers can trace it.
-    if len(task.Evidence) == 0 {
-        switch {
-        case strings.Contains(lower, "setup db"):
-            task.Evidence = append(task.Evidence, m.Evidence{
-                Type:       "code_analysis",
-                Source:     "planner/internal/app/plan/doc_loader.go#applyTaskDefaults",
-                Excerpt:    "DB setup defaults: acceptance check via psql; logging requires db_response_time",
-                Confidence: 0.9,
-                Rationale:  "stub default grounded in code defaults",
-            })
-        case strings.Contains(lower, "migrate schema"):
-            task.Evidence = append(task.Evidence, m.Evidence{
-                Type:       "docs",
-                Source:     "docs/go-architecture.md",
-                Excerpt:    "Migrations modeled as technical edges with ordering",
-                Confidence: 0.85,
-                Rationale:  "ordering requirement documented in architecture",
-            })
-        case strings.Contains(lower, "api handler"):
-            task.Evidence = append(task.Evidence, m.Evidence{
-                Type:       "docs",
-                Source:     "README.md",
-                Excerpt:    "CLI/demo health endpoint used for acceptance (/healthz)",
-                Confidence: 0.8,
-                Rationale:  "acceptance derived from documented demo",
-            })
-        default:
-            task.Evidence = append(task.Evidence, m.Evidence{
-                Type:       "docs",
-                Source:     "docs/v8/v8.md",
-                Excerpt:    "Tasks require machine-verifiable acceptance and traceable evidence",
-                Confidence: 0.75,
-                Rationale:  "fallback evidence for stub plans",
-            })
-        }
+func seedEvidenceDefaults(task *m.Task) {
+    if len(task.Evidence) != 0 { return }
+    lower := strings.ToLower(task.Title)
+    switch {
+    case strings.Contains(lower, "setup db"):
+        task.Evidence = append(task.Evidence, m.Evidence{
+            Type:       "code_analysis",
+            Source:     "planner/internal/app/plan/doc_loader.go#applyTaskDefaults",
+            Excerpt:    "DB setup defaults: acceptance check via psql; logging requires db_response_time",
+            Confidence: 0.9,
+            Rationale:  "stub default grounded in code defaults",
+        })
+    case strings.Contains(lower, "migrate schema"):
+        task.Evidence = append(task.Evidence, m.Evidence{
+            Type:       "docs",
+            Source:     "docs/go-architecture.md",
+            Excerpt:    "Migrations modeled as technical edges with ordering",
+            Confidence: 0.85,
+            Rationale:  "ordering requirement documented in architecture",
+        })
+    case strings.Contains(lower, "api handler"):
+        task.Evidence = append(task.Evidence, m.Evidence{
+            Type:       "docs",
+            Source:     "README.md",
+            Excerpt:    "CLI/demo health endpoint used for acceptance (/healthz)",
+            Confidence: 0.8,
+            Rationale:  "acceptance derived from documented demo",
+        })
+    default:
+        task.Evidence = append(task.Evidence, m.Evidence{
+            Type:       "docs",
+            Source:     "docs/v8/v8.md",
+            Excerpt:    "Tasks require machine-verifiable acceptance and traceable evidence",
+            Confidence: 0.75,
+            Rationale:  "fallback evidence for stub plans",
+        })
     }
 }
 

planner/internal/app/plan/doc_loader_test.go

@@ -111,9 +111,9 @@ func TestMarkdownDocLoaderRejectsDuplicateTitles(t *testing.T) {
 }
 
 func TestApplyTaskDefaults(t *testing.T) {
-	task := m.Task{}
-	task.Duration = m.DurationPERT{Optimistic: 1, MostLikely: 2, Pessimistic: 3}
-	applyTaskDefaults(&task)
+    task := m.Task{}
+    task.Duration = m.DurationPERT{Optimistic: 1, MostLikely: 2, Pessimistic: 3}
+    applyTaskDefaults(&task)
 	if len(task.AcceptanceChecks) == 0 {
 		t.Fatalf("expected default acceptance check")
 	}
@@ -136,6 +136,23 @@ func TestApplyTaskDefaults(t *testing.T) {
 	}
 }
 
+func TestApplyTaskDefaultsIdempotent(t *testing.T) {
+    task := m.Task{Title: TaskTitleImplementAPIHandlers}
+    task.Duration = m.DurationPERT{Optimistic: 2, MostLikely: 6, Pessimistic: 12}
+    applyTaskDefaults(&task)
+    // capture lengths
+    rf1 := len(task.ExecutionLogging.RequiredFields)
+    ev1 := len(task.Evidence)
+    // call twice
+    applyTaskDefaults(&task)
+    if len(task.ExecutionLogging.RequiredFields) != rf1 {
+        t.Fatalf("required fields duplicated: %d -> %d", rf1, len(task.ExecutionLogging.RequiredFields))
+    }
+    if len(task.Evidence) != ev1 {
+        t.Fatalf("evidence duplicated: %d -> %d", ev1, len(task.Evidence))
+    }
+}
+
 func TestResolveTaskIDAllowsLongerIDs(t *testing.T) {
 	got := resolveTaskID("T12345", map[string]string{"task": "T001"})
 	if got != "T12345" {