From 2c41ad12b34cc42ea7318f13b52f05e18dde3b82 Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Thu, 27 Aug 2026 05:27:58 -0400 Subject: [PATCH 1/2] feat: establish canonical task dependency reads Model repository-global depends_on data, strict graph health, SCC cycle attribution, derived gate state, and distinct causal/action blocker projections. Close generic write bypasses, make safe legacy debt advisory, preserve path-faithful diagnostics, and record the two adversarial audit dispositions and downstream contracts. --- internal/cli/lint.go | 11 +- internal/cli/lint_test.go | 63 + internal/cli/render/render.go | 26 +- .../golden/audit_findings_json.golden | 2 +- .../golden/audit_findings_open_json.golden | 2 +- .../testdata/golden/audit_info_json.golden | 2 +- .../testdata/golden/audit_path_json.golden | 2 +- .../cli/testdata/golden/board_json.golden | 2 +- .../testdata/golden/config_show_json.golden | 2 +- .../cli/testdata/golden/epic_list_json.golden | 2 +- .../cli/testdata/golden/epic_path_json.golden | 2 +- .../cli/testdata/golden/epic_show_json.golden | 2 +- internal/cli/testdata/golden/lint_json.golden | 2 +- .../cli/testdata/golden/schema_json.golden | 2 +- .../testdata/golden/schema_jsonschema.golden | 13 +- .../testdata/golden/schema_task_json.golden | 2 +- .../testdata/golden/status_all_json.golden | 2 +- .../cli/testdata/golden/status_json.golden | 2 +- .../golden/task_acceptance_json.golden | 2 +- .../cli/testdata/golden/task_info_json.golden | 2 +- .../cli/testdata/golden/task_list_json.golden | 2 +- .../cli/testdata/golden/task_path_json.golden | 2 +- .../cli/testdata/golden/task_show_json.golden | 2 +- .../testdata/golden/template_list_json.golden | 2 +- .../golden/template_show_security_json.golden | 2 +- .../planning/tasks/6fjangd7kvh0-alpha-task.md | 1 + internal/core/dependency_graph.go | 1098 +++++++++++++++++ internal/core/dependency_graph_test.go | 482 ++++++++ internal/core/service.go | 93 ++ internal/core/service_task.go | 28 + internal/core/setfields_coercion_test.go | 17 + internal/domain/entity.go | 1 + internal/domain/fields.go | 14 + internal/domain/lint.go | 15 +- internal/domain/schema_test.go | 2 +- internal/domain/task.go | 14 + internal/store/create.go | 9 + internal/store/dependency_persistence_test.go | 146 +++ internal/store/edit.go | 82 +- internal/store/fix.go | 39 + internal/store/fix_test.go | 20 + internal/store/fsstore.go | 5 + internal/store/setfields_test.go | 13 + internal/wire/dto.go | 10 + internal/wire/dto_test.go | 13 + internal/wire/schema_comments.json | 4 + internal/wire/wire.go | 9 +- .../adrs/0006-adopt-threads-as-task-dags.md | 66 +- ...nonical-task-dependency-read-foundation.md | 245 ++++ ...-task-dependency-read-foundation-claude.md | 836 +++++++++++++ ...ask-dependencies-and-strict-graph-reads.md | 111 +- ...aph-mutations-portable-and-serializable.md | 4 + ...-dependency-mutations-and-graph-queries.md | 9 + 53 files changed, 3490 insertions(+), 51 deletions(-) create mode 100644 internal/core/dependency_graph.go create mode 100644 internal/core/dependency_graph_test.go create mode 100644 internal/store/dependency_persistence_test.go create mode 100644 planning/audits/6g417v97bx8s-2026-08-26-canonical-task-dependency-read-foundation.md create mode 100644 planning/audits/6g41amrnje2j-2026-08-26-canonical-task-dependency-read-foundation-claude.md diff --git a/internal/cli/lint.go b/internal/cli/lint.go index a48f7c46..cfa7d269 100644 --- a/internal/cli/lint.go +++ b/internal/cli/lint.go @@ -6,6 +6,7 @@ import ( "github.com/spf13/cobra" "github.com/andy-esch/taskflow/internal/cli/render" + "github.com/andy-esch/taskflow/internal/core" "github.com/andy-esch/taskflow/internal/domain" ) @@ -59,9 +60,10 @@ func runLint(app *App, links bool) error { fmt.Fprintf(app.Out, "%s all active tasks and epics pass lint\n", app.Style.Green("✔")) } } - if len(results)+len(problems) > 0 { + blocking := core.BlockingLintResultCount(results) + if blocking+len(problems) > 0 { return fmt.Errorf("%w: %d item(s) with issues, %d unreadable file(s)", - domain.ErrValidation, len(results), len(problems)) + domain.ErrValidation, blocking, len(problems)) } return nil } @@ -108,9 +110,10 @@ func runLintFix(app *App, dryRun bool) error { render.FixHuman(app.Out, app.Style, results, results2, dryRun) render.ProblemsHuman(app.ErrOut, app.Style, problems) } - if len(results2)+len(problems) > 0 { + blocking := core.BlockingLintResultCount(results2) + if blocking+len(problems) > 0 { return fmt.Errorf("%w: %d item(s) still with issues, %d unreadable file(s)", - domain.ErrValidation, len(results2), len(problems)) + domain.ErrValidation, blocking, len(problems)) } return nil } diff --git a/internal/cli/lint_test.go b/internal/cli/lint_test.go index e998a031..64940bbc 100644 --- a/internal/cli/lint_test.go +++ b/internal/cli/lint_test.go @@ -75,6 +75,69 @@ func TestLint_Clean(t *testing.T) { } } +func TestLintReportsLegacyAndCanonicalDependencyDefects(t *testing.T) { + r := testutil.NewRepo(t) + r.Epic("01-e.md", "---\nstatus: active\npriority: high\ndescription: e\n---\n# E\n") + targetID := testutil.TaskID("target") + dependentID := testutil.TaskID("dependent") + selfID := testutil.TaskID("self") + r.Task("completed", "target.md", "---\nid: "+targetID+"\nstatus: completed\nepic: 01-e\n---\n# target\n") + r.Task("completed", "dependent.md", "---\nid: "+dependentID+"\nstatus: completed\nepic: 01-e\nblocked_by: [target]\n---\n# dependent\n") + r.Task("completed", "self.md", "---\nid: "+selfID+"\nstatus: completed\nepic: 01-e\ndepends_on: ["+selfID+"]\n---\n# self\n") + + out, err := runRootRC(t, "-C", r.Root, "lint") + if err == nil || ExitCode(err) != 11 { + t.Fatalf("dependency defects must fail ordinary lint with exit 11, got %v", err) + } + for _, want := range []string{ + "legacy dependency field", targetID, "guarded dependency operations", + "cannot depend on itself", "advisory finding", + } { + if !strings.Contains(out, want) { + t.Errorf("lint output missing %q:\n%s", want, out) + } + } +} + +func TestLintResolvedLegacyDependencyIsAdvisoryWithExitZero(t *testing.T) { + r := testutil.NewRepo(t) + r.Epic("01-e.md", "---\nstatus: active\npriority: high\ndescription: e\n---\n# E\n") + targetID := testutil.TaskID("target") + dependentID := testutil.TaskID("dependent") + r.Task("completed", "target.md", "---\nid: "+targetID+"\nstatus: completed\nepic: 01-e\n---\n# target\n") + r.Task("completed", "dependent.md", "---\nid: "+dependentID+"\nstatus: completed\nepic: 01-e\nblocked_by: [target]\n---\n# dependent\n") + + human, err := runRootRC(t, "-C", r.Root, "lint") + if err != nil { + t.Fatalf("safe legacy advisory must exit zero: %v\n%s", err, human) + } + if !strings.Contains(human, "legacy dependency field") || !strings.Contains(human, "1 advisory finding") { + t.Fatalf("human advisory output =\n%s", human) + } + jsonOut, err := runRootRC(t, "-C", r.Root, "lint", "--json") + if err != nil { + t.Fatalf("JSON advisory must exit zero: %v\n%s", err, jsonOut) + } + if !strings.Contains(jsonOut, `"severity":"advisory"`) { + t.Fatalf("JSON advisory severity missing:\n%s", jsonOut) + } +} + +func TestLintUnsafeLegacyDependencyRemainsValidationError(t *testing.T) { + r := testutil.NewRepo(t) + r.Epic("01-e.md", "---\nstatus: active\npriority: high\ndescription: e\n---\n# E\n") + selfID := testutil.TaskID("self") + r.Task("completed", "self.md", "---\nid: "+selfID+"\nstatus: completed\nepic: 01-e\nblocked_by: [self]\n---\n# self\n") + + out, err := runRootRC(t, "-C", r.Root, "lint", "--json") + if err == nil || ExitCode(err) != 11 { + t.Fatalf("unsafe legacy projection must exit 11: %v\n%s", err, out) + } + if !strings.Contains(out, "structurally unsafe") || strings.Contains(out, `"severity":"advisory"`) { + t.Fatalf("unsafe legacy output =\n%s", out) + } +} + // TestLint_FlagsNonNNEpicFailOpen pins the epic NN- gate end-to-end: a non-NN- // epic is lint-flagged (exit 11, naming the convention) yet STILL lists/resolves — the // fail-open contract, not a dropped FileProblem. diff --git a/internal/cli/render/render.go b/internal/cli/render/render.go index 8c406a95..72154786 100644 --- a/internal/cli/render/render.go +++ b/internal/cli/render/render.go @@ -9,6 +9,7 @@ package render import ( "fmt" "io" + "sort" "strings" "unicode" @@ -93,6 +94,11 @@ func TaskShowHuman(w io.Writer, st Style, t domain.Task, body string) error { if len(t.Tags) > 0 { field("tags", strings.Join(t.Tags, ", ")) } + if len(t.DependsOn) > 0 { + deps := append([]string(nil), t.DependsOn...) + sort.Strings(deps) + field("depends on", strings.Join(deps, ", ")) + } if t.Description != "" { field("description", t.Description) } @@ -810,14 +816,28 @@ func ProblemsHuman(w io.Writer, st Style, problems []domain.FileProblem) { // entity for the footer ("task", "audit") since the same result/render shape backs // both `lint` and `audit lint`. func LintHuman(w io.Writer, st Style, results []core.LintResult, noun string) { + blockingItems, advisories := 0, 0 for _, r := range results { fmt.Fprintf(w, "%s\n", st.Bold(r.Slug)) for _, iss := range r.Issues { - fmt.Fprintf(w, " %s %s\n", st.Red(iss.Field+":"), iss.Message) + field := st.Red(iss.Field + ":") + if !iss.Blocking() { + field = st.Warn(iss.Field + ":") + advisories++ + } + fmt.Fprintf(w, " %s %s\n", field, iss.Message) + } + if r.Blocking() { + blockingItems++ } } - if len(results) > 0 { - fmt.Fprintf(w, "\n%s\n", st.Dim(fmt.Sprintf("%d %s(s) with issues", len(results), noun))) + switch { + case blockingItems > 0 && advisories > 0: + fmt.Fprintf(w, "\n%s\n", st.Dim(fmt.Sprintf("%d %s(s) with issues · %d advisory finding(s)", blockingItems, noun, advisories))) + case blockingItems > 0: + fmt.Fprintf(w, "\n%s\n", st.Dim(fmt.Sprintf("%d %s(s) with issues", blockingItems, noun))) + case advisories > 0: + fmt.Fprintf(w, "\n%s\n", st.Dim(fmt.Sprintf("%d advisory finding(s)", advisories))) } } diff --git a/internal/cli/testdata/golden/audit_findings_json.golden b/internal/cli/testdata/golden/audit_findings_json.golden index a678e0d6..8dc8b1cd 100644 --- a/internal/cli/testdata/golden/audit_findings_json.golden +++ b/internal/cli/testdata/golden/audit_findings_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","findings":[{"audit":"2026-01-02-fixture-area","bucket":"open","code":"S1","title":"Tighten the fixture gateway","status":"open","component":"fixturepipe","effort":"S","urgency":"soon"},{"audit":"2026-01-02-fixture-area","bucket":"open","code":"H1","title":"Fix the fixture bypass","status":"fixed","component":"auth","effort":"M","urgency":"acute","status_decoration":"2026-01-03 (PR #1)"}]} +{"schema_version":"1.49","findings":[{"audit":"2026-01-02-fixture-area","bucket":"open","code":"S1","title":"Tighten the fixture gateway","status":"open","component":"fixturepipe","effort":"S","urgency":"soon"},{"audit":"2026-01-02-fixture-area","bucket":"open","code":"H1","title":"Fix the fixture bypass","status":"fixed","component":"auth","effort":"M","urgency":"acute","status_decoration":"2026-01-03 (PR #1)"}]} diff --git a/internal/cli/testdata/golden/audit_findings_open_json.golden b/internal/cli/testdata/golden/audit_findings_open_json.golden index 840f6ba1..99f692b9 100644 --- a/internal/cli/testdata/golden/audit_findings_open_json.golden +++ b/internal/cli/testdata/golden/audit_findings_open_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","findings":[{"audit":"2026-01-02-fixture-area","bucket":"open","code":"S1","title":"Tighten the fixture gateway","status":"open","component":"fixturepipe","effort":"S","urgency":"soon"}]} +{"schema_version":"1.49","findings":[{"audit":"2026-01-02-fixture-area","bucket":"open","code":"S1","title":"Tighten the fixture gateway","status":"open","component":"fixturepipe","effort":"S","urgency":"soon"}]} diff --git a/internal/cli/testdata/golden/audit_info_json.golden b/internal/cli/testdata/golden/audit_info_json.golden index b61474ee..dc96b624 100644 --- a/internal/cli/testdata/golden/audit_info_json.golden +++ b/internal/cli/testdata/golden/audit_info_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","audit_info":{"id":"6fjangd7kvh3","slug":"2026-01-02-fixture-area","bucket":"open","path":"/audits/6fjangd7kvh3-2026-01-02-fixture-area.md","findings":{"total":2,"open":1,"in_progress":0,"done":1,"dropped":0}}} +{"schema_version":"1.49","audit_info":{"id":"6fjangd7kvh3","slug":"2026-01-02-fixture-area","bucket":"open","path":"/audits/6fjangd7kvh3-2026-01-02-fixture-area.md","findings":{"total":2,"open":1,"in_progress":0,"done":1,"dropped":0}}} diff --git a/internal/cli/testdata/golden/audit_path_json.golden b/internal/cli/testdata/golden/audit_path_json.golden index 1fe60089..de77fd2f 100644 --- a/internal/cli/testdata/golden/audit_path_json.golden +++ b/internal/cli/testdata/golden/audit_path_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","path":"/audits/6fjangd7kvh3-2026-01-02-fixture-area.md"} +{"schema_version":"1.49","path":"/audits/6fjangd7kvh3-2026-01-02-fixture-area.md"} diff --git a/internal/cli/testdata/golden/board_json.golden b/internal/cli/testdata/golden/board_json.golden index b77b7e1d..a86a6e66 100644 --- a/internal/cli/testdata/golden/board_json.golden +++ b/internal/cli/testdata/golden/board_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","columns":[{"status":"next-up","tasks":[]},{"status":"ready-to-start","tasks":[{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","description":"A fully specified ready-to-start task for golden snapshots","effort":"S","tier":2,"priority":"high","autonomy_level":3,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli","testing"]}]},{"status":"in-progress","tasks":[{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]}]}]} +{"schema_version":"1.49","columns":[{"status":"next-up","tasks":[]},{"status":"ready-to-start","tasks":[{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","description":"A fully specified ready-to-start task for golden snapshots","effort":"S","tier":2,"priority":"high","autonomy_level":3,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli","testing"],"depends_on":["6fjangd7kvh2"]}]},{"status":"in-progress","tasks":[{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]}]}]} diff --git a/internal/cli/testdata/golden/config_show_json.golden b/internal/cli/testdata/golden/config_show_json.golden index e162f057..4e6f99c0 100644 --- a/internal/cli/testdata/golden/config_show_json.golden +++ b/internal/cli/testdata/golden/config_show_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","repository":{"path":"/.tskflwctl.toml","dir":"","planning_root":"","mode":"scaffold","taskflow_root":".","id":"6fjangd7kvhz","tracked_repos":[],"theme":{"name":"catppuccin"},"pager":{"enabled":false,"command":"delta"},"pending_migrations":[]},"user":{"path":"/config.toml","exists":false,"theme":{},"pager":{},"registry_path":"/spaces.toml"},"effective":{"theme":{"value":"catppuccin","source":"repository"},"pager_enabled":{"value":false,"source":"repository"},"pager_command":{"value":"delta","source":"repository"}}} +{"schema_version":"1.49","repository":{"path":"/.tskflwctl.toml","dir":"","planning_root":"","mode":"scaffold","taskflow_root":".","id":"6fjangd7kvhz","tracked_repos":[],"theme":{"name":"catppuccin"},"pager":{"enabled":false,"command":"delta"},"pending_migrations":[]},"user":{"path":"/config.toml","exists":false,"theme":{},"pager":{},"registry_path":"/spaces.toml"},"effective":{"theme":{"value":"catppuccin","source":"repository"},"pager_enabled":{"value":false,"source":"repository"},"pager_command":{"value":"delta","source":"repository"}}} diff --git a/internal/cli/testdata/golden/epic_list_json.golden b/internal/cli/testdata/golden/epic_list_json.golden index d1eb629d..e44a4ddc 100644 --- a/internal/cli/testdata/golden/epic_list_json.golden +++ b/internal/cli/testdata/golden/epic_list_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","epics":[{"id":"01-fixture-epic","status":"active","description":"A fixture epic for golden snapshots","priority":"high","created":"2026-01-01","tags":["fixture"],"total":3,"done":1,"open":2,"percent":33,"deprecated":0,"liveness":"working"}]} +{"schema_version":"1.49","epics":[{"id":"01-fixture-epic","status":"active","description":"A fixture epic for golden snapshots","priority":"high","created":"2026-01-01","tags":["fixture"],"total":3,"done":1,"open":2,"percent":33,"deprecated":0,"liveness":"working"}]} diff --git a/internal/cli/testdata/golden/epic_path_json.golden b/internal/cli/testdata/golden/epic_path_json.golden index d2eefc8f..ccdd00a2 100644 --- a/internal/cli/testdata/golden/epic_path_json.golden +++ b/internal/cli/testdata/golden/epic_path_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","path":"/epics/01-fixture-epic.md"} +{"schema_version":"1.49","path":"/epics/01-fixture-epic.md"} diff --git a/internal/cli/testdata/golden/epic_show_json.golden b/internal/cli/testdata/golden/epic_show_json.golden index 2e7f823a..97130d84 100644 --- a/internal/cli/testdata/golden/epic_show_json.golden +++ b/internal/cli/testdata/golden/epic_show_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","epic":{"id":"01-fixture-epic","status":"active","description":"A fixture epic for golden snapshots","priority":"high","created":"2026-01-01","tags":["fixture"]},"tasks":[{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","description":"A fully specified ready-to-start task for golden snapshots","effort":"S","tier":2,"priority":"high","autonomy_level":3,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli","testing"]},{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]},{"id":"6fjangd7kvh2","slug":"gamma-task","status":"completed","epic":"01-fixture-epic","description":"A completed fixture task","tier":1,"priority":"low","created":"2026-01-01","updated_at":"2026-01-02","tags":["docs"]}],"body":"# Fixture Epic\n\nThe epic that the fixture tasks roll up into.\n"} +{"schema_version":"1.49","epic":{"id":"01-fixture-epic","status":"active","description":"A fixture epic for golden snapshots","priority":"high","created":"2026-01-01","tags":["fixture"]},"tasks":[{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","description":"A fully specified ready-to-start task for golden snapshots","effort":"S","tier":2,"priority":"high","autonomy_level":3,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli","testing"],"depends_on":["6fjangd7kvh2"]},{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]},{"id":"6fjangd7kvh2","slug":"gamma-task","status":"completed","epic":"01-fixture-epic","description":"A completed fixture task","tier":1,"priority":"low","created":"2026-01-01","updated_at":"2026-01-02","tags":["docs"]}],"body":"# Fixture Epic\n\nThe epic that the fixture tasks roll up into.\n"} diff --git a/internal/cli/testdata/golden/lint_json.golden b/internal/cli/testdata/golden/lint_json.golden index 0a77f4c0..7610c12b 100644 --- a/internal/cli/testdata/golden/lint_json.golden +++ b/internal/cli/testdata/golden/lint_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","unreadable":[],"issues":[]} +{"schema_version":"1.49","unreadable":[],"issues":[]} diff --git a/internal/cli/testdata/golden/schema_json.golden b/internal/cli/testdata/golden/schema_json.golden index 2e165d6f..7653a64f 100644 --- a/internal/cli/testdata/golden/schema_json.golden +++ b/internal/cli/testdata/golden/schema_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","statuses":[{"value":"next-up","active":true},{"value":"ready-to-start","active":true},{"value":"in-progress","active":true},{"value":"completed","active":false},{"value":"deprecated","active":false},{"value":"deferred","active":false}],"epic_statuses":["active","retired","deprecated"],"audit_buckets":["open","closed","deferred"],"finding_statuses":["deferred","fixed","in-progress","open","superseded","tracked","wontfix"],"criterion_states":["deferred","n/a","tracked","wontfix"],"task_fields":[{"name":"audit_sources","type":"list"},{"name":"audited","type":"date"},{"name":"autonomy_level","type":"int"},{"name":"blocked_by","type":"list"},{"name":"blocks","type":"list"},{"name":"completed_at","type":"date"},{"name":"created","type":"date"},{"name":"deferred_at","type":"date"},{"name":"dependencies","type":"list"},{"name":"deprecated_at","type":"date"},{"name":"description","type":"string"},{"name":"effort","type":"string"},{"name":"epic","type":"string"},{"name":"priority","type":"string"},{"name":"projects","type":"list"},{"name":"related_tasks","type":"list"},{"name":"revisit_at","type":"date"},{"name":"started_at","type":"date"},{"name":"status","type":"string"},{"name":"tags","type":"list"},{"name":"tier","type":"int"},{"name":"updated_at","type":"date"}],"epic_fields":["created","description","priority","status","tags"],"research_fields":[{"name":"created","type":"date"},{"name":"description","type":"string"},{"name":"tags","type":"list"},{"name":"updated_at","type":"date"}],"exit_codes":[{"code":10,"name":"not-found"},{"code":11,"name":"validation"},{"code":13,"name":"ambiguous"},{"code":14,"name":"conflict"}],"kinds":["task","epic","audit","research"]} +{"schema_version":"1.49","statuses":[{"value":"next-up","active":true},{"value":"ready-to-start","active":true},{"value":"in-progress","active":true},{"value":"completed","active":false},{"value":"deprecated","active":false},{"value":"deferred","active":false}],"epic_statuses":["active","retired","deprecated"],"audit_buckets":["open","closed","deferred"],"finding_statuses":["deferred","fixed","in-progress","open","superseded","tracked","wontfix"],"criterion_states":["deferred","n/a","tracked","wontfix"],"task_fields":[{"name":"audit_sources","type":"list"},{"name":"audited","type":"date"},{"name":"autonomy_level","type":"int"},{"name":"blocked_by","type":"list"},{"name":"blocks","type":"list"},{"name":"completed_at","type":"date"},{"name":"created","type":"date"},{"name":"deferred_at","type":"date"},{"name":"dependencies","type":"list"},{"name":"depends_on","type":"list"},{"name":"deprecated_at","type":"date"},{"name":"description","type":"string"},{"name":"effort","type":"string"},{"name":"epic","type":"string"},{"name":"priority","type":"string"},{"name":"projects","type":"list"},{"name":"related_tasks","type":"list"},{"name":"revisit_at","type":"date"},{"name":"started_at","type":"date"},{"name":"status","type":"string"},{"name":"tags","type":"list"},{"name":"tier","type":"int"},{"name":"updated_at","type":"date"}],"epic_fields":["created","description","priority","status","tags"],"research_fields":[{"name":"created","type":"date"},{"name":"description","type":"string"},{"name":"tags","type":"list"},{"name":"updated_at","type":"date"}],"exit_codes":[{"code":10,"name":"not-found"},{"code":11,"name":"validation"},{"code":13,"name":"ambiguous"},{"code":14,"name":"conflict"}],"kinds":["task","epic","audit","research"]} diff --git a/internal/cli/testdata/golden/schema_jsonschema.golden b/internal/cli/testdata/golden/schema_jsonschema.golden index 4879f753..6f5fed7f 100644 --- a/internal/cli/testdata/golden/schema_jsonschema.golden +++ b/internal/cli/testdata/golden/schema_jsonschema.golden @@ -1324,6 +1324,10 @@ }, "message": { "type": "string" + }, + "severity": { + "type": "string", + "description": "Severity is \"advisory\" for visible non-blocking debt and is omitted for\nordinary validation errors, preserving the existing wire shape." } }, "additionalProperties": false, @@ -2163,6 +2167,13 @@ }, "type": "array", "description": "topical tags" + }, + "depends_on": { + "items": { + "type": "string" + }, + "type": "array", + "description": "sorted stable task IDs that must be soundly completed before this task is ordinarily eligible to start" } }, "additionalProperties": false, @@ -2643,6 +2654,6 @@ ] } }, - "title": "tskflwctl --json output (schema_version 1.48)", + "title": "tskflwctl --json output (schema_version 1.49)", "description": "Each property of the root names a --json envelope and references its definition in $defs; validate a command's --json output against the matching definition." } diff --git a/internal/cli/testdata/golden/schema_task_json.golden b/internal/cli/testdata/golden/schema_task_json.golden index 19f3eeec..b671cb99 100644 --- a/internal/cli/testdata/golden/schema_task_json.golden +++ b/internal/cli/testdata/golden/schema_task_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","kind":"task","sections":["Objective","Acceptance criteria","Out of scope","Related"],"body_template":"\n# \u003ctitle\u003e\n\n## Objective\n\n\u003cwhy / what — one short paragraph\u003e\n\n## Acceptance criteria\n\n- [ ] \u003cobservable outcome\u003e\n\n## Out of scope\n\n- \u003cexplicitly excluded\u003e\n\n## Related\n\n- Epic [\u003cepic-id\u003e](../epics/\u003cepic-id\u003e.md)\n","fields":[{"name":"epic","type":"string","required":true,"description":"ID of the epic this task belongs to; must already exist.","example":"17-pm-go-cli"},{"name":"description","type":"string","required":false,"description":"One line summarizing the task (≤200 chars); required once next-up/in-progress.","example":"Add retry backoff to the Strava webhook"},{"name":"effort","type":"string","required":false,"description":"Rough size estimate (free-form).","example":"1-2 hours"},{"name":"tier","type":"int","required":false,"description":"Importance, 1 (highest) – 5 (lowest).","example":"2"},{"name":"priority","type":"string","required":false,"description":"One of: high | medium | low.","example":"medium"},{"name":"autonomy_level","type":"int","required":false,"description":"How autonomously this can be done, 1–5.","example":"3"},{"name":"tags","type":"list","required":true,"description":"At least one topical tag (required at creation).","example":"[cli, core]"}],"conventions":["status lives in frontmatter (authoritative) and is changed only via the lifecycle verbs (start/next/complete/…), which edit it in place — don't edit it directly.","description is a single line, ≤200 characters.","at least one tag is required at creation.","the filename slug is derived from the title; any title is accepted (colons, dashes, arrows, …) and the full title is kept as the body H1.","an acceptance criterion is `- [x]` (met) or `- [ ]` (not met). A not-met criterion may say WHY with a trailing `· **deferred:** reason` — one of: deferred | n/a | tracked | wontfix. A reason is required for those, and a checked criterion takes no suffix.","never hand-edit the acceptance criteria — `task ac` owns them: --check/--uncheck \u003cn\u003e, --defer/--wontfix/--tracked/--na \u003cn\u003e --reason \u003cwhy\u003e for a state, and --add/--remove/--replace \u003cn\u003e to change which criteria exist."],"templates":[{"kind":"task","name":"default","description":"Standard task scaffold: objective, acceptance criteria, out-of-scope, related epic."}]} +{"schema_version":"1.49","kind":"task","sections":["Objective","Acceptance criteria","Out of scope","Related"],"body_template":"\n# \u003ctitle\u003e\n\n## Objective\n\n\u003cwhy / what — one short paragraph\u003e\n\n## Acceptance criteria\n\n- [ ] \u003cobservable outcome\u003e\n\n## Out of scope\n\n- \u003cexplicitly excluded\u003e\n\n## Related\n\n- Epic [\u003cepic-id\u003e](../epics/\u003cepic-id\u003e.md)\n","fields":[{"name":"epic","type":"string","required":true,"description":"ID of the epic this task belongs to; must already exist.","example":"17-pm-go-cli"},{"name":"description","type":"string","required":false,"description":"One line summarizing the task (≤200 chars); required once next-up/in-progress.","example":"Add retry backoff to the Strava webhook"},{"name":"effort","type":"string","required":false,"description":"Rough size estimate (free-form).","example":"1-2 hours"},{"name":"tier","type":"int","required":false,"description":"Importance, 1 (highest) – 5 (lowest).","example":"2"},{"name":"priority","type":"string","required":false,"description":"One of: high | medium | low.","example":"medium"},{"name":"autonomy_level","type":"int","required":false,"description":"How autonomously this can be done, 1–5.","example":"3"},{"name":"tags","type":"list","required":true,"description":"At least one topical tag (required at creation).","example":"[cli, core]"}],"conventions":["status lives in frontmatter (authoritative) and is changed only via the lifecycle verbs (start/next/complete/…), which edit it in place — don't edit it directly.","depends_on is a sorted set of stable task IDs owned by the repository-global DAG; use `task depend add/remove` once available — generic `task set` and `task edit` cannot change it.","description is a single line, ≤200 characters.","at least one tag is required at creation.","the filename slug is derived from the title; any title is accepted (colons, dashes, arrows, …) and the full title is kept as the body H1.","an acceptance criterion is `- [x]` (met) or `- [ ]` (not met). A not-met criterion may say WHY with a trailing `· **deferred:** reason` — one of: deferred | n/a | tracked | wontfix. A reason is required for those, and a checked criterion takes no suffix.","never hand-edit the acceptance criteria — `task ac` owns them: --check/--uncheck \u003cn\u003e, --defer/--wontfix/--tracked/--na \u003cn\u003e --reason \u003cwhy\u003e for a state, and --add/--remove/--replace \u003cn\u003e to change which criteria exist."],"templates":[{"kind":"task","name":"default","description":"Standard task scaffold: objective, acceptance criteria, out-of-scope, related epic."}]} diff --git a/internal/cli/testdata/golden/status_all_json.golden b/internal/cli/testdata/golden/status_all_json.golden index 4abb3d03..07faab42 100644 --- a/internal/cli/testdata/golden/status_all_json.golden +++ b/internal/cli/testdata/golden/status_all_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","spaces":[{"id":"planning","planning_id":"6fjangd7kvhz","selected_entry_point":"planning","entry_points":[{"id":"implementation","path":"","verify_id":"6fjangd7kvhz","planning_id":"6fjangd7kvhz","role":"pointer","added":"2026-08-21","state":"ok","root":""},{"id":"planning","path":"","verify_id":"6fjangd7kvhz","planning_id":"6fjangd7kvhz","role":"direct","added":"2026-08-21","state":"ok","root":""},{"id":"missing","path":"","verify_id":"6fjangd7kvhz","planning_id":"6fjangd7kvhz","role":"unknown","added":"2026-08-21","state":"missing","detail":"not found at ","remedy":"`space forget missing`, then `space add \u003cnew-path\u003e --id missing`"}],"summary":{"counts":[{"status":"next-up","count":0},{"status":"ready-to-start","count":1},{"status":"in-progress","count":1},{"status":"completed","count":1},{"status":"deprecated","count":0},{"status":"deferred","count":0}],"in_progress":[{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]}],"epics":[{"id":"01-fixture-epic","status":"active","description":"A fixture epic for golden snapshots","priority":"high","created":"2026-01-01","tags":["fixture"],"total":3,"done":1,"open":2,"percent":33,"deprecated":0,"liveness":"working"}],"open_audits":[{"id":"6fjangd7kvh3","slug":"2026-01-02-fixture-area","bucket":"open","area":"fixture-area","date":"2026-01-02","findings":2,"open_findings":1,"in_progress_findings":0,"done_findings":1,"dropped_findings":0}],"findings":{"open":1,"in_progress":0,"by_urgency":[{"key":"soon","count":1}],"by_component":[{"key":"fixturepipe","count":1}]},"revisit_due":0,"bad_epic_status":0}}],"in_progress":[{"space":"planning","planning_id":"6fjangd7kvhz","task":{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]}}]} +{"schema_version":"1.49","spaces":[{"id":"planning","planning_id":"6fjangd7kvhz","selected_entry_point":"planning","entry_points":[{"id":"implementation","path":"","verify_id":"6fjangd7kvhz","planning_id":"6fjangd7kvhz","role":"pointer","added":"2026-08-21","state":"ok","root":""},{"id":"planning","path":"","verify_id":"6fjangd7kvhz","planning_id":"6fjangd7kvhz","role":"direct","added":"2026-08-21","state":"ok","root":""},{"id":"missing","path":"","verify_id":"6fjangd7kvhz","planning_id":"6fjangd7kvhz","role":"unknown","added":"2026-08-21","state":"missing","detail":"not found at ","remedy":"`space forget missing`, then `space add \u003cnew-path\u003e --id missing`"}],"summary":{"counts":[{"status":"next-up","count":0},{"status":"ready-to-start","count":1},{"status":"in-progress","count":1},{"status":"completed","count":1},{"status":"deprecated","count":0},{"status":"deferred","count":0}],"in_progress":[{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]}],"epics":[{"id":"01-fixture-epic","status":"active","description":"A fixture epic for golden snapshots","priority":"high","created":"2026-01-01","tags":["fixture"],"total":3,"done":1,"open":2,"percent":33,"deprecated":0,"liveness":"working"}],"open_audits":[{"id":"6fjangd7kvh3","slug":"2026-01-02-fixture-area","bucket":"open","area":"fixture-area","date":"2026-01-02","findings":2,"open_findings":1,"in_progress_findings":0,"done_findings":1,"dropped_findings":0}],"findings":{"open":1,"in_progress":0,"by_urgency":[{"key":"soon","count":1}],"by_component":[{"key":"fixturepipe","count":1}]},"revisit_due":0,"bad_epic_status":0}}],"in_progress":[{"space":"planning","planning_id":"6fjangd7kvhz","task":{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]}}]} diff --git a/internal/cli/testdata/golden/status_json.golden b/internal/cli/testdata/golden/status_json.golden index 5e789ef2..9cfffc45 100644 --- a/internal/cli/testdata/golden/status_json.golden +++ b/internal/cli/testdata/golden/status_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","counts":[{"status":"next-up","count":0},{"status":"ready-to-start","count":1},{"status":"in-progress","count":1},{"status":"completed","count":1},{"status":"deprecated","count":0},{"status":"deferred","count":0}],"in_progress":[{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]}],"epics":[{"id":"01-fixture-epic","status":"active","description":"A fixture epic for golden snapshots","priority":"high","created":"2026-01-01","tags":["fixture"],"total":3,"done":1,"open":2,"percent":33,"deprecated":0,"liveness":"working"}],"open_audits":[{"id":"6fjangd7kvh3","slug":"2026-01-02-fixture-area","bucket":"open","area":"fixture-area","date":"2026-01-02","findings":2,"open_findings":1,"in_progress_findings":0,"done_findings":1,"dropped_findings":0}],"findings":{"open":1,"in_progress":0,"by_urgency":[{"key":"soon","count":1}],"by_component":[{"key":"fixturepipe","count":1}]},"revisit_due":0,"bad_epic_status":0} +{"schema_version":"1.49","counts":[{"status":"next-up","count":0},{"status":"ready-to-start","count":1},{"status":"in-progress","count":1},{"status":"completed","count":1},{"status":"deprecated","count":0},{"status":"deferred","count":0}],"in_progress":[{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]}],"epics":[{"id":"01-fixture-epic","status":"active","description":"A fixture epic for golden snapshots","priority":"high","created":"2026-01-01","tags":["fixture"],"total":3,"done":1,"open":2,"percent":33,"deprecated":0,"liveness":"working"}],"open_audits":[{"id":"6fjangd7kvh3","slug":"2026-01-02-fixture-area","bucket":"open","area":"fixture-area","date":"2026-01-02","findings":2,"open_findings":1,"in_progress_findings":0,"done_findings":1,"dropped_findings":0}],"findings":{"open":1,"in_progress":0,"by_urgency":[{"key":"soon","count":1}],"by_component":[{"key":"fixturepipe","count":1}]},"revisit_due":0,"bad_epic_status":0} diff --git a/internal/cli/testdata/golden/task_acceptance_json.golden b/internal/cli/testdata/golden/task_acceptance_json.golden index d2251e7a..b593da74 100644 --- a/internal/cli/testdata/golden/task_acceptance_json.golden +++ b/internal/cli/testdata/golden/task_acceptance_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","slug":"alpha-task","acceptance":[{"index":1,"checked":true,"text":"the first criterion is done"},{"index":2,"checked":false,"text":"the second criterion is not"}]} +{"schema_version":"1.49","slug":"alpha-task","acceptance":[{"index":1,"checked":true,"text":"the first criterion is done"},{"index":2,"checked":false,"text":"the second criterion is not"}]} diff --git a/internal/cli/testdata/golden/task_info_json.golden b/internal/cli/testdata/golden/task_info_json.golden index 88494667..a810efc4 100644 --- a/internal/cli/testdata/golden/task_info_json.golden +++ b/internal/cli/testdata/golden/task_info_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","task_info":{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","path":"/tasks/6fjangd7kvh0-alpha-task.md","ac":{"checked":1,"total":2}}} +{"schema_version":"1.49","task_info":{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","path":"/tasks/6fjangd7kvh0-alpha-task.md","ac":{"checked":1,"total":2}}} diff --git a/internal/cli/testdata/golden/task_list_json.golden b/internal/cli/testdata/golden/task_list_json.golden index 3022aa52..6e8c0554 100644 --- a/internal/cli/testdata/golden/task_list_json.golden +++ b/internal/cli/testdata/golden/task_list_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","tasks":[{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","description":"A fully specified ready-to-start task for golden snapshots","effort":"S","tier":2,"priority":"high","autonomy_level":3,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli","testing"]},{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]},{"id":"6fjangd7kvh2","slug":"gamma-task","status":"completed","epic":"01-fixture-epic","description":"A completed fixture task","tier":1,"priority":"low","created":"2026-01-01","updated_at":"2026-01-02","tags":["docs"]}]} +{"schema_version":"1.49","tasks":[{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","description":"A fully specified ready-to-start task for golden snapshots","effort":"S","tier":2,"priority":"high","autonomy_level":3,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli","testing"],"depends_on":["6fjangd7kvh2"]},{"id":"6fjangd7kvh1","slug":"beta-task","status":"in-progress","epic":"01-fixture-epic","description":"An in-progress fixture task","effort":"M","tier":3,"priority":"medium","autonomy_level":2,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli"]},{"id":"6fjangd7kvh2","slug":"gamma-task","status":"completed","epic":"01-fixture-epic","description":"A completed fixture task","tier":1,"priority":"low","created":"2026-01-01","updated_at":"2026-01-02","tags":["docs"]}]} diff --git a/internal/cli/testdata/golden/task_path_json.golden b/internal/cli/testdata/golden/task_path_json.golden index cdd06a28..034adc4b 100644 --- a/internal/cli/testdata/golden/task_path_json.golden +++ b/internal/cli/testdata/golden/task_path_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","path":"/tasks/6fjangd7kvh0-alpha-task.md"} +{"schema_version":"1.49","path":"/tasks/6fjangd7kvh0-alpha-task.md"} diff --git a/internal/cli/testdata/golden/task_show_json.golden b/internal/cli/testdata/golden/task_show_json.golden index 3f33b601..abb8848a 100644 --- a/internal/cli/testdata/golden/task_show_json.golden +++ b/internal/cli/testdata/golden/task_show_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","task":{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","description":"A fully specified ready-to-start task for golden snapshots","effort":"S","tier":2,"priority":"high","autonomy_level":3,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli","testing"]},"body":"# Alpha Task\n\nBody for the alpha fixture task.\n\n## Acceptance criteria\n\n- [x] the first criterion is done\n- [ ] the second criterion is not\n"} +{"schema_version":"1.49","task":{"id":"6fjangd7kvh0","slug":"alpha-task","status":"ready-to-start","epic":"01-fixture-epic","description":"A fully specified ready-to-start task for golden snapshots","effort":"S","tier":2,"priority":"high","autonomy_level":3,"created":"2026-01-02","updated_at":"2026-01-03","tags":["cli","testing"],"depends_on":["6fjangd7kvh2"]},"body":"# Alpha Task\n\nBody for the alpha fixture task.\n\n## Acceptance criteria\n\n- [x] the first criterion is done\n- [ ] the second criterion is not\n"} diff --git a/internal/cli/testdata/golden/template_list_json.golden b/internal/cli/testdata/golden/template_list_json.golden index 8d7482c9..51e56dd2 100644 --- a/internal/cli/testdata/golden/template_list_json.golden +++ b/internal/cli/testdata/golden/template_list_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","templates":[{"kind":"task","name":"default","description":"Standard task scaffold: objective, acceptance criteria, out-of-scope, related epic."},{"kind":"epic","name":"default","description":"Standard epic scaffold: goal, why-it's-its-own-epic, out-of-scope."},{"kind":"audit","name":"default","description":"Standard audit scaffold: findings + candidate tasks."},{"kind":"audit","name":"security","description":"Security review: threat model, checklist, severity-tagged findings."},{"kind":"research","name":"default","description":"Standard research scaffold: question, findings, recommendation."}]} +{"schema_version":"1.49","templates":[{"kind":"task","name":"default","description":"Standard task scaffold: objective, acceptance criteria, out-of-scope, related epic."},{"kind":"epic","name":"default","description":"Standard epic scaffold: goal, why-it's-its-own-epic, out-of-scope."},{"kind":"audit","name":"default","description":"Standard audit scaffold: findings + candidate tasks."},{"kind":"audit","name":"security","description":"Security review: threat model, checklist, severity-tagged findings."},{"kind":"research","name":"default","description":"Standard research scaffold: question, findings, recommendation."}]} diff --git a/internal/cli/testdata/golden/template_show_security_json.golden b/internal/cli/testdata/golden/template_show_security_json.golden index 14a6b4cc..bf150a5b 100644 --- a/internal/cli/testdata/golden/template_show_security_json.golden +++ b/internal/cli/testdata/golden/template_show_security_json.golden @@ -1 +1 @@ -{"schema_version":"1.48","template":{"kind":"audit","name":"security","description":"Security review: threat model, checklist, severity-tagged findings."},"body":"\n# Security audit: \u003carea\u003e — \u003cdate\u003e\n\n\u003e Security review. Edit findings in place and flip each `**Status:**` as you work it.\n\n## Threat model\n\n- **Assets / trust boundaries:** \u003cwhat's worth protecting; where untrusted input crosses in\u003e\n- **Attacker \u0026 entry points:** \u003cwho, and through which surfaces\u003e\n\n## Review checklist\n\n- [ ] Authn / authz — every privileged path checks identity *and* permission\n- [ ] Input validation — untrusted input is parsed/escaped (injection, path traversal)\n- [ ] Secrets — no hard-coded creds; least-privilege tokens; nothing sensitive logged\n- [ ] Dependencies — known-vuln scan; versions pinned\n- [ ] Data at rest / in transit — encryption + safe defaults\n\n## Findings\n\n\u003c!-- One finding per issue, in this shape (un-fence it): --\u003e\n\n```\n#### H1. \u003ctitle\u003e · **Status:** open\n\n**File:** \u003cpath:line\u003e | **Component:** \u003ccomponent\u003e\n**Severity:** \u003ccritical|high|medium|low\u003e · **Effort:** \u003cXS|S|M|L\u003e · **Urgency:** \u003cacute|soon|eventually\u003e\n\n\u003cwhat's exploitable, the impact, and how\u003e\n\n**Recommendation:** \u003cthe fix\u003e\n```\n\n## Candidate tasks\n\n\u003c!-- Mirror each finding: ✅ done · ⚠️ partial · ⏳ open · ⛔ won't do --\u003e\n\n- ⏳ `tskflwctl task new \"\u003ctitle\u003e\" --epic \u003cid\u003e --tags security` — \u003cone line\u003e\n"} +{"schema_version":"1.49","template":{"kind":"audit","name":"security","description":"Security review: threat model, checklist, severity-tagged findings."},"body":"\n# Security audit: \u003carea\u003e — \u003cdate\u003e\n\n\u003e Security review. Edit findings in place and flip each `**Status:**` as you work it.\n\n## Threat model\n\n- **Assets / trust boundaries:** \u003cwhat's worth protecting; where untrusted input crosses in\u003e\n- **Attacker \u0026 entry points:** \u003cwho, and through which surfaces\u003e\n\n## Review checklist\n\n- [ ] Authn / authz — every privileged path checks identity *and* permission\n- [ ] Input validation — untrusted input is parsed/escaped (injection, path traversal)\n- [ ] Secrets — no hard-coded creds; least-privilege tokens; nothing sensitive logged\n- [ ] Dependencies — known-vuln scan; versions pinned\n- [ ] Data at rest / in transit — encryption + safe defaults\n\n## Findings\n\n\u003c!-- One finding per issue, in this shape (un-fence it): --\u003e\n\n```\n#### H1. \u003ctitle\u003e · **Status:** open\n\n**File:** \u003cpath:line\u003e | **Component:** \u003ccomponent\u003e\n**Severity:** \u003ccritical|high|medium|low\u003e · **Effort:** \u003cXS|S|M|L\u003e · **Urgency:** \u003cacute|soon|eventually\u003e\n\n\u003cwhat's exploitable, the impact, and how\u003e\n\n**Recommendation:** \u003cthe fix\u003e\n```\n\n## Candidate tasks\n\n\u003c!-- Mirror each finding: ✅ done · ⚠️ partial · ⏳ open · ⛔ won't do --\u003e\n\n- ⏳ `tskflwctl task new \"\u003ctitle\u003e\" --epic \u003cid\u003e --tags security` — \u003cone line\u003e\n"} diff --git a/internal/cli/testdata/planning/tasks/6fjangd7kvh0-alpha-task.md b/internal/cli/testdata/planning/tasks/6fjangd7kvh0-alpha-task.md index ac56ef6b..b8002c6b 100644 --- a/internal/cli/testdata/planning/tasks/6fjangd7kvh0-alpha-task.md +++ b/internal/cli/testdata/planning/tasks/6fjangd7kvh0-alpha-task.md @@ -8,6 +8,7 @@ tier: 2 priority: high autonomy_level: 3 tags: [cli, testing] +depends_on: [6fjangd7kvh2] created: "2026-01-02" updated_at: "2026-01-03" --- diff --git a/internal/core/dependency_graph.go b/internal/core/dependency_graph.go new file mode 100644 index 00000000..ad6a017b --- /dev/null +++ b/internal/core/dependency_graph.go @@ -0,0 +1,1098 @@ +package core + +import ( + "fmt" + "path/filepath" + "sort" + "strings" + "sync" + + "github.com/andy-esch/taskflow/internal/domain" + "github.com/andy-esch/taskflow/internal/id" +) + +// GraphHealth describes whether one immutable repository task snapshot is safe +// for graph-sensitive decisions. Degraded is intentionally distinct from broken: +// every legacy reference resolves, but those constraints are not canonical edges +// yet. Both degraded and broken snapshots fail closed for ordinary mutations and +// dispatch-oriented selectors. +type GraphHealth string + +const ( + GraphHealthy GraphHealth = "healthy" + GraphDegraded GraphHealth = "degraded" + GraphBroken GraphHealth = "broken" +) + +// GraphProblemCode is taskflow-owned diagnostic vocabulary. A graph library may +// help implement algorithms, but its error types and wording never cross this seam. +type GraphProblemCode string + +const ( + ProblemUnreadable GraphProblemCode = "unreadable-task" + ProblemMissingTaskID GraphProblemCode = "missing-task-id" + ProblemTaskIDDrift GraphProblemCode = "task-id-drift" + ProblemDuplicateTaskID GraphProblemCode = "duplicate-task-id" + ProblemInvalidStatus GraphProblemCode = "invalid-status" + ProblemDuplicateDependency GraphProblemCode = "duplicate-dependency" + ProblemSelfDependency GraphProblemCode = "self-dependency" + ProblemInvalidDependencyID GraphProblemCode = "invalid-dependency-id" + ProblemMissingDependency GraphProblemCode = "missing-dependency" + ProblemCycle GraphProblemCode = "cycle" + ProblemLegacyMissing GraphProblemCode = "legacy-reference-missing" + ProblemLegacyAmbiguous GraphProblemCode = "legacy-reference-ambiguous" +) + +// GraphProblem is one deterministic, attributable reason a strict snapshot is +// broken. Cycle repeats its first ID at the end; it is empty for non-cycle defects. +type GraphProblem struct { + Code GraphProblemCode + TaskID string + RelatedTaskID string + Field string + Path string + Message string + Cycle []string +} + +type LegacyResolution string + +const ( + LegacyResolved LegacyResolution = "resolved" + LegacyUnsafe LegacyResolution = "unsafe" + LegacyMissing LegacyResolution = "missing" + LegacyAmbiguous LegacyResolution = "ambiguous" +) + +// DependencyEdge follows graph direction: From is the prerequisite and To is the +// dependent whose task file owns From in depends_on. +type DependencyEdge struct { + From string + To string +} + +// LegacyReference records how one legacy slug/ID maps to the canonical namespace. +// A resolved reference also carries the edge the guarded migration will add. +type LegacyReference struct { + Value string + Resolution LegacyResolution + CandidateIDs []string + Edge DependencyEdge +} + +// LegacyDependencyDiagnostic groups one legacy field occurrence. The production +// repository currently has six such occurrences (some contain several references), +// so lint reports six focused issues rather than one line per edge. +type LegacyDependencyDiagnostic struct { + TaskID string + TaskSlug string + TaskPath string + Field string + References []LegacyReference +} + +type LifecycleRole string + +const ( + RoleQueued LifecycleRole = "queued" + RoleCandidate LifecycleRole = "candidate" + RoleInFlight LifecycleRole = "in-flight" + RoleParked LifecycleRole = "parked" + RoleNominallyComplete LifecycleRole = "nominally-complete" + RoleWithdrawn LifecycleRole = "withdrawn" + RoleUnknown LifecycleRole = "unknown" +) + +type GateState string + +const ( + GateClear GateState = "clear" + GateBlocked GateState = "blocked" + GateBroken GateState = "broken" +) + +// BlockerReason is stable explanatory vocabulary. Invalid-status and cycle are +// deliberately explicit: normal commands prevent them, but hand edits and older +// binaries can still create states that must be diagnosed without euphemism. +type BlockerReason string + +const ( + BlockerNotStarted BlockerReason = "not-started" + BlockerInFlight BlockerReason = "in-flight" + BlockerUnsoundCompleted BlockerReason = "unsound-completed" + BlockerWithdrawn BlockerReason = "withdrawn" + BlockerMissing BlockerReason = "missing" + BlockerParked BlockerReason = "parked" + BlockerInvalidStatus BlockerReason = "invalid-status" + BlockerCycle BlockerReason = "cycle" + BlockerUnreadable BlockerReason = "unreadable" + BlockerInvalidReference BlockerReason = "invalid-reference" + BlockerInvalidTask BlockerReason = "invalid-task" +) + +// Blocker explains one unfinished or broken prerequisite reachable from a task. +// Path starts at the queried task and ends at TaskID. +type Blocker struct { + TaskID string + Reason BlockerReason + Path []string + Direct bool +} + +type TaskGraphState struct { + TaskID string + Role LifecycleRole + Gate GateState + SoundlyCompleted bool + Eligible bool + Drained bool + Inconsistent bool +} + +// GateExplanation keeps authorization and diagnosis coupled without making an +// empty prerequisite projection look like permission. State.Eligible is the +// authorization result; LocalProblems and Frontier explain a refusal. +type GateExplanation struct { + Health GraphHealth + State TaskGraphState + LocalProblems []GraphProblem + Frontier []Blocker +} + +// dagInput and dagAnalysis keep the owned structural algorithm independent from +// task lifecycle and diagnostic policy. +type dagInput struct { + Nodes []string + Edges []DependencyEdge +} + +type dagAnalysis struct { + CyclicComponents [][]string + RepresentativeCycles [][]string + TopologicalWaves [][]string + TopologicalComplete bool +} + +// analyzeDAG is deliberately taskflow-owned. The bounded implementation bake-off +// did not justify retaining a public adapter seam or a third-party dependency. +func analyzeDAG(input dagInput) dagAnalysis { + nodes := sortedUnique(input.Nodes) + known := make(map[string]bool, len(nodes)) + for _, node := range nodes { + known[node] = true + } + outgoing := make(map[string][]string, len(nodes)) + indegree := make(map[string]int, len(nodes)) + seenEdges := make(map[DependencyEdge]bool, len(input.Edges)) + for _, edge := range input.Edges { + if !known[edge.From] || !known[edge.To] || seenEdges[edge] { + continue + } + seenEdges[edge] = true + outgoing[edge.From] = append(outgoing[edge.From], edge.To) + indegree[edge.To]++ + } + for node := range outgoing { + outgoing[node] = sortedUnique(outgoing[node]) + } + + components, cycles := stronglyConnectedCycles(nodes, outgoing) + if len(components) > 0 { + return dagAnalysis{CyclicComponents: components, RepresentativeCycles: cycles} + } + remaining := len(nodes) + current := make([]string, 0) + for _, node := range nodes { + if indegree[node] == 0 { + current = append(current, node) + } + } + waves := make([][]string, 0) + for len(current) > 0 { + waves = append(waves, append([]string(nil), current...)) + next := make([]string, 0) + for _, node := range current { + remaining-- + for _, dependent := range outgoing[node] { + indegree[dependent]-- + if indegree[dependent] == 0 { + next = append(next, dependent) + } + } + } + sort.Strings(next) + current = next + } + if remaining != 0 { + components, cycles = stronglyConnectedCycles(nodes, outgoing) + return dagAnalysis{CyclicComponents: components, RepresentativeCycles: cycles} + } + return dagAnalysis{TopologicalWaves: waves, TopologicalComplete: true} +} + +type soundResult struct { + sound bool + broken bool +} + +// TaskGraph is an immutable projection over one repository scan. Its internal +// query caches are synchronized; callers always receive copies of slices/maps. +type TaskGraph struct { + tasks map[string]domain.Task + ids []string + dependencies map[string][]string + outgoing map[string][]string + problems []GraphProblem + legacy []LegacyDependencyDiagnostic + health GraphHealth + hardBroken map[string]bool + unreadableIDs map[string]bool + cycleMembers map[string]bool + sound map[string]soundResult + states map[string]TaskGraphState + waves [][]string + wavesComplete bool + + mu sync.Mutex + causalCache map[string][]Blocker + frontierCache map[string][]Blocker + downstreamCache map[string][]string + soundVisits map[string]int +} + +// NewTaskGraph builds the production strict snapshot with the owned analyzer. +func NewTaskGraph(tasks []domain.Task, unreadable []domain.FileProblem) *TaskGraph { + return newTaskGraph(tasks, unreadable) +} + +func newTaskGraph(tasks []domain.Task, unreadable []domain.FileProblem) *TaskGraph { + g := &TaskGraph{ + tasks: make(map[string]domain.Task, len(tasks)), + dependencies: make(map[string][]string, len(tasks)), + outgoing: make(map[string][]string, len(tasks)), + hardBroken: make(map[string]bool), + unreadableIDs: make(map[string]bool), + cycleMembers: make(map[string]bool), + sound: make(map[string]soundResult, len(tasks)), + states: make(map[string]TaskGraphState, len(tasks)), + causalCache: make(map[string][]Blocker), + frontierCache: make(map[string][]Blocker), + downstreamCache: make(map[string][]string), + soundVisits: make(map[string]int, len(tasks)), + } + for _, problem := range unreadable { + taskID := taskIDFromPath(problem.Path) + if taskID != "" { + g.unreadableIDs[taskID] = true + g.hardBroken[taskID] = true + } + g.problems = append(g.problems, GraphProblem{ + Code: ProblemUnreadable, TaskID: taskID, Path: problem.Path, + Message: "unreadable task file: " + problem.Message, + }) + } + + ordered := append([]domain.Task(nil), tasks...) + sort.SliceStable(ordered, func(i, j int) bool { + left, right := canonicalTaskID(ordered[i]), canonicalTaskID(ordered[j]) + if left != right { + return left < right + } + if ordered[i].Path != ordered[j].Path { + return ordered[i].Path < ordered[j].Path + } + return ordered[i].Slug < ordered[j].Slug + }) + idCounts := make(map[string]int, len(ordered)) + idPaths := make(map[string][]string, len(ordered)) + for _, task := range ordered { + if taskID := canonicalTaskID(task); taskID != "" { + idCounts[taskID]++ + idPaths[taskID] = append(idPaths[taskID], displayPath(task.Path)) + } + } + for _, task := range ordered { + taskID := canonicalTaskID(task) + if strings.TrimSpace(task.ID) == "" { + g.addProblem(GraphProblem{Code: ProblemMissingTaskID, TaskID: taskID, Field: "id", Path: task.Path, + Message: "missing stable task id in frontmatter"}) + g.hardBroken[taskID] = true + } + if taskID == "" { + continue + } + if task.ID != "" && task.FilenameID != "" && task.ID != task.FilenameID { + g.addProblem(GraphProblem{Code: ProblemTaskIDDrift, TaskID: taskID, RelatedTaskID: task.ID, Field: "id", Path: task.Path, + Message: fmt.Sprintf("frontmatter id %q disagrees with filename id %q", task.ID, task.FilenameID)}) + g.hardBroken[taskID] = true + } + if idCounts[taskID] > 1 { + g.addProblem(GraphProblem{Code: ProblemDuplicateTaskID, TaskID: taskID, Field: "id", Path: task.Path, + Message: fmt.Sprintf("duplicate stable task id %q across %s; no source is uniquely authoritative", taskID, strings.Join(idPaths[taskID], ", "))}) + g.hardBroken[taskID] = true + } + if _, exists := g.tasks[taskID]; !exists { + g.tasks[taskID] = cloneTask(task) + g.ids = append(g.ids, taskID) + } + if !task.Status.Valid() { + g.addProblem(GraphProblem{Code: ProblemInvalidStatus, TaskID: taskID, Field: "status", Path: task.Path, + Message: fmt.Sprintf("task %s has missing or invalid status %q", taskID, task.Status)}) + g.hardBroken[taskID] = true + } + } + sort.Strings(g.ids) + + canonicalEdges := make([]DependencyEdge, 0) + for _, task := range ordered { + taskID := canonicalTaskID(task) + if taskID == "" { + continue + } + representative, isRepresentative := g.tasks[taskID] + isRepresentative = isRepresentative && representative.Path == task.Path && representative.Slug == task.Slug + dependencies := append([]string(nil), task.DependsOn...) + sort.Strings(dependencies) + if isRepresentative { + g.dependencies[taskID] = sortedUnique(dependencies) + } + seen := make(map[string]bool, len(dependencies)) + for _, prerequisite := range dependencies { + if seen[prerequisite] { + g.addProblem(GraphProblem{Code: ProblemDuplicateDependency, TaskID: taskID, RelatedTaskID: prerequisite, + Field: "depends_on", Path: task.Path, + Message: fmt.Sprintf("task %s repeats dependency %s", taskID, prerequisite)}) + g.hardBroken[taskID] = true + continue + } + seen[prerequisite] = true + switch { + case prerequisite == taskID: + g.addProblem(GraphProblem{Code: ProblemSelfDependency, TaskID: taskID, RelatedTaskID: prerequisite, + Field: "depends_on", Path: task.Path, + Message: fmt.Sprintf("task %s cannot depend on itself", taskID)}) + g.hardBroken[taskID] = true + // Retain the representative self-edge for exact SCC membership. + if isRepresentative { + canonicalEdges = append(canonicalEdges, DependencyEdge{From: prerequisite, To: taskID}) + g.outgoing[prerequisite] = append(g.outgoing[prerequisite], taskID) + } + case !id.Valid(prerequisite): + g.addProblem(GraphProblem{Code: ProblemInvalidDependencyID, TaskID: taskID, RelatedTaskID: prerequisite, + Field: "depends_on", Path: task.Path, + Message: fmt.Sprintf("task %s depends_on value %q is not a stable task id", taskID, prerequisite)}) + g.hardBroken[taskID] = true + case !taskExists(g.tasks, prerequisite) && !g.unreadableIDs[prerequisite]: + g.addProblem(GraphProblem{Code: ProblemMissingDependency, TaskID: taskID, RelatedTaskID: prerequisite, + Field: "depends_on", Path: task.Path, + Message: fmt.Sprintf("task %s depends on missing task %s", taskID, prerequisite)}) + g.hardBroken[taskID] = true + default: + if isRepresentative && taskExists(g.tasks, prerequisite) { + canonicalEdges = append(canonicalEdges, DependencyEdge{From: prerequisite, To: taskID}) + g.outgoing[prerequisite] = append(g.outgoing[prerequisite], taskID) + } + } + } + } + for taskID := range g.outgoing { + g.outgoing[taskID] = sortedUnique(g.outgoing[taskID]) + } + + legacyDiagnostics, legacyEdges := g.resolveLegacyDiagnostics(ordered) + g.legacy = legacyDiagnostics + projectedEdges := append(append([]DependencyEdge(nil), canonicalEdges...), legacyEdges...) + structure := analyzeDAG(dagInput{Nodes: append([]string(nil), g.ids...), Edges: projectedEdges}) + g.waves = cloneWaves(structure.TopologicalWaves) + g.wavesComplete = structure.TopologicalComplete + componentByTask := make(map[string]int) + for componentIndex, component := range structure.CyclicComponents { + for _, taskID := range component { + g.cycleMembers[taskID] = true + componentByTask[taskID] = componentIndex + } + cycle := structure.RepresentativeCycles[componentIndex] + for _, taskID := range component { + if len(component) == 1 && g.hasProblem(ProblemSelfDependency, taskID) { + continue + } + path := "" + if task, ok := g.tasks[taskID]; ok { + path = task.Path + } + g.addProblem(GraphProblem{Code: ProblemCycle, TaskID: taskID, Field: "depends_on", Path: path, + Message: "dependency cycle: " + strings.Join(cycle, " -> "), Cycle: append([]string(nil), cycle...)}) + } + } + g.markUnsafeLegacy(componentByTask) + sortGraphProblems(g.problems) + sort.Slice(g.legacy, func(i, j int) bool { + if g.legacy[i].TaskID != g.legacy[j].TaskID { + return g.legacy[i].TaskID < g.legacy[j].TaskID + } + if g.legacy[i].TaskPath != g.legacy[j].TaskPath { + return g.legacy[i].TaskPath < g.legacy[j].TaskPath + } + return g.legacy[i].Field < g.legacy[j].Field + }) + switch { + case len(g.problems) > 0: + g.health = GraphBroken + case len(g.legacy) > 0: + g.health = GraphDegraded + default: + g.health = GraphHealthy + } + + for _, taskID := range g.ids { + g.computeSound(taskID, make(map[string]bool)) + } + for _, taskID := range g.ids { + g.states[taskID] = g.deriveState(taskID) + } + return g +} + +func taskExists(tasks map[string]domain.Task, taskID string) bool { + _, ok := tasks[taskID] + return ok +} + +func canonicalTaskID(task domain.Task) string { + if task.FilenameID != "" { + return task.FilenameID + } + return task.ID +} + +func taskIDFromPath(path string) string { + base := filepath.Base(path) + if len(base) <= id.Length || base[id.Length] != '-' { + return "" + } + candidate := base[:id.Length] + if !id.Valid(candidate) { + return "" + } + return candidate +} + +func cloneTask(task domain.Task) domain.Task { + task.Tags = append([]string(nil), task.Tags...) + task.DependsOn = append([]string(nil), task.DependsOn...) + task.LegacyBlockedBy = append([]string(nil), task.LegacyBlockedBy...) + task.LegacyDependencies = append([]string(nil), task.LegacyDependencies...) + task.LegacyBlocks = append([]string(nil), task.LegacyBlocks...) + return task +} + +func displayPath(path string) string { + if path == "" { + return "" + } + return filepath.ToSlash(path) +} + +func (g *TaskGraph) addProblem(problem GraphProblem) { + g.problems = append(g.problems, problem) +} + +func (g *TaskGraph) hasProblem(code GraphProblemCode, taskID string) bool { + for _, problem := range g.problems { + if problem.Code == code && problem.TaskID == taskID { + return true + } + } + return false +} + +func (g *TaskGraph) resolveLegacyDiagnostics(records []domain.Task) ([]LegacyDependencyDiagnostic, []DependencyEdge) { + bySlug := make(map[string][]string, len(g.tasks)) + for _, taskID := range g.ids { + bySlug[g.tasks[taskID].Slug] = append(bySlug[g.tasks[taskID].Slug], taskID) + } + for slug := range bySlug { + bySlug[slug] = sortedUnique(bySlug[slug]) + } + + type legacyField struct { + name string + values func(domain.Task) []string + blocks bool + } + fields := []legacyField{ + {name: "blocked_by", values: func(t domain.Task) []string { return t.LegacyBlockedBy }}, + {name: "dependencies", values: func(t domain.Task) []string { return t.LegacyDependencies }}, + {name: "blocks", values: func(t domain.Task) []string { return t.LegacyBlocks }, blocks: true}, + } + + var diagnostics []LegacyDependencyDiagnostic + var edges []DependencyEdge + seenEdges := make(map[DependencyEdge]bool) + for _, task := range records { + taskID := canonicalTaskID(task) + if taskID == "" { + continue + } + for _, field := range fields { + values := sortedUnique(field.values(task)) + if len(values) == 0 { + continue + } + diagnostic := LegacyDependencyDiagnostic{ + TaskID: taskID, TaskSlug: task.Slug, TaskPath: task.Path, Field: field.name, + } + for _, value := range values { + ref := LegacyReference{Value: value} + var candidates []string + if taskExists(g.tasks, value) { + candidates = []string{value} + } else { + candidates = append([]string(nil), bySlug[value]...) + } + ref.CandidateIDs = candidates + switch len(candidates) { + case 0: + ref.Resolution = LegacyMissing + g.addProblem(GraphProblem{Code: ProblemLegacyMissing, TaskID: taskID, Field: field.name, Path: task.Path, + Message: fmt.Sprintf("legacy %s reference %q on task %s has no exact task ID or slug match", field.name, value, taskID)}) + g.hardBroken[taskID] = true + case 1: + ref.Resolution = LegacyResolved + if field.blocks { + ref.Edge = DependencyEdge{From: taskID, To: candidates[0]} + } else { + ref.Edge = DependencyEdge{From: candidates[0], To: taskID} + } + if ref.Edge.From == ref.Edge.To { + ref.Resolution = LegacyUnsafe + g.addProblem(GraphProblem{Code: ProblemSelfDependency, TaskID: taskID, RelatedTaskID: taskID, + Field: field.name, Path: task.Path, + Message: fmt.Sprintf("legacy %s reference %q makes task %s depend on itself", field.name, value, taskID)}) + g.hardBroken[taskID] = true + } + if !seenEdges[ref.Edge] { + seenEdges[ref.Edge] = true + edges = append(edges, ref.Edge) + } + default: + ref.Resolution = LegacyAmbiguous + g.addProblem(GraphProblem{Code: ProblemLegacyAmbiguous, TaskID: taskID, Field: field.name, Path: task.Path, + Message: fmt.Sprintf("legacy %s reference %q on task %s is ambiguous across task IDs %s", field.name, value, taskID, strings.Join(candidates, ", "))}) + g.hardBroken[taskID] = true + } + diagnostic.References = append(diagnostic.References, ref) + } + diagnostics = append(diagnostics, diagnostic) + } + } + return diagnostics, edges +} + +func (g *TaskGraph) markUnsafeLegacy(componentByTask map[string]int) { + for i := range g.legacy { + for j := range g.legacy[i].References { + ref := &g.legacy[i].References[j] + if ref.Resolution != LegacyResolved { + continue + } + fromComponent, fromCyclic := componentByTask[ref.Edge.From] + toComponent, toCyclic := componentByTask[ref.Edge.To] + if fromCyclic && toCyclic && fromComponent == toComponent { + ref.Resolution = LegacyUnsafe + g.hardBroken[g.legacy[i].TaskID] = true + } + } + } +} + +func (g *TaskGraph) computeSound(taskID string, visiting map[string]bool) soundResult { + if result, ok := g.sound[taskID]; ok { + return result + } + g.soundVisits[taskID]++ + task, ok := g.tasks[taskID] + if !ok { + result := soundResult{broken: true} + g.sound[taskID] = result + return result + } + if visiting[taskID] || g.cycleMembers[taskID] || g.hardBroken[taskID] || !task.Status.Valid() || task.Status == domain.StatusDeprecated { + result := soundResult{broken: true} + g.sound[taskID] = result + return result + } + visiting[taskID] = true + defer delete(visiting, taskID) + allSound := true + for _, prerequisite := range g.dependencies[taskID] { + result := g.computeSound(prerequisite, visiting) + if result.broken { + result = soundResult{broken: true} + g.sound[taskID] = result + return result + } + if !result.sound { + allSound = false + } + } + result := soundResult{sound: task.Status == domain.StatusCompleted && allSound} + g.sound[taskID] = result + return result +} + +func roleForStatus(status domain.Status) LifecycleRole { + switch status { + case domain.StatusNextUp: + return RoleQueued + case domain.StatusReadyToStart: + return RoleCandidate + case domain.StatusInProgress: + return RoleInFlight + case domain.StatusDeferred: + return RoleParked + case domain.StatusCompleted: + return RoleNominallyComplete + case domain.StatusDeprecated: + return RoleWithdrawn + default: + return RoleUnknown + } +} + +func (g *TaskGraph) gate(taskID string) GateState { + task, ok := g.tasks[taskID] + if !ok || !task.Status.Valid() || g.hardBroken[taskID] { + return GateBroken + } + blocked := false + for _, prerequisite := range g.dependencies[taskID] { + result := g.computeSound(prerequisite, make(map[string]bool)) + if result.broken { + return GateBroken + } + if !result.sound { + blocked = true + } + } + if blocked { + return GateBlocked + } + return GateClear +} + +func (g *TaskGraph) deriveState(taskID string) TaskGraphState { + task, ok := g.tasks[taskID] + if !ok { + return TaskGraphState{TaskID: taskID, Role: RoleUnknown, Gate: GateBroken} + } + role := roleForStatus(task.Status) + gate := g.gate(taskID) + sound := g.computeSound(taskID, make(map[string]bool)).sound + return TaskGraphState{ + TaskID: taskID, Role: role, Gate: gate, SoundlyCompleted: sound, + Eligible: g.health == GraphHealthy && role == RoleCandidate && gate == GateClear, + Drained: role == RoleNominallyComplete && sound, + Inconsistent: (role == RoleInFlight || role == RoleNominallyComplete) && gate != GateClear, + } +} + +func (g *TaskGraph) Health() GraphHealth { return g.health } + +// MutationReady is deliberately stricter than "no cycle": legacy constraints must +// be migrated and every task must be readable before an ordinary graph write. +func (g *TaskGraph) MutationReady() bool { return g.health == GraphHealthy } + +func (g *TaskGraph) Problems() []GraphProblem { + out := make([]GraphProblem, len(g.problems)) + copy(out, g.problems) + for i := range out { + out[i].Cycle = append([]string(nil), out[i].Cycle...) + } + return out +} + +func (g *TaskGraph) LegacyDiagnostics() []LegacyDependencyDiagnostic { + out := make([]LegacyDependencyDiagnostic, len(g.legacy)) + for i, diagnostic := range g.legacy { + out[i] = diagnostic + out[i].References = make([]LegacyReference, len(diagnostic.References)) + copy(out[i].References, diagnostic.References) + for j := range out[i].References { + out[i].References[j].CandidateIDs = append([]string(nil), diagnostic.References[j].CandidateIDs...) + } + } + return out +} + +func (g *TaskGraph) Task(taskID string) (domain.Task, bool) { + task, ok := g.tasks[taskID] + return cloneTask(task), ok +} + +func (g *TaskGraph) TaskIDs() []string { return append([]string(nil), g.ids...) } + +func (g *TaskGraph) State(taskID string) TaskGraphState { + if state, ok := g.states[taskID]; ok { + return state + } + return TaskGraphState{TaskID: taskID, Role: RoleUnknown, Gate: GateBroken} +} + +func (g *TaskGraph) SoundlyCompleted(taskID string) (sound, broken bool) { + result, ok := g.sound[taskID] + if !ok { + return false, true + } + return result.sound, result.broken +} + +// CausalBlockers returns every reachable unsound prerequisite. It is a forensic +// projection, not an authorization predicate; use ExplainGate().State.Eligible +// for that decision. Results carry deterministic shortest paths. +func (g *TaskGraph) CausalBlockers(taskID string) []Blocker { + g.mu.Lock() + defer g.mu.Unlock() + if cached, ok := g.causalCache[taskID]; ok { + return cloneBlockers(cached) + } + result := g.projectBlockers(taskID, false) + g.causalCache[taskID] = result + return cloneBlockers(result) +} + +// BlockingFrontier returns the deepest actionable constraints while stopping at +// terminal damage. It is the bounded projection used by lint and user guidance. +func (g *TaskGraph) BlockingFrontier(taskID string) []Blocker { + g.mu.Lock() + defer g.mu.Unlock() + if cached, ok := g.frontierCache[taskID]; ok { + return cloneBlockers(cached) + } + result := g.projectBlockers(taskID, true) + g.frontierCache[taskID] = result + return cloneBlockers(result) +} + +func (g *TaskGraph) projectBlockers(taskID string, frontier bool) []Blocker { + if !taskExists(g.tasks, taskID) { + reason := g.blockerReason(taskID) + return []Blocker{{TaskID: taskID, Reason: reason, Path: []string{taskID}}} + } + queue := []string{taskID} + visited := map[string]bool{taskID: true} + parent := make(map[string]string) + emitted := make(map[string]bool) + for len(queue) > 0 { + current := queue[0] + queue = queue[1:] + for _, prerequisite := range g.unsoundPrerequisites(current) { + if visited[prerequisite] { + continue + } + visited[prerequisite] = true + parent[prerequisite] = current + reason := g.blockerReason(prerequisite) + children := g.unsoundPrerequisites(prerequisite) + terminal := isTerminalBlocker(reason) || !taskExists(g.tasks, prerequisite) + if !frontier || terminal || len(children) == 0 { + emitted[prerequisite] = true + } + if taskExists(g.tasks, prerequisite) && (!frontier || (!terminal && len(children) > 0)) { + queue = append(queue, prerequisite) + } + } + } + ids := make([]string, 0, len(emitted)) + for taskID := range emitted { + ids = append(ids, taskID) + } + sort.Strings(ids) + result := make([]Blocker, 0, len(ids)) + for _, blockerID := range ids { + path := blockerPath(taskID, blockerID, parent) + result = append(result, Blocker{ + TaskID: blockerID, Reason: g.blockerReason(blockerID), + Path: path, Direct: len(path) == 2, + }) + } + return result +} + +func (g *TaskGraph) unsoundPrerequisites(taskID string) []string { + result := make([]string, 0, len(g.dependencies[taskID])) + for _, prerequisite := range g.dependencies[taskID] { + sound, ok := g.sound[prerequisite] + if !ok || sound.broken || !sound.sound { + result = append(result, prerequisite) + } + } + return result +} + +func blockerPath(root, taskID string, parent map[string]string) []string { + reversed := []string{taskID} + for current := taskID; current != root; { + previous, ok := parent[current] + if !ok { + break + } + reversed = append(reversed, previous) + current = previous + } + for left, right := 0, len(reversed)-1; left < right; left, right = left+1, right-1 { + reversed[left], reversed[right] = reversed[right], reversed[left] + } + return reversed +} + +func isTerminalBlocker(reason BlockerReason) bool { + switch reason { + case BlockerWithdrawn, BlockerMissing, BlockerInvalidStatus, BlockerCycle, + BlockerUnreadable, BlockerInvalidReference, BlockerInvalidTask: + return true + default: + return false + } +} + +func (g *TaskGraph) blockerReason(taskID string) BlockerReason { + task, ok := g.tasks[taskID] + if !ok { + if g.unreadableIDs[taskID] { + return BlockerUnreadable + } + if !id.Valid(taskID) { + return BlockerInvalidReference + } + return BlockerMissing + } + if !task.Status.Valid() { + return BlockerInvalidStatus + } + if g.cycleMembers[taskID] { + return BlockerCycle + } + if g.hardBroken[taskID] { + return BlockerInvalidTask + } + switch task.Status { + case domain.StatusNextUp, domain.StatusReadyToStart: + return BlockerNotStarted + case domain.StatusInProgress: + return BlockerInFlight + case domain.StatusCompleted: + return BlockerUnsoundCompleted + case domain.StatusDeprecated: + return BlockerWithdrawn + case domain.StatusDeferred: + return BlockerParked + default: + return BlockerInvalidStatus + } +} + +func (g *TaskGraph) LocalProblems(taskID string) []GraphProblem { + problems := make([]GraphProblem, 0) + for _, problem := range g.problems { + if problem.TaskID == taskID { + problem.Cycle = append([]string(nil), problem.Cycle...) + problems = append(problems, problem) + } + } + return problems +} + +func (g *TaskGraph) ExplainGate(taskID string) GateExplanation { + return GateExplanation{ + Health: g.health, State: g.State(taskID), + LocalProblems: g.LocalProblems(taskID), Frontier: g.BlockingFrontier(taskID), + } +} + +// Downstream returns all transitive dependents in stable ID order, memoized per task. +func (g *TaskGraph) Downstream(taskID string) []string { + g.mu.Lock() + defer g.mu.Unlock() + if cached, ok := g.downstreamCache[taskID]; ok { + return append([]string(nil), cached...) + } + seen := make(map[string]bool) + queue := []string{taskID} + for len(queue) > 0 { + current := queue[0] + queue = queue[1:] + for _, dependent := range g.outgoing[current] { + if seen[dependent] { + continue + } + seen[dependent] = true + queue = append(queue, dependent) + } + } + delete(seen, taskID) + result := make([]string, 0, len(seen)) + for dependent := range seen { + result = append(result, dependent) + } + sort.Strings(result) + g.downstreamCache[taskID] = result + return append([]string(nil), result...) +} + +func (g *TaskGraph) TopologicalWaves() ([][]string, bool) { + return cloneWaves(g.waves), g.wavesComplete && g.health == GraphHealthy +} + +func cloneBlockers(values []Blocker) []Blocker { + out := make([]Blocker, len(values)) + copy(out, values) + for i := range out { + out[i].Path = append([]string(nil), out[i].Path...) + } + return out +} + +func cloneWaves(values [][]string) [][]string { + out := make([][]string, len(values)) + for i := range values { + out[i] = append([]string(nil), values[i]...) + } + return out +} + +func sortedUnique(values []string) []string { + if len(values) == 0 { + return nil + } + out := append([]string(nil), values...) + sort.Strings(out) + n := 0 + for _, value := range out { + if n == 0 || value != out[n-1] { + out[n] = value + n++ + } + } + return out[:n] +} + +// stronglyConnectedCycles uses Tarjan's algorithm to make cycle membership +// exact. It returns one sorted member list and one deterministic representative +// edge-following cycle for every cyclic SCC; it deliberately does not enumerate +// every simple cycle, which can be exponential. +func stronglyConnectedCycles(nodes []string, outgoing map[string][]string) ([][]string, [][]string) { + index := 0 + indices := make(map[string]int, len(nodes)) + lowlink := make(map[string]int, len(nodes)) + onStack := make(map[string]bool, len(nodes)) + stack := make([]string, 0, len(nodes)) + components := make([][]string, 0) + var connect func(string) + connect = func(node string) { + indices[node] = index + lowlink[node] = index + index++ + stack = append(stack, node) + onStack[node] = true + for _, dependent := range outgoing[node] { + dependentIndex, visited := indices[dependent] + if !visited { + connect(dependent) + if lowlink[dependent] < lowlink[node] { + lowlink[node] = lowlink[dependent] + } + } else if onStack[dependent] && dependentIndex < lowlink[node] { + lowlink[node] = dependentIndex + } + } + if lowlink[node] != indices[node] { + return + } + component := make([]string, 0) + for { + last := stack[len(stack)-1] + stack = stack[:len(stack)-1] + onStack[last] = false + component = append(component, last) + if last == node { + break + } + } + sort.Strings(component) + cyclic := len(component) > 1 + if len(component) == 1 { + for _, dependent := range outgoing[component[0]] { + if dependent == component[0] { + cyclic = true + break + } + } + } + if cyclic { + components = append(components, component) + } + } + for _, node := range nodes { + if _, visited := indices[node]; !visited { + connect(node) + } + } + sort.Slice(components, func(i, j int) bool { + return strings.Join(components[i], "\x00") < strings.Join(components[j], "\x00") + }) + cycles := make([][]string, 0, len(components)) + for _, component := range components { + cycles = append(cycles, representativeCycle(component, outgoing)) + } + return components, cycles +} + +func representativeCycle(component []string, outgoing map[string][]string) []string { + start := component[0] + if len(component) == 1 { + return []string{start, start} + } + inComponent := make(map[string]bool, len(component)) + for _, taskID := range component { + inComponent[taskID] = true + } + path := []string{start} + visited := map[string]bool{start: true} + var find func(string) bool + find = func(current string) bool { + for _, next := range outgoing[current] { + if !inComponent[next] { + continue + } + if next == start && len(path) > 1 { + path = append(path, start) + return true + } + if visited[next] { + continue + } + visited[next] = true + path = append(path, next) + if find(next) { + return true + } + path = path[:len(path)-1] + delete(visited, next) + } + return false + } + if find(start) { + return append([]string(nil), path...) + } + panic("cyclic strongly connected component has no representative cycle") +} + +func sortGraphProblems(problems []GraphProblem) { + sort.SliceStable(problems, func(i, j int) bool { + left, right := problems[i], problems[j] + lk := strings.Join([]string{left.TaskID, string(left.Code), left.Field, left.RelatedTaskID, left.Path, left.Message}, "\x00") + rk := strings.Join([]string{right.TaskID, string(right.Code), right.Field, right.RelatedTaskID, right.Path, right.Message}, "\x00") + return lk < rk + }) +} diff --git a/internal/core/dependency_graph_test.go b/internal/core/dependency_graph_test.go new file mode 100644 index 00000000..379e48c1 --- /dev/null +++ b/internal/core/dependency_graph_test.go @@ -0,0 +1,482 @@ +package core + +import ( + "fmt" + "math/rand" + "reflect" + "slices" + "strings" + "testing" + + "github.com/andy-esch/taskflow/internal/domain" + "github.com/andy-esch/taskflow/internal/testutil" +) + +func graphRecord(seed string, status domain.Status, dependencies ...string) domain.Task { + taskID := testutil.TaskID(seed) + return domain.Task{ + ID: taskID, FilenameID: taskID, Slug: seed, Path: "tasks/" + taskID + "-" + seed + ".md", + Status: status, DependsOn: append([]string(nil), dependencies...), + } +} + +func TestTaskGraphHealthAndDeterministicStructuralProblems(t *testing.T) { + a := graphRecord("a", domain.StatusCompleted) + b := graphRecord("b", domain.StatusReadyToStart, a.ID, a.ID) + c := graphRecord("c", domain.StatusReadyToStart) + c.DependsOn = []string{c.ID} + d := graphRecord("d", domain.StatusReadyToStart, testutil.TaskID("missing")) + e := graphRecord("e", domain.Status("invented")) + f := graphRecord("f", domain.StatusCompleted) + f.ID = testutil.TaskID("drifted-frontmatter") + + tasks := []domain.Task{f, e, d, c, b, a} + graph := NewTaskGraph(tasks, []domain.FileProblem{{Path: "tasks/broken.md", Message: "malformed frontmatter"}}) + if graph.Health() != GraphBroken || graph.MutationReady() { + t.Fatalf("health = %s mutationReady=%v", graph.Health(), graph.MutationReady()) + } + wantCodes := []GraphProblemCode{ + ProblemUnreadable, ProblemDuplicateDependency, ProblemSelfDependency, + ProblemMissingDependency, ProblemInvalidStatus, ProblemTaskIDDrift, + } + gotCodes := make([]GraphProblemCode, 0) + for _, problem := range graph.Problems() { + gotCodes = append(gotCodes, problem.Code) + } + for _, code := range wantCodes { + if !slices.Contains(gotCodes, code) { + t.Errorf("problems %v do not contain %s", gotCodes, code) + } + } + + baseline := fmt.Sprintf("%+v", graph.Problems()) + for seed := int64(0); seed < 20; seed++ { + shuffled := append([]domain.Task(nil), tasks...) + rand.New(rand.NewSource(seed)).Shuffle(len(shuffled), func(i, j int) { shuffled[i], shuffled[j] = shuffled[j], shuffled[i] }) + for i := range shuffled { + rand.New(rand.NewSource(seed+int64(i)+100)).Shuffle(len(shuffled[i].DependsOn), func(x, y int) { + shuffled[i].DependsOn[x], shuffled[i].DependsOn[y] = shuffled[i].DependsOn[y], shuffled[i].DependsOn[x] + }) + } + if got := fmt.Sprintf("%+v", NewTaskGraph(shuffled, []domain.FileProblem{{Path: "tasks/broken.md", Message: "malformed frontmatter"}}).Problems()); got != baseline { + t.Fatalf("seed %d changed diagnostics\nwant %s\n got %s", seed, baseline, got) + } + } +} + +func TestTaskGraphDiagnosesEveryIdentityAndEdgeShape(t *testing.T) { + missingFrontmatterID := graphRecord("missing-frontmatter-id", domain.StatusReadyToStart) + missingFrontmatterID.ID = "" + duplicateA := graphRecord("duplicate-a", domain.StatusReadyToStart) + duplicateB := graphRecord("duplicate-b", domain.StatusReadyToStart) + duplicateB.ID = duplicateA.ID + duplicateB.FilenameID = duplicateA.ID + invalidEdge := graphRecord("invalid-edge", domain.StatusReadyToStart, "not-a-stable-id") + + graph := NewTaskGraph([]domain.Task{invalidEdge, duplicateB, missingFrontmatterID, duplicateA}, nil) + want := []GraphProblemCode{ProblemMissingTaskID, ProblemDuplicateTaskID, ProblemInvalidDependencyID} + got := make([]GraphProblemCode, 0) + for _, problem := range graph.Problems() { + got = append(got, problem.Code) + } + for _, code := range want { + if !slices.Contains(got, code) { + t.Errorf("problems %v do not contain %s", got, code) + } + } +} + +func TestTaskGraphGatePrecedenceReasonsAndShortestPaths(t *testing.T) { + root := graphRecord("root", domain.StatusReadyToStart) + inFlight := graphRecord("in-flight", domain.StatusInProgress) + parked := graphRecord("parked", domain.StatusDeferred) + withdrawn := graphRecord("withdrawn", domain.StatusDeprecated) + invalid := graphRecord("invalid", domain.Status("bogus")) + unsoundDone := graphRecord("unsound-done", domain.StatusCompleted, root.ID) + missingID := testutil.TaskID("missing") + target := graphRecord("target", domain.StatusReadyToStart, + root.ID, inFlight.ID, parked.ID, withdrawn.ID, invalid.ID, unsoundDone.ID, missingID) + + graph := NewTaskGraph([]domain.Task{target, unsoundDone, invalid, withdrawn, parked, inFlight, root}, nil) + if state := graph.State(target.ID); state.Gate != GateBroken || state.Eligible { + t.Fatalf("target state = %+v; broken must outrank blocked", state) + } + reasons := map[string]BlockerReason{} + for _, blocker := range graph.CausalBlockers(target.ID) { + reasons[blocker.TaskID] = blocker.Reason + if blocker.TaskID == root.ID && (!blocker.Direct || !reflect.DeepEqual(blocker.Path, []string{target.ID, root.ID})) { + t.Errorf("direct root path = %+v", blocker) + } + } + want := map[string]BlockerReason{ + root.ID: BlockerNotStarted, + inFlight.ID: BlockerInFlight, + parked.ID: BlockerParked, + withdrawn.ID: BlockerWithdrawn, + invalid.ID: BlockerInvalidStatus, + unsoundDone.ID: BlockerUnsoundCompleted, + missingID: BlockerMissing, + } + if !reflect.DeepEqual(reasons, want) { + t.Fatalf("reasons = %v, want %v", reasons, want) + } + + // Two equal-length paths to root choose the lexicographically first immediate + // prerequisite, independent of insertion order. + left := graphRecord("left", domain.StatusCompleted, root.ID) + right := graphRecord("right", domain.StatusCompleted, root.ID) + join := graphRecord("join", domain.StatusReadyToStart, right.ID, left.ID) + shortest := NewTaskGraph([]domain.Task{join, right, left, root}, nil).CausalBlockers(join.ID) + var rootPath []string + for _, blocker := range shortest { + if blocker.TaskID == root.ID { + rootPath = blocker.Path + } + } + first := left.ID + if right.ID < left.ID { + first = right.ID + } + if !reflect.DeepEqual(rootPath, []string{join.ID, first, root.ID}) { + t.Fatalf("root shortest path = %v, want lexicographic tie via %s", rootPath, first) + } +} + +func TestTaskGraphLegacyResolutionHealthAndDirection(t *testing.T) { + prerequisite := graphRecord("prerequisite", domain.StatusCompleted) + dependent := graphRecord("dependent", domain.StatusReadyToStart) + dependent.LegacyBlockedBy = []string{prerequisite.Slug} + dependent.LegacyDependencies = []string{prerequisite.ID} + prerequisite.LegacyBlocks = []string{dependent.Slug} + + graph := NewTaskGraph([]domain.Task{dependent, prerequisite}, nil) + if graph.Health() != GraphDegraded || graph.MutationReady() { + t.Fatalf("resolved legacy health = %s mutationReady=%v", graph.Health(), graph.MutationReady()) + } + if len(graph.Problems()) != 0 || len(graph.LegacyDiagnostics()) != 3 { + t.Fatalf("problems=%+v legacy=%+v", graph.Problems(), graph.LegacyDiagnostics()) + } + for _, diagnostic := range graph.LegacyDiagnostics() { + for _, ref := range diagnostic.References { + if ref.Resolution != LegacyResolved || ref.Edge != (DependencyEdge{From: prerequisite.ID, To: dependent.ID}) { + t.Errorf("legacy %s resolution = %+v", diagnostic.Field, ref) + } + } + } + // Degraded means canonical queries remain explanatory, but dispatch does not + // claim eligible work while the legacy constraint is still hidden from the DAG. + if state := graph.State(dependent.ID); state.Gate != GateClear || state.Eligible { + t.Fatalf("degraded candidate state = %+v", state) + } +} + +func TestTaskGraphLegacyMissingAndAmbiguousAreBroken(t *testing.T) { + first := graphRecord("same-one", domain.StatusCompleted) + second := graphRecord("same-two", domain.StatusCompleted) + first.Slug, second.Slug = "same", "same" + dependent := graphRecord("dependent", domain.StatusReadyToStart) + dependent.LegacyBlockedBy = []string{"same", "gone"} + graph := NewTaskGraph([]domain.Task{dependent, first, second}, nil) + if graph.Health() != GraphBroken { + t.Fatalf("health = %s", graph.Health()) + } + codes := make([]GraphProblemCode, 0) + for _, problem := range graph.Problems() { + codes = append(codes, problem.Code) + } + if !slices.Contains(codes, ProblemLegacyAmbiguous) || !slices.Contains(codes, ProblemLegacyMissing) { + t.Fatalf("legacy problem codes = %v", codes) + } + diagnostic := graph.LegacyDiagnostics()[0] + if len(diagnostic.References) != 2 || diagnostic.References[0].Value != "gone" || diagnostic.References[1].Value != "same" { + t.Fatalf("legacy references not stable: %+v", diagnostic.References) + } +} + +func TestTaskGraphTopologicalWavesAndDownstream(t *testing.T) { + a := graphRecord("a", domain.StatusCompleted) + b := graphRecord("b", domain.StatusCompleted, a.ID) + c := graphRecord("c", domain.StatusCompleted, a.ID) + d := graphRecord("d", domain.StatusReadyToStart, b.ID, c.ID) + e := graphRecord("disconnected", domain.StatusReadyToStart) + graph := NewTaskGraph([]domain.Task{d, b, e, c, a}, nil) + waves, complete := graph.TopologicalWaves() + first := []string{a.ID, e.ID} + sortStrings(first) + second := []string{b.ID, c.ID} + sortStrings(second) + if !complete || !reflect.DeepEqual(waves, [][]string{first, second, {d.ID}}) { + t.Fatalf("waves = %v complete=%v", waves, complete) + } + downstream := []string{b.ID, c.ID, d.ID} + sortStrings(downstream) + if got := graph.Downstream(a.ID); !reflect.DeepEqual(got, downstream) { + t.Fatalf("downstream = %v, want %v", got, downstream) + } + // Returned slices are copies; callers cannot mutate snapshot caches. + got := graph.Downstream(a.ID) + got[0] = "corrupt" + if reflect.DeepEqual(got, graph.Downstream(a.ID)) { + t.Fatal("downstream cache leaked a mutable slice") + } +} + +func TestAnalyzeDAGDeepWideAndDisconnected(t *testing.T) { + const depth = 2048 + nodes := make([]string, 0, depth+129) + edges := make([]DependencyEdge, 0, depth-1) + for i := 0; i < depth; i++ { + node := fmt.Sprintf("%012d", i) + nodes = append(nodes, node) + if i > 0 { + edges = append(edges, DependencyEdge{From: fmt.Sprintf("%012d", i-1), To: node}) + } + } + for i := 0; i < 128; i++ { + nodes = append(nodes, fmt.Sprintf("w%011d", i)) + } + nodes = append(nodes, "z-disconnected") + + analysis := analyzeDAG(dagInput{Nodes: nodes, Edges: edges}) + if !analysis.TopologicalComplete || len(analysis.CyclicComponents) != 0 || len(analysis.TopologicalWaves) != depth { + t.Fatalf("analysis complete=%v cycles=%d waves=%d, want true/0/%d", + analysis.TopologicalComplete, len(analysis.CyclicComponents), len(analysis.TopologicalWaves), depth) + } + if got := len(analysis.TopologicalWaves[0]); got != 130 { + t.Fatalf("first wide/disconnected frontier has %d nodes, want 130", got) + } +} + +func TestTaskGraphCycleBlockerReason(t *testing.T) { + a := graphRecord("cycle-a", domain.StatusCompleted) + b := graphRecord("cycle-b", domain.StatusCompleted, a.ID) + a.DependsOn = []string{b.ID} + target := graphRecord("cycle-target", domain.StatusReadyToStart, a.ID) + graph := NewTaskGraph([]domain.Task{target, b, a}, nil) + + if state := graph.State(target.ID); state.Gate != GateBroken || state.Eligible { + t.Fatalf("target state = %+v", state) + } + blockers := graph.CausalBlockers(target.ID) + if len(blockers) != 2 { + t.Fatalf("cycle blockers = %+v, want both cycle members", blockers) + } + for _, blocker := range blockers { + if blocker.Reason != BlockerCycle { + t.Fatalf("cycle blocker = %+v", blocker) + } + } +} + +func TestTaskGraphSCCMarksEveryMemberAndEmitsRepresentativePath(t *testing.T) { + a := graphRecord("scc-a", domain.StatusCompleted) + b := graphRecord("scc-b", domain.StatusCompleted) + c := graphRecord("scc-c", domain.StatusCompleted) + a.DependsOn = []string{b.ID} + b.DependsOn = []string{a.ID, c.ID} + c.DependsOn = []string{a.ID} + graph := NewTaskGraph([]domain.Task{c, a, b}, nil) + + cycleProblems := make(map[string]GraphProblem) + for _, problem := range graph.Problems() { + if problem.Code == ProblemCycle { + cycleProblems[problem.TaskID] = problem + } + } + if len(cycleProblems) != 3 { + t.Fatalf("cycle attribution = %+v, want one problem for every SCC member", cycleProblems) + } + for _, task := range []domain.Task{a, b, c} { + problem := cycleProblems[task.ID] + if len(problem.Cycle) < 3 || problem.Cycle[0] != problem.Cycle[len(problem.Cycle)-1] { + t.Fatalf("representative cycle for %s = %v", task.ID, problem.Cycle) + } + if graph.blockerReason(task.ID) != BlockerCycle { + t.Fatalf("member %s reason = %s", task.ID, graph.blockerReason(task.ID)) + } + byID := map[string]domain.Task{a.ID: a, b.ID: b, c.ID: c} + for i := 0; i < len(problem.Cycle)-1; i++ { + prerequisite, dependent := problem.Cycle[i], problem.Cycle[i+1] + if !slices.Contains(byID[dependent].DependsOn, prerequisite) { + t.Fatalf("representative path contains non-edge %s -> %s: %v", prerequisite, dependent, problem.Cycle) + } + } + } +} + +func TestTaskGraphSelfDependencyDoesNotDuplicateCycleDiagnostic(t *testing.T) { + task := graphRecord("self-only", domain.StatusReadyToStart) + task.DependsOn = []string{task.ID} + graph := NewTaskGraph([]domain.Task{task}, nil) + var self, cycle int + for _, problem := range graph.Problems() { + switch problem.Code { + case ProblemSelfDependency: + self++ + case ProblemCycle: + cycle++ + } + } + if self != 1 || cycle != 0 { + t.Fatalf("self=%d cycle=%d problems=%+v", self, cycle, graph.Problems()) + } +} + +func TestTaskGraphLegacyProjectedCycleIsUnsafeAndBroken(t *testing.T) { + a := graphRecord("legacy-cycle-a", domain.StatusReadyToStart) + b := graphRecord("legacy-cycle-b", domain.StatusReadyToStart) + a.LegacyBlockedBy = []string{b.ID} + b.LegacyBlockedBy = []string{a.ID} + graph := NewTaskGraph([]domain.Task{b, a}, nil) + if graph.Health() != GraphBroken || graph.MutationReady() { + t.Fatalf("health=%s mutationReady=%v", graph.Health(), graph.MutationReady()) + } + for _, diagnostic := range graph.LegacyDiagnostics() { + if len(diagnostic.References) != 1 || diagnostic.References[0].Resolution != LegacyUnsafe { + t.Fatalf("legacy diagnostic = %+v, want unsafe", diagnostic) + } + } + if _, complete := graph.TopologicalWaves(); complete { + t.Fatal("a legacy-projected cycle cannot yield a complete topology") + } +} + +func TestTaskGraphBrokenOrDegradedTopologyIsNeverComplete(t *testing.T) { + missing := graphRecord("topology-missing", domain.StatusReadyToStart, testutil.TaskID("absent")) + broken := NewTaskGraph([]domain.Task{missing}, nil) + if waves, complete := broken.TopologicalWaves(); complete || len(waves) == 0 { + t.Fatalf("broken waves=%v complete=%v; partial waves are diagnostic only", waves, complete) + } + + prerequisite := graphRecord("topology-legacy-prereq", domain.StatusCompleted) + dependent := graphRecord("topology-legacy-dependent", domain.StatusReadyToStart) + dependent.LegacyBlockedBy = []string{prerequisite.ID} + degraded := NewTaskGraph([]domain.Task{dependent, prerequisite}, nil) + if waves, complete := degraded.TopologicalWaves(); complete || len(waves) != 2 { + t.Fatalf("degraded projected waves=%v complete=%v", waves, complete) + } +} + +func TestTaskGraphSeparatesCausalBlockersFromActionFrontier(t *testing.T) { + root := graphRecord("frontier-root", domain.StatusReadyToStart) + middle := graphRecord("frontier-middle", domain.StatusCompleted, root.ID) + target := graphRecord("frontier-target", domain.StatusReadyToStart, middle.ID) + graph := NewTaskGraph([]domain.Task{target, middle, root}, nil) + + causal := graph.CausalBlockers(target.ID) + if len(causal) != 2 { + t.Fatalf("causal blockers = %+v, want middle and root", causal) + } + frontier := graph.BlockingFrontier(target.ID) + if len(frontier) != 1 || frontier[0].TaskID != root.ID || frontier[0].Reason != BlockerNotStarted { + t.Fatalf("frontier = %+v, want root only", frontier) + } + + broken := graphRecord("locally-broken", domain.Status("invalid")) + explanation := NewTaskGraph([]domain.Task{broken, root}, nil).ExplainGate(broken.ID) + if explanation.State.Eligible || explanation.State.Gate != GateBroken || len(explanation.LocalProblems) == 0 || len(explanation.Frontier) != 0 { + t.Fatalf("broken explanation = %+v", explanation) + } +} + +func TestTaskGraphFrontierStopsAtWithdrawnWhileCausalProjectionContinues(t *testing.T) { + root := graphRecord("withdrawn-root", domain.StatusReadyToStart) + withdrawn := graphRecord("withdrawn-middle", domain.StatusDeprecated, root.ID) + target := graphRecord("withdrawn-target", domain.StatusReadyToStart, withdrawn.ID) + graph := NewTaskGraph([]domain.Task{target, withdrawn, root}, nil) + if causal := graph.CausalBlockers(target.ID); len(causal) != 2 { + t.Fatalf("causal blockers = %+v, want withdrawn and its upstream", causal) + } + frontier := graph.BlockingFrontier(target.ID) + if len(frontier) != 1 || frontier[0].TaskID != withdrawn.ID || frontier[0].Reason != BlockerWithdrawn { + t.Fatalf("frontier = %+v, want terminal withdrawn task", frontier) + } +} + +func TestTaskGraphDistinguishesUnreadableAndInvalidReferences(t *testing.T) { + unreadableID := testutil.TaskID("unreadable") + target := graphRecord("unreadable-target", domain.StatusReadyToStart, unreadableID, "not-an-id") + graph := NewTaskGraph([]domain.Task{target}, []domain.FileProblem{{ + Path: "tasks/" + unreadableID + "-broken.md", Message: "malformed frontmatter", + }}) + reasons := make(map[string]BlockerReason) + for _, blocker := range graph.CausalBlockers(target.ID) { + reasons[blocker.TaskID] = blocker.Reason + } + if reasons[unreadableID] != BlockerUnreadable || reasons["not-an-id"] != BlockerInvalidReference { + t.Fatalf("blocker reasons = %+v", reasons) + } +} + +func TestTaskGraphDuplicateIDsRetainPathFaithfulDiagnostics(t *testing.T) { + first := graphRecord("duplicate-path-first", domain.StatusReadyToStart) + second := graphRecord("duplicate-path-second", domain.StatusReadyToStart, "bad-reference") + second.ID, second.FilenameID = first.ID, first.ID + graph := NewTaskGraph([]domain.Task{second, first}, nil) + duplicates := make(map[string]bool) + invalidOnSecond := false + for _, problem := range graph.Problems() { + if problem.Code == ProblemDuplicateTaskID { + duplicates[problem.Path] = true + } + if problem.Code == ProblemInvalidDependencyID && problem.Path == second.Path { + invalidOnSecond = true + } + } + if !duplicates[first.Path] || !duplicates[second.Path] || !invalidOnSecond { + t.Fatalf("path-faithful problems = %+v", graph.Problems()) + } + lintByPath := dependencyLintIssues(graph) + if len(lintByPath[first.Path]) == 0 || len(lintByPath[second.Path]) < 2 { + t.Fatalf("path-faithful lint issues = %+v", lintByPath) + } + for _, issue := range lintByPath[first.Path] { + if strings.Contains(issue.Message, "bad-reference") { + t.Fatalf("second record defect leaked onto first path: %+v", lintByPath) + } + } +} + +func TestTaskGraphSoundCompletionMemoizesReconvergentDiamonds(t *testing.T) { + tasks := []domain.Task{graphRecord("root", domain.StatusCompleted)} + previous := tasks[0].ID + for i := 0; i < 120; i++ { + left := graphRecord(fmt.Sprintf("left-%03d", i), domain.StatusCompleted, previous) + right := graphRecord(fmt.Sprintf("right-%03d", i), domain.StatusCompleted, previous) + join := graphRecord(fmt.Sprintf("join-%03d", i), domain.StatusCompleted, left.ID, right.ID) + tasks = append(tasks, left, right, join) + previous = join.ID + } + graph := NewTaskGraph(tasks, nil) + sound, broken := graph.SoundlyCompleted(previous) + if !sound || broken { + t.Fatalf("last join sound=%v broken=%v", sound, broken) + } + for taskID, visits := range graph.soundVisits { + if visits != 1 { + t.Fatalf("task %s computed %d times; want exactly once", taskID, visits) + } + } + if len(graph.soundVisits) != len(tasks) { + t.Fatalf("visited %d tasks, want %d", len(graph.soundVisits), len(tasks)) + } +} + +func TestTaskGraphReopenInvalidatesCompletedDescendantsWithoutRewriting(t *testing.T) { + base := graphRecord("base", domain.StatusCompleted) + downstream := graphRecord("downstream", domain.StatusCompleted, base.ID) + initial := NewTaskGraph([]domain.Task{downstream, base}, nil) + if state := initial.State(downstream.ID); !state.Drained || state.Inconsistent { + t.Fatalf("initial downstream = %+v", state) + } + base.Status = domain.StatusReadyToStart + reopened := NewTaskGraph([]domain.Task{downstream, base}, nil) + state := reopened.State(downstream.ID) + if state.Drained || !state.Inconsistent || state.Gate != GateBlocked || downstream.Status != domain.StatusCompleted { + t.Fatalf("reopened downstream = %+v persisted=%s", state, downstream.Status) + } +} + +func sortStrings(values []string) { + slices.Sort(values) +} diff --git a/internal/core/service.go b/internal/core/service.go index 844e959c..73620018 100644 --- a/internal/core/service.go +++ b/internal/core/service.go @@ -224,6 +224,25 @@ type LintResult struct { Issues []domain.Issue } +func (r LintResult) Blocking() bool { + for _, issue := range r.Issues { + if issue.Blocking() { + return true + } + } + return false +} + +func BlockingLintResultCount(results []LintResult) int { + count := 0 + for _, result := range results { + if result.Blocking() { + count++ + } + } + return count +} + // Lint validates active tasks' frontmatter (joining against known epics for the // epic-existence check) AND the epics themselves. Returns one LintResult per // task or epic with issues. @@ -232,6 +251,7 @@ func (s *Service) Lint() ([]LintResult, []domain.FileProblem, error) { if err != nil { return nil, nil, err } + taskProblems := append([]domain.FileProblem(nil), problems...) epics, ep2, err := s.store.ListEpics() if err != nil { return nil, nil, err @@ -242,6 +262,12 @@ func (s *Service) Lint() ([]LintResult, []domain.FileProblem, error) { valid[domain.EpicRefKey(e.ID)] = true } validEpic := func(id string) bool { return valid[domain.EpicRefKey(id)] } + taskRecords := make([]domain.Task, len(tasks)) + for i := range tasks { + taskRecords[i] = tasks[i].Task + } + graph := NewTaskGraph(taskRecords, taskProblems) + graphIssues := dependencyLintIssues(graph) var results []LintResult for _, tb := range tasks { @@ -262,6 +288,7 @@ func (s *Service) Lint() ([]LintResult, []domain.FileProblem, error) { issues = append(domain.FrontmatterStatusIssues(t), domain.MissingIDIssue(t.ID)...) issues = append(issues, domain.IDDriftIssue(t.ID, t.FilenameID)...) } + issues = append(issues, graphIssues[t.Path]...) if len(issues) > 0 { results = append(results, LintResult{Slug: t.Slug, Issues: issues}) } @@ -316,6 +343,72 @@ func (s *Service) Lint() ([]LintResult, []domain.FileProblem, error) { return results, problems, nil } +func dependencyLintIssues(graph *TaskGraph) map[string][]domain.Issue { + out := make(map[string][]domain.Issue) + for _, problem := range graph.Problems() { + // These already have established ordinary-lint/FileProblem renderings. Keep + // strict snapshot attribution without printing the same defect twice. + switch problem.Code { + case ProblemUnreadable, ProblemMissingTaskID, ProblemTaskIDDrift, ProblemInvalidStatus, + ProblemLegacyMissing, ProblemLegacyAmbiguous: + continue + } + if problem.Path == "" { + continue + } + field := problem.Field + if field == "" { + field = "depends_on" + } + out[problem.Path] = append(out[problem.Path], domain.Issue{Field: field, Message: problem.Message}) + } + for _, diagnostic := range graph.LegacyDiagnostics() { + parts := make([]string, 0, len(diagnostic.References)) + severity := domain.IssueAdvisory + for _, ref := range diagnostic.References { + switch ref.Resolution { + case LegacyResolved: + parts = append(parts, fmt.Sprintf("%q resolves to %s (edge %s -> %s)", ref.Value, ref.CandidateIDs[0], ref.Edge.From, ref.Edge.To)) + case LegacyUnsafe: + severity = "" + parts = append(parts, fmt.Sprintf("%q resolves to %s but its projected edge %s -> %s is structurally unsafe", ref.Value, ref.CandidateIDs[0], ref.Edge.From, ref.Edge.To)) + case LegacyMissing: + severity = "" + parts = append(parts, fmt.Sprintf("%q has no exact task ID or slug match", ref.Value)) + case LegacyAmbiguous: + severity = "" + parts = append(parts, fmt.Sprintf("%q is ambiguous across %s", ref.Value, strings.Join(ref.CandidateIDs, ", "))) + } + } + out[diagnostic.TaskPath] = append(out[diagnostic.TaskPath], domain.Issue{ + Field: diagnostic.Field, Severity: severity, + Message: fmt.Sprintf("legacy dependency field: %s; canonical migration is intentionally deferred to guarded dependency operations", + strings.Join(parts, "; ")), + }) + } + for _, taskID := range graph.TaskIDs() { + state := graph.State(taskID) + if !state.Inconsistent { + continue + } + blockers := graph.BlockingFrontier(taskID) + explanations := make([]string, 0, len(blockers)) + for _, blocker := range blockers { + explanations = append(explanations, fmt.Sprintf("%s (%s via %s)", + blocker.TaskID, blocker.Reason, strings.Join(blocker.Path, " -> "))) + } + message := fmt.Sprintf("persisted %s task has a %s dependency gate", state.Role, state.Gate) + if len(explanations) > 0 { + message += ": " + strings.Join(explanations, "; ") + } + task, ok := graph.Task(taskID) + if ok && task.Path != "" { + out[task.Path] = append(out[task.Path], domain.Issue{Field: "status", Message: message}) + } + } + return out +} + func hasTag(tags []string, want string) bool { for _, t := range tags { if strings.EqualFold(t, want) { diff --git a/internal/core/service_task.go b/internal/core/service_task.go index 0d2964ba..8b7b581a 100644 --- a/internal/core/service_task.go +++ b/internal/core/service_task.go @@ -77,6 +77,18 @@ func (s *Service) ShowTask(slug string) (domain.Task, string, error) { return s.store.GetTask(slug) } +// ReadTaskGraph performs one resilient repository scan and projects it into the +// strict, immutable dependency snapshot. Files remain repair-listable through the +// ordinary store API; graph consumers instead inspect Health/Problems and fail +// closed when the snapshot is degraded or broken. +func (s *Service) ReadTaskGraph() (*TaskGraph, error) { + tasks, problems, err := s.store.ListTasks() + if err != nil { + return nil, err + } + return NewTaskGraph(tasks, problems), nil +} + // TaskPath resolves a task's file path without reading or parsing it — the seam // for `task path`, which must work even on a file with broken frontmatter. func (s *Service) TaskPath(slug string) (string, error) { @@ -238,6 +250,15 @@ func (s *Service) SetFields(slug string, updates map[string]any, force, dryRun b } withMeta := make(map[string]any, len(updates)+1) for field, val := range updates { + // A dependency change is a repository-global graph mutation, not ordinary + // frontmatter surgery. Reject it before the known/custom-field and --force + // branches so neither spelling can bypass cycle/referential validation. The + // guarded commands land in the next production slice. + if domain.IsGraphOwnedTaskField(field) { + return domain.Task{}, fmt.Errorf( + "%w: %s is graph-owned and cannot be changed with `task set` (including --force); use guarded dependency operations%s", + domain.ErrValidation, field, graphFieldDirection(field)) + } // status isn't a settable field: a status change relocates the file (frontmatter // is authoritative — ADR-0003 Phase A — but the mirror dir must move with it, and // SetFields writes in place). Route it through the lifecycle verbs, or the dir and @@ -292,6 +313,13 @@ func (s *Service) SetFields(slug string, updates map[string]any, force, dryRun b }) } +func graphFieldDirection(field string) string { + if field == "depends_on" { + return " (`task depend add/remove` once available)" + } + return "; legacy dependency fields are removed only by the guarded migration" +} + // unknownFieldErr is the shared rejection for a field outside the registry, used // by both the set and unset paths of SetFields. func unknownFieldErr(field string) error { diff --git a/internal/core/setfields_coercion_test.go b/internal/core/setfields_coercion_test.go index fb710ba5..4be9ef1b 100644 --- a/internal/core/setfields_coercion_test.go +++ b/internal/core/setfields_coercion_test.go @@ -113,6 +113,23 @@ func TestSetFields_RejectsNonNumericTypedField(t *testing.T) { } } +func TestSetFields_RejectsEveryGraphOwnedFieldEvenWithForce(t *testing.T) { + for _, field := range []string{"depends_on", "blocked_by", "dependencies", "blocks"} { + for _, force := range []bool{false, true} { + svc := setFieldsRepo(t) + _, err := svc.SetFields("t", map[string]any{field: testutil.TaskID("other")}, force, false) + if !errors.Is(err, domain.ErrValidation) || !strings.Contains(err.Error(), "guarded dependency") { + t.Fatalf("field=%s force=%v error=%v; want guarded-operation direction", field, force, err) + } + task, _, showErr := svc.ShowTask("t") + if showErr != nil || len(task.DependsOn) != 0 || len(task.LegacyBlockedBy) != 0 || + len(task.LegacyDependencies) != 0 || len(task.LegacyBlocks) != 0 { + t.Fatalf("field=%s force=%v changed task after rejection: task=%+v err=%v", field, force, task, showErr) + } + } + } +} + // TestSetFields_RejectsUnknownEpic mirrors NewTask: set can't orphan a task onto // a non-existent epic. func TestSetFields_RejectsUnknownEpic(t *testing.T) { diff --git a/internal/domain/entity.go b/internal/domain/entity.go index 4fc5d5e1..b096040a 100644 --- a/internal/domain/entity.go +++ b/internal/domain/entity.go @@ -71,6 +71,7 @@ var entities = []Descriptor{ }, Conventions: []string{ "status lives in frontmatter (authoritative) and is changed only via the lifecycle verbs (start/next/complete/…), which edit it in place — don't edit it directly.", + "depends_on is a sorted set of stable task IDs owned by the repository-global DAG; use `task depend add/remove` once available — generic `task set` and `task edit` cannot change it.", fmt.Sprintf("description is a single line, ≤%d characters.", MaxDescriptionLen), "at least one tag is required at creation.", "the filename slug is derived from the title; any title is accepted (colons, dashes, arrows, …) and the full title is kept as the body H1.", diff --git a/internal/domain/fields.go b/internal/domain/fields.go index 905d5503..fe56b1ac 100644 --- a/internal/domain/fields.go +++ b/internal/domain/fields.go @@ -30,6 +30,7 @@ var taskFields = []taskField{ {"tier", "int"}, {"autonomy_level", "int"}, {"tags", "list"}, + {"depends_on", "list"}, {"related_tasks", "list"}, {"dependencies", "list"}, {"blocks", "list"}, @@ -81,6 +82,19 @@ func IsListField(f string) bool { return listFields[f] } // must not silently persist (decided 2026-06-12). func KnownTaskField(f string) bool { return knownTaskFields[f] } +// IsGraphOwnedTaskField reports whether changing a field changes the repository- +// global dependency graph. The canonical field and all supported legacy aliases +// are read-only through generic set/edit paths: canonical writes need guarded +// validation, while legacy values may only be removed by the guarded migration. +func IsGraphOwnedTaskField(f string) bool { + switch f { + case "depends_on", "blocked_by", "dependencies", "blocks": + return true + default: + return false + } +} + // UnsetField is a sentinel value in a SetFields update map: the key is // removed from the frontmatter instead of being assigned. It exists so field // removal flows through the same validated, surgical, atomic write path as diff --git a/internal/domain/lint.go b/internal/domain/lint.go index bf0292a4..e21765b0 100644 --- a/internal/domain/lint.go +++ b/internal/domain/lint.go @@ -25,12 +25,25 @@ func EpicNameIssue(id string) []Issue { return []Issue{{Field: "filename", Message: fmt.Sprintf("epic filename %q should be NN- (a zero-padded number) — rename it so epics order consistently", id)}} } -// Issue is a single frontmatter lint finding. +// IssueSeverity distinguishes non-blocking migration guidance from the +// established default validation-error behavior. +type IssueSeverity string + +const IssueAdvisory IssueSeverity = "advisory" + +// Issue is a single frontmatter lint finding. Empty Severity is the established +// blocking error behavior; advisory findings remain visible but do not make the +// ordinary lint command fail. type Issue struct { Field string `json:"field"` Message string `json:"message"` + // Severity is "advisory" for visible non-blocking debt and is omitted for + // ordinary validation errors, preserving the existing wire shape. + Severity IssueSeverity `json:"severity,omitempty"` } +func (i Issue) Blocking() bool { return i.Severity != IssueAdvisory } + // DuplicateEpicNNIssues flags epics that share a leading NN key. Two epics on the same key // (e.g. `01-a`, `01-b`) co-mingle their tasks in the rollup and canonicalEpic silently // resolves an `epic:` ref to the first — an invalid state nothing else enforces. Returns one diff --git a/internal/domain/schema_test.go b/internal/domain/schema_test.go index 25f87716..e8e2af2a 100644 --- a/internal/domain/schema_test.go +++ b/internal/domain/schema_test.go @@ -98,7 +98,7 @@ func TestTaskFieldsMatchStruct(t *testing.T) { func TestFieldType(t *testing.T) { for name, want := range map[string]string{ "tier": "int", "autonomy_level": "int", - "tags": "list", "dependencies": "list", + "tags": "list", "dependencies": "list", "depends_on": "list", "created": "date", "audited": "date", "description": "string", "epic": "string", "nonexistent": "string", } { diff --git a/internal/domain/task.go b/internal/domain/task.go index 6ca2914a..6ea3ab83 100644 --- a/internal/domain/task.go +++ b/internal/domain/task.go @@ -33,4 +33,18 @@ type Task struct { StartedAt string `yaml:"started_at"` // stamped when a task enters in-progress (incl. `new --start`) RevisitAt string `yaml:"revisit_at,omitempty"` // optional "snooze until" date for a deferred task (set by `task defer`) Tags []string `yaml:"tags"` + + // DependsOn is the canonical repository-global dependency set from ADR-0006. + // Values are stable task IDs, never slugs. Valid writers serialize the semantic + // set in sorted order; readers deliberately retain malformed duplicate values so + // the strict graph snapshot and lint can diagnose hand-edited files precisely. + DependsOn []string `yaml:"depends_on,omitempty"` + + // These fields are read-only legacy vocabulary. Keeping them on the typed record + // lets the strict snapshot resolve and diagnose the live slug references without + // treating them as canonical edges or silently dropping them during analysis. The + // guarded dependency-migration slice removes them later. + LegacyBlockedBy []string `yaml:"blocked_by,omitempty"` + LegacyDependencies []string `yaml:"dependencies,omitempty"` + LegacyBlocks []string `yaml:"blocks,omitempty"` } diff --git a/internal/store/create.go b/internal/store/create.go index 114e3cc0..24b7c116 100644 --- a/internal/store/create.go +++ b/internal/store/create.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "regexp" + "sort" "strconv" yaml "go.yaml.in/yaml/v3" @@ -79,6 +80,11 @@ func taskFields(t domain.Task) []fmField { {"tags", t.Tags}, {"created", t.Created}, } + if len(t.DependsOn) > 0 { + dependencies := append([]string(nil), t.DependsOn...) + sort.Strings(dependencies) + fields = append(fields, fmField{"depends_on", dependencies}) + } if t.StartedAt != "" { fields = append(fields, fmField{"started_at", t.StartedAt}) } @@ -113,6 +119,9 @@ func (s *FS) CreateTask(t domain.Task, body string, dryRun bool) (domain.Task, e if err := validEntityID(t.ID); err != nil { return domain.Task{}, err } + if len(t.DependsOn) > 0 || len(t.LegacyBlockedBy) > 0 || len(t.LegacyDependencies) > 0 || len(t.LegacyBlocks) > 0 { + return domain.Task{}, fmt.Errorf("%w: task creation cannot set graph-owned dependency fields until guarded dependency creation is available", domain.ErrValidation) + } // The id makes the flat filename unique, so writeNewFile's O_EXCL is the whole // collision guard — no cross-dir slug scan. A duplicate slug (distinct id) is // allowed under the flat layout and stays resolvable by id. diff --git a/internal/store/dependency_persistence_test.go b/internal/store/dependency_persistence_test.go new file mode 100644 index 00000000..ca5b909f --- /dev/null +++ b/internal/store/dependency_persistence_test.go @@ -0,0 +1,146 @@ +package store + +import ( + "errors" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/andy-esch/taskflow/internal/domain" + "github.com/andy-esch/taskflow/internal/testutil" +) + +func TestTaskDependencyFieldsRoundTrip(t *testing.T) { + root := t.TempDir() + first, second := testutil.TaskID("first"), testutil.TaskID("second") + content := "---\nid: " + testutil.TaskID("dependent") + "\nstatus: ready-to-start\n" + + "depends_on: [" + second + ", " + first + "]\n" + + "blocked_by: [legacy-a]\ndependencies: [legacy-b]\nblocks: [legacy-c]\n---\n# dependent\n" + writeTask(t, root, "ready-to-start", "dependent.md", content) + task, _, err := NewFS(root).GetTask("dependent") + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(task.DependsOn, []string{second, first}) { + t.Fatalf("reader must retain raw dependency evidence, got %v", task.DependsOn) + } + if !reflect.DeepEqual(task.LegacyBlockedBy, []string{"legacy-a"}) || + !reflect.DeepEqual(task.LegacyDependencies, []string{"legacy-b"}) || + !reflect.DeepEqual(task.LegacyBlocks, []string{"legacy-c"}) { + t.Fatalf("legacy fields did not round-trip: %+v", task) + } +} + +func TestCreateTaskRejectsDependenciesUntilGuardedCreationExists(t *testing.T) { + root := t.TempDir() + first, second := testutil.TaskID("first"), testutil.TaskID("second") + task := domain.Task{ + ID: testutil.TaskID("dependent"), Slug: "dependent", Status: domain.StatusReadyToStart, + DependsOn: []string{second, first}, + } + if _, err := NewFS(root).CreateTask(task, "# dependent\n", false); !errors.Is(err, domain.ErrValidation) || !strings.Contains(err.Error(), "graph-owned") { + t.Fatalf("unguarded create error = %v", err) + } + if entries, err := os.ReadDir(filepath.Join(root, "tasks")); err == nil && len(entries) != 0 { + t.Fatalf("rejected create wrote files: %v", entries) + } + + // Keep the serializer deterministic for the future guarded create primitive, + // without exposing it through today's public store write. + raw, err := buildFile(taskFields(task), "# dependent\n") + if err != nil { + t.Fatal(err) + } + want := "depends_on: [" + first + ", " + second + "]" + if !strings.Contains(string(raw), want) { + t.Fatalf("created dependency order is not stable; want %q\n%s", want, raw) + } +} + +func TestEditTaskRejectsDependencyDeltaButAllowsReordering(t *testing.T) { + root := t.TempDir() + first, second := testutil.TaskID("first"), testutil.TaskID("second") + original := "---\nid: " + testutil.TaskID("dependent") + "\nstatus: ready-to-start\n" + + "depends_on: [" + second + ", " + first + "]\n---\n# dependent\n" + writeTask(t, root, "ready-to-start", "dependent.md", original) + fs := NewFS(root) + + attempts := 0 + _, changed, err := fs.EditTask("dependent", bodyNow, func(current string, prevErr error) (string, error) { + attempts++ + if attempts == 1 { + return strings.Replace(current, second+", "+first, first, 1), nil + } + if prevErr == nil || !strings.Contains(prevErr.Error(), "guarded dependency") { + t.Fatalf("reopened without guarded dependency direction: %v", prevErr) + } + return current, nil // give up on the rejected edit + }) + if changed || !errors.Is(err, domain.ErrValidation) { + t.Fatalf("dependency delta changed=%v err=%v", changed, err) + } + + reordered := strings.Replace(original, second+", "+first, first+", "+second, 1) + _, changed, err = fs.EditTask("dependent", bodyNow, func(string, error) (string, error) { return reordered, nil }) + if err != nil || !changed { + t.Fatalf("order-only edit changed=%v err=%v", changed, err) + } +} + +func TestEditTaskRejectsLegacyDependencyDeltaButAllowsReordering(t *testing.T) { + root := t.TempDir() + original := "---\nid: " + testutil.TaskID("dependent") + "\nstatus: ready-to-start\n" + + "blocked_by: [legacy-b, legacy-a]\ndependencies: [legacy-c]\nblocks: [legacy-d]\n---\n# dependent\n" + writeTask(t, root, "ready-to-start", "dependent.md", original) + fs := NewFS(root) + + attempts := 0 + _, changed, err := fs.EditTask("dependent", bodyNow, func(current string, prevErr error) (string, error) { + attempts++ + if attempts == 1 { + return strings.Replace(current, "dependencies: [legacy-c]", "dependencies: [legacy-new]", 1), nil + } + if prevErr == nil || !strings.Contains(prevErr.Error(), "guarded dependency") { + t.Fatalf("reopened without guarded dependency direction: %v", prevErr) + } + return current, nil + }) + if changed || !errors.Is(err, domain.ErrValidation) { + t.Fatalf("legacy dependency delta changed=%v err=%v", changed, err) + } + + reordered := strings.Replace(original, "legacy-b, legacy-a", "legacy-a, legacy-b", 1) + _, changed, err = fs.EditTask("dependent", bodyNow, func(string, error) (string, error) { return reordered, nil }) + if err != nil || !changed { + t.Fatalf("legacy order-only edit changed=%v err=%v", changed, err) + } +} + +func TestEditTaskMalformedDependencyCannotBeDeletedAsRepair(t *testing.T) { + root := t.TempDir() + taskID := testutil.TaskID("malformed-dependent") + original := "---\nid: " + taskID + "\nstatus: ready-to-start\ndepends_on: " + testutil.TaskID("prerequisite") + "\n---\n# dependent\n" + writeTask(t, root, "ready-to-start", "malformed-dependent.md", original) + fs := NewFS(root) + + attempts := 0 + _, changed, err := fs.EditTask(taskID, bodyNow, func(current string, prevErr error) (string, error) { + attempts++ + if attempts == 1 { + return strings.Replace(current, "depends_on: "+testutil.TaskID("prerequisite")+"\n", "", 1), nil + } + if prevErr == nil || !strings.Contains(prevErr.Error(), "cannot verify") { + t.Fatalf("malformed graph edit was not rejected clearly: %v", prevErr) + } + return current, nil + }) + if changed || !errors.Is(err, domain.ErrValidation) { + t.Fatalf("malformed dependency deletion changed=%v err=%v", changed, err) + } + raw, readErr := os.ReadFile(filepath.Join(root, "tasks", taskID+"-malformed-dependent.md")) + if readErr != nil || string(raw) != original { + t.Fatalf("rejected edit changed source: err=%v\n%s", readErr, raw) + } +} diff --git a/internal/store/edit.go b/internal/store/edit.go index 92037743..0ac9435e 100644 --- a/internal/store/edit.go +++ b/internal/store/edit.go @@ -4,8 +4,12 @@ import ( "errors" "fmt" "os" + "slices" + "sort" "time" + yaml "go.yaml.in/yaml/v3" + "github.com/andy-esch/taskflow/internal/domain" ) @@ -151,10 +155,26 @@ func (s *FS) EditTask(slug string, now time.Time, edit func(current string, prev return domain.Task{}, false, fmt.Errorf("read task %s: %w", path, err) } ifVersion := hashContent(orig) - return editFile("task", path, orig, now, - acceptEdited( + originalDependencies, dependenciesReadable := dependencyValues(orig) + parseAcceptedTask := func(content []byte) (domain.Task, error) { + t, err := acceptEdited( func(content []byte) (domain.Task, error) { return parseTask(content, path) }, - func(t domain.Task) string { return t.ID }), + func(t domain.Task) string { return t.ID })(content) + if err != nil { + return t, err + } + candidate := dependencyFieldsFromTask(t) + if !dependenciesReadable { + // Parser failure is not an empty dependency set. Reject every candidate so + // deleting the malformed field cannot sneak through as a "repair". + return t, fmt.Errorf("%w: cannot verify the original graph-owned fields while repairing malformed frontmatter; repair them directly, run lint, then use guarded dependency operations", domain.ErrValidation) + } else if !candidate.equal(originalDependencies) { + return t, fmt.Errorf("%w: task edit cannot change depends_on or legacy dependency fields; use guarded dependency operations", domain.ErrValidation) + } + return t, nil + } + return editFile("task", path, orig, now, + parseAcceptedTask, s.writeLock, // Version-CAS across the (long) editor window: conflict if the file relocated (a // concurrent `task move` → resurrect hazard) OR its content changed under us. @@ -162,6 +182,62 @@ func (s *FS) EditTask(slug string, now time.Time, edit func(current string, prev edit) } +type taskDependencyFields struct { + dependsOn []string + blockedBy []string + dependencies []string + blocks []string +} + +func sortedCopy(values []string) []string { + out := append([]string(nil), values...) + sort.Strings(out) + return out +} + +func dependencyFieldsFromTask(task domain.Task) taskDependencyFields { + return taskDependencyFields{ + dependsOn: sortedCopy(task.DependsOn), + blockedBy: sortedCopy(task.LegacyBlockedBy), + dependencies: sortedCopy(task.LegacyDependencies), + blocks: sortedCopy(task.LegacyBlocks), + } +} + +func (fields taskDependencyFields) equal(other taskDependencyFields) bool { + return slices.Equal(fields.dependsOn, other.dependsOn) && + slices.Equal(fields.blockedBy, other.blockedBy) && + slices.Equal(fields.dependencies, other.dependencies) && + slices.Equal(fields.blocks, other.blocks) +} + +// dependencyValues extracts every dependency-affecting field from an original +// task. A narrow decode can recover the graph baseline even when an unrelated +// typed field (for example tier) is malformed, preserving task edit's existing +// ability to repair such files without opening a dependency bypass. False means +// the YAML itself is not trustworthy. +func dependencyValues(content []byte) (taskDependencyFields, bool) { + fm, _, err := splitFrontmatterStrict(content) + if err != nil || fm == nil { + return taskDependencyFields{}, false + } + var fields struct { + DependsOn []string `yaml:"depends_on"` + BlockedBy []string `yaml:"blocked_by"` + Dependencies []string `yaml:"dependencies"` + Blocks []string `yaml:"blocks"` + } + if err := yaml.Unmarshal(fm, &fields); err != nil { + return taskDependencyFields{}, false + } + return taskDependencyFields{ + dependsOn: sortedCopy(fields.DependsOn), + blockedBy: sortedCopy(fields.BlockedBy), + dependencies: sortedCopy(fields.Dependencies), + blocks: sortedCopy(fields.Blocks), + }, true +} + // EditAudit is the audit twin of EditTask: same parse-before-accept editor-loop, // with the compare-and-swap guarding against a concurrent `audit close`/`reopen`/ // `defer` relocating the file across buckets during the editor window. Finding-level diff --git a/internal/store/fix.go b/internal/store/fix.go index 9b08ed8e..c7f29059 100644 --- a/internal/store/fix.go +++ b/internal/store/fix.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "strings" "time" @@ -51,6 +52,15 @@ func (s *FS) FixFrontmatter(dryRun bool) ([]domain.FixResult, error) { if err != nil { return fmt.Errorf("read %s: %w", path, err) } + if dir == s.tasksDir { + if guarded := graphOwnedFixChanges(content); len(guarded) > 0 { + results = append(results, domain.FixResult{ + Path: path, Skipped: true, + Changes: []string{fmt.Sprintf("graph-owned repair refused (%s); repair deliberately, run lint, then use guarded dependency operations", strings.Join(guarded, ", "))}, + }) + continue + } + } fixed, changes := fixFrontmatterText(content) // An id-led name whose id is misspelled is repairable in place: i/l/o have a // canonical Crockford decode, so the same identity can be spelled legally @@ -333,6 +343,13 @@ func isIdentifier(s string) bool { } func fixValue(key, value string) (fixed, change string) { + if domain.IsGraphOwnedTaskField(key) { + return value, "" + } + return fixValueAllowed(key, value) +} + +func fixValueAllowed(key, value string) (fixed, change string) { if value == "" { return value, "" // empty (e.g. a block list/map follows) } @@ -359,6 +376,28 @@ func fixValue(key, value string) (fixed, change string) { return value, "" } +func graphOwnedFixChanges(content []byte) []string { + fm, _ := splitFrontmatter(content) + if fm == nil { + return nil + } + seen := make(map[string]bool) + var changes []string + for _, rawLine := range strings.Split(string(fm), "\n") { + line := strings.TrimSuffix(rawLine, "\r") + key, value, ok := splitKeyValue(line) + if !ok || !domain.IsGraphOwnedTaskField(key) || seen[key] { + continue + } + if _, change := fixValueAllowed(key, value); change != "" { + seen[key] = true + changes = append(changes, key) + } + } + sort.Strings(changes) + return changes +} + // splitInlineComment separates an unquoted YAML scalar from a trailing // `# comment`. A '#' begins a comment only when preceded by whitespace, so a // value like `C# rocks` is left intact. diff --git a/internal/store/fix_test.go b/internal/store/fix_test.go index 50c99d24..805730ad 100644 --- a/internal/store/fix_test.go +++ b/internal/store/fix_test.go @@ -84,6 +84,26 @@ func TestFixFrontmatterText_Idempotent(t *testing.T) { } } +func TestFixFrontmatterRefusesGraphOwnedNormalization(t *testing.T) { + root := t.TempDir() + taskID := testutil.TaskID("graph-fix") + path := filepath.Join(root, "tasks", taskID+"-graph-fix.md") + original := "---\nid: " + taskID + "\nstatus: ready-to-start\ndepends_on: " + testutil.TaskID("first") + ", " + testutil.TaskID("second") + "\ntags: one,two\n---\n# task\n" + testutil.Write(t, path, original) + + results, err := NewFS(root).FixFrontmatter(false) + if err != nil { + t.Fatal(err) + } + if len(results) != 1 || !results[0].Skipped || !strings.Contains(results[0].Changes[0], "graph-owned") { + t.Fatalf("graph repair result = %+v", results) + } + raw, err := os.ReadFile(path) + if err != nil || string(raw) != original { + t.Fatalf("guarded file changed: err=%v\n%s", err, raw) + } +} + func TestFS_FixFrontmatter_DryRunThenWrite(t *testing.T) { root := t.TempDir() path, out := testutil.TaskFixture(root, "ready-to-start", "bad.md", "---\nstatus: ready-to-start\ntags: a,b\n---\n# B\n") diff --git a/internal/store/fsstore.go b/internal/store/fsstore.go index 54e09823..e57ed034 100644 --- a/internal/store/fsstore.go +++ b/internal/store/fsstore.go @@ -259,6 +259,11 @@ func (s *FS) SetFields(slug string, updates map[string]any, dryRun bool) (domain if _, ok := updates["status"]; ok { return domain.Task{}, fmt.Errorf("%w: status is not a settable field — use Move", domain.ErrValidation) } + for field := range updates { + if domain.IsGraphOwnedTaskField(field) { + return domain.Task{}, fmt.Errorf("%w: %s is graph-owned — use guarded dependency operations", domain.ErrValidation, field) + } + } path, err := s.resolve(slug) if err != nil { return domain.Task{}, err diff --git a/internal/store/setfields_test.go b/internal/store/setfields_test.go index 142ae2db..f5a6b7bd 100644 --- a/internal/store/setfields_test.go +++ b/internal/store/setfields_test.go @@ -1,11 +1,13 @@ package store import ( + "errors" "os" "path/filepath" "strings" "testing" + "github.com/andy-esch/taskflow/internal/domain" "github.com/andy-esch/taskflow/internal/testutil" yaml "go.yaml.in/yaml/v3" ) @@ -49,6 +51,17 @@ func TestFS_SetFields(t *testing.T) { } } +func TestFS_SetFieldsRejectsEveryGraphOwnedField(t *testing.T) { + for _, field := range []string{"depends_on", "blocked_by", "dependencies", "blocks"} { + root := t.TempDir() + writeTask(t, root, "ready-to-start", "alpha.md", editSeed) + _, err := NewFS(root).SetFields("alpha", map[string]any{field: []string{testutil.TaskID("beta")}}, false) + if !errors.Is(err, domain.ErrValidation) || !strings.Contains(err.Error(), "graph-owned") { + t.Fatalf("SetFields %s error = %v", field, err) + } + } +} + func TestFS_SetFields_NotFound(t *testing.T) { _, err := NewFS(t.TempDir()).SetFields("ghost", map[string]any{"priority": "low"}, false) if err == nil { diff --git a/internal/wire/dto.go b/internal/wire/dto.go index 4c57a6dd..bab2e0bf 100644 --- a/internal/wire/dto.go +++ b/internal/wire/dto.go @@ -1,6 +1,8 @@ package wire import ( + "sort" + "github.com/andy-esch/taskflow/internal/core" "github.com/andy-esch/taskflow/internal/domain" ) @@ -36,6 +38,7 @@ type TaskJSON struct { Updated string `json:"updated_at,omitempty" jsonschema:"description=last-modified date YYYY-MM-DD"` RevisitAt string `json:"revisit_at,omitempty" jsonschema:"description=snooze-until date YYYY-MM-DD for a deferred task (set by task defer)"` Tags []string `json:"tags,omitempty" jsonschema:"description=topical tags"` + DependsOn []string `json:"depends_on,omitempty" jsonschema:"description=sorted stable task IDs that must be soundly completed before this task is ordinarily eligible to start"` } // ToTaskJSON maps a domain task to its wire DTO. @@ -45,10 +48,17 @@ func ToTaskJSON(t domain.Task) TaskJSON { Description: t.Description, Effort: t.Effort, Tier: t.Tier, Priority: t.Priority, Autonomy: t.Autonomy, Created: t.Created, Updated: t.Updated, RevisitAt: t.RevisitAt, Tags: t.Tags, + DependsOn: sortedStrings(t.DependsOn), } return j } +func sortedStrings(values []string) []string { + out := append([]string(nil), values...) + sort.Strings(out) + return out +} + // ACJSON is a task's acceptance-criteria checkbox tally (the `ac` field of // `task info`): how many criteria are checked out of the total. type ACJSON struct { diff --git a/internal/wire/dto_test.go b/internal/wire/dto_test.go index a1a26c77..a7d2c19e 100644 --- a/internal/wire/dto_test.go +++ b/internal/wire/dto_test.go @@ -27,6 +27,19 @@ func TestToTaskJSON_CarriesID(t *testing.T) { } } +func TestToTaskJSON_CarriesStableDependencyOrderWithoutMutatingDomain(t *testing.T) { + original := []string{"600000000003", "600000000001", "600000000002"} + task := domain.Task{ID: "600000000004", DependsOn: append([]string(nil), original...)} + got := ToTaskJSON(task) + want := []string{"600000000001", "600000000002", "600000000003"} + if !reflect.DeepEqual(got.DependsOn, want) { + t.Fatalf("depends_on = %v, want %v", got.DependsOn, want) + } + if !reflect.DeepEqual(task.DependsOn, original) { + t.Fatalf("wire mapper mutated domain dependencies: %v", task.DependsOn) + } +} + // TestToAuditJSON_CarriesID mirrors the task check for audits. func TestToAuditJSON_CarriesID(t *testing.T) { got := ToAuditJSON(domain.Audit{Slug: "2026-01-02-x", Bucket: domain.AuditOpen, ID: "6fjangd7kvh3"}) diff --git a/internal/wire/schema_comments.json b/internal/wire/schema_comments.json index ad6802b3..8460d97f 100644 --- a/internal/wire/schema_comments.json +++ b/internal/wire/schema_comments.json @@ -49,6 +49,8 @@ "github.com/andy-esch/taskflow/internal/domain.FixResult": "FixResult records the auto-repairs applied (or proposed) for one file.", "github.com/andy-esch/taskflow/internal/domain.FixResult.Skipped": "Skipped marks a file the pass deliberately did NOT repair, with Changes carrying the\nreason. Reported alongside repairs because the reason is the useful part — \"this id\nis still referenced by three files\" is what the operator has to act on — but counted\nseparately, since calling a refusal a fix is how a tool loses trust.", "github.com/andy-esch/taskflow/internal/domain.Issue": "Issue is a single frontmatter lint finding.", + "github.com/andy-esch/taskflow/internal/domain.Issue.Severity": "Severity is \"advisory\" for visible non-blocking debt and is omitted for\nordinary validation errors, preserving the existing wire shape.", + "github.com/andy-esch/taskflow/internal/domain.IssueSeverity": "IssueSeverity distinguishes non-blocking migration guidance from the established default validation-error behavior.", "github.com/andy-esch/taskflow/internal/domain.NamedTemplate": "NamedTemplate is one body scaffold a kind offers under a name.", "github.com/andy-esch/taskflow/internal/domain.Placeholder": "Placeholder is a {{Key}} token a kind's templates may fill: the real value at create time, or Label in a placeholder preview (`template show` / `schema`).", "github.com/andy-esch/taskflow/internal/domain.Placeholder.Key": "the {{Key}} token, e.g. \"title\"", @@ -62,8 +64,10 @@ "github.com/andy-esch/taskflow/internal/domain.Span": "Span is a byte range in the audit body: [Start, End).", "github.com/andy-esch/taskflow/internal/domain.Status": "Status is a task lifecycle state, authoritative in frontmatter (ADR-0003 §4).", "github.com/andy-esch/taskflow/internal/domain.Task": "Task is a planning task.", + "github.com/andy-esch/taskflow/internal/domain.Task.DependsOn": "DependsOn is the canonical repository-global dependency set from ADR-0006.\nValues are stable task IDs, never slugs. Valid writers serialize the semantic\nset in sorted order; readers deliberately retain malformed duplicate values so\nthe strict graph snapshot and lint can diagnose hand-edited files precisely.", "github.com/andy-esch/taskflow/internal/domain.Task.FilenameID": "FilenameID is that same id as parsed from the flat filename's leading field\n(set by the store via splitFlatName). It is the canonical key resolveID/CAS\nmatch on; the frontmatter `id:` above is a co-located copy that must equal it,\nand lint flags any drift (IDDriftIssue). Derived, not frontmatter.", "github.com/andy-esch/taskflow/internal/domain.Task.ID": "ID is the stable 12-char identifier (ADR-0003 §3): it leads the flat filename\n(tasks/\u003cid\u003e-\u003cslug\u003e.md) and is the primary resolution key.", + "github.com/andy-esch/taskflow/internal/domain.Task.LegacyBlockedBy": "These fields are read-only legacy vocabulary. Keeping them on the typed record\nlets the strict snapshot resolve and diagnose the live slug references without\ntreating them as canonical edges or silently dropping them during analysis. The\nguarded dependency-migration slice removes them later.", "github.com/andy-esch/taskflow/internal/domain.Task.RevisitAt": "optional \"snooze until\" date for a deferred task (set by `task defer`)", "github.com/andy-esch/taskflow/internal/domain.Task.StartedAt": "stamped when a task enters in-progress (incl. `new --start`)", "github.com/andy-esch/taskflow/internal/domain.Task.StatusFellBack": "StatusFellBack is set by the store when the frontmatter status is missing or\nunrecognized — under the flat layout (ADR-0003 §4) there is no directory to fall\nback to, so Status keeps its raw value; the task still lists and lint flags it\n(FrontmatterStatusIssues).", diff --git a/internal/wire/wire.go b/internal/wire/wire.go index 0896c736..0195478b 100644 --- a/internal/wire/wire.go +++ b/internal/wire/wire.go @@ -170,6 +170,13 @@ import ( // point selected for reading, and a combined space-badged in-progress working set. The // envelope owns one top-level schema_version; nested summaries reuse the versionless // SummaryJSON payload rather than pretending to be independent envelopes. +// 1.49: task payloads carry `depends_on`, the sorted stable IDs of repository-global +// prerequisites declared by that task. Additive and omitted for tasks without edges. +// The task field/schema contract also recognizes the persisted list while generic +// mutation remains forbidden until the guarded dependency commands land. Lint issues +// may carry `severity: "advisory"`; omitted severity retains the established blocking +// error behavior. +// // 1.48: the `schema` contract carries `criterion_states` — the non-binary acceptance // criterion states, published for the same reason `finding_statuses` is: `state` has been a // criterion wire field since 1.46, and without the set an agent had to trigger an error and @@ -201,7 +208,7 @@ import ( // 1.43: fresh `init --json` receipts may include `registration`, describing the // best-effort machine-local space registration (including preview vs applied and whether // the physical checkout was already registered). -const SchemaVersion = "1.48" +const SchemaVersion = "1.49" // EncodeJSON writes the payload as compact (un-indented) JSON with a single // trailing newline. Machine output: pretty-printing is pure token cost for a diff --git a/planning/adrs/0006-adopt-threads-as-task-dags.md b/planning/adrs/0006-adopt-threads-as-task-dags.md index ab08148b..50e36ba8 100644 --- a/planning/adrs/0006-adopt-threads-as-task-dags.md +++ b/planning/adrs/0006-adopt-threads-as-task-dags.md @@ -155,6 +155,17 @@ depends_on: [6fjangd7kvh0, 6fjangd7kvh2] - Graph mutation fails closed when the repository dependency graph cannot be read soundly. `lint` remains the fail-open, full-sweep diagnostic surface for hand-edited missing IDs, unreadable tasks, legacy fields, and cycles. +- Every immutable snapshot reports one repository-level health marker. `healthy` means the + canonical graph is valid and no legacy dependency vocabulary remains. `degraded` means every + legacy reference resolves exactly but has not yet been migrated; diagnostic reads may explain + it, while mutation and dispatch still fail closed. `broken` means any task is unreadable, an ID + or status is invalid, an edge is duplicate/self/invalid/missing, a legacy reference is missing or + ambiguous, or the graph is cyclic. `broken` takes precedence over `degraded`. +- Supported commands prevent graph-invalid states rather than relying on lint after the fact. + Generic set/edit paths treat both `depends_on` and the legacy dependency fields as graph-owned, + including under `--force`; guarded dependency commands are the only product write path. Direct + filesystem edits and older binaries remain possible, so every structural defect and legacy + occurrence must also appear during an ordinary `lint` call with deterministic attribution. - The existing unmodelled `dependencies`, `blocked_by`, and `blocks` fields are legacy vocabulary. Implementation must migrate the six current `blocked_by` users or report them with actionable lint, then converge on `depends_on` alone. The unused task-level `projects` field is deprecated @@ -186,7 +197,15 @@ them into a shadow status vocabulary: (`completed`), or withdrawn (`deprecated`). - **Gate state** is `clear` when every prerequisite is soundly completed; `blocked` when the graph is readable but at least one prerequisite is not soundly completed; or `broken` when an upstream - path contains a missing, unreadable, or withdrawn prerequisite. + path contains a missing, unreadable, withdrawn, invalid-status, cyclic, or otherwise recursively + broken prerequisite. + +Blocker projections use stable reason tokens: `not-started`, `in-flight`, `parked`, `withdrawn`, +`missing`, `unsound-completed`, `invalid-status`, and `cycle`. The last two are forensic vocabulary, +not states that supported commands may create: they let lint and diagnostic reads explain damage +from direct filesystem edits, old binaries, or an interrupted external writer without collapsing it +into a vague missing/blocked result. Every blocker also carries one deterministic shortest path from +the queried task to that blocker. Named views are compositions of those fields: @@ -658,6 +677,51 @@ implicit. The following amendments supersede conflicting wording above: IDs, reason/path data, and taskflow-owned error vocabulary; concurrency attribution may enrich an error but does not change a cycle from validation into a retryable conflict. +### 2026-08-26: Dependency-foundation adversarial hardening + +Two independent implementation audits—[Gemini](../audits/6g417v97bx8s-2026-08-26-canonical-task-dependency-read-foundation.md) +and [Claude](../audits/6g41amrnje2j-2026-08-26-canonical-task-dependency-read-foundation-claude.md)—found +places where the first read foundation was safe in aggregate but its individual APIs and repair paths +were too easy to misread or bypass. The following clarifications supersede conflicting wording above: + +1. **Cycle identity is an SCC property.** Validation identifies every member of each non-trivial + strongly connected component, plus a one-task component with a self-edge. Diagnostics emit one + deterministic representative edge-following cycle per component and attribute cyclic membership + to every affected task; they do not promise to enumerate every simple cycle. A self-edge is not + also reported as an indistinguishable generic cycle on the same task. +2. **Graph health qualifies every projection.** Canonical edges and exactly resolved legacy edges + are validated as one projected union before a snapshot may be called degraded. A resolvable + legacy self-edge or cycle is broken, not merely unmigrated. A topological plan may return useful + partial waves for diagnosis, but its completeness flag is true only for a healthy snapshot. +3. **Every first-party write honors graph ownership.** Generic set, edit, creation, and lint-repair + paths may neither add, delete, nor normalize canonical or legacy dependency fields. Until guarded + dependency creation exists, ordinary task creation rejects non-empty graph-owned fields. Malformed + graph frontmatter fails closed and must be repaired through an explicitly guarded migration or + deliberate filesystem edit; parser failure is never interpreted as an empty dependency set. +4. **Safe legacy debt is visible without poisoning routine lint.** An exactly resolved legacy + reference whose projected edge is structurally legal is an advisory in normal human and JSON lint + output and does not make lint exit non-zero. Missing, ambiguous, self-referential, or cyclic legacy + projections remain errors. Snapshot health remains degraded and mutation/dispatch remains closed + until the advisory debt is migrated. +5. **Authorization and explanation are separate contracts.** Lifecycle authorization uses the + typed derived state (`Eligible` and its gate), never the length of a blocker list. A gate + explanation includes that state, task-local structural problems, and an action-oriented blocking + frontier. The API also exposes a separately named full causal prerequisite projection for + forensic queries; neither projection's empty result is itself permission to start work. +6. **Blocking projections declare their traversal semantics.** The causal projection may traverse + through all reachable unsound prerequisites. The action frontier stops at terminal constraints + such as missing, unreadable, withdrawn, invalid, or cyclic records and otherwise returns the + deepest current constraints a user can act on. Both use stable reason tokens and deterministic + shortest paths. +7. **Complexity claims include output cost.** Snapshot state derivation remains O(V+E). A path + projection is O(V+E plus the size of the paths it returns); implementations keep predecessor + links during traversal and materialize paths only for emitted results. Lint uses the bounded + action frontier instead of expanding the full causal closure for every inconsistent task. +8. **Diagnostics preserve source identity.** Unreadable task filenames retain recoverable stable-ID + identity, invalid dependency tokens remain distinguishable from missing valid IDs, and duplicate + IDs are attributed to every source path without silently assigning one record's graph defects to + another. Strict mutation still fails closed when no unique authoritative record exists. + ## Related - Supersedes: [0002-adopt-projects](0002-adopt-projects.md). diff --git a/planning/audits/6g417v97bx8s-2026-08-26-canonical-task-dependency-read-foundation.md b/planning/audits/6g417v97bx8s-2026-08-26-canonical-task-dependency-read-foundation.md new file mode 100644 index 00000000..af099f88 --- /dev/null +++ b/planning/audits/6g417v97bx8s-2026-08-26-canonical-task-dependency-read-foundation.md @@ -0,0 +1,245 @@ +--- +schema: 1 +id: 6g417v97bx8s +bucket: closed +area: canonical-task-dependency-read-foundation +date: "2026-08-26" +--- + +# Audit: Canonical Task-Dependency Read Foundation — 2026-08-26 + +Adversarial implementation review of the canonical task-dependency read foundation in Taskflow (task `6g3q4rst78qy`, epic `30-threads-and-task-dependency-graphs`, branch `feat/canonical-task-dependency-reads`), stress-tested against ADR-0006, the production domain/store/wire models, and real repository data. + +**Executive Verdict: Safe with amendments.** The core architectural model holds: `depends_on` is cleanly modeled as a sorted, duplicate-free set of stable IDs; `TaskGraph` provides an immutable, thread-safe strict snapshot; reconvergent sound-completion memoization is proven O(V+E); and hard mutation guardrails on generic `task set` (including `--force`) and interactive `task edit` successfully prevent invalid graph states from entering the store. However, two defects must be resolved before this slice merges or immediately as part of slice 2: +1. Cycle detection in `deterministicCycles` (`internal/core/dependency_graph.go:795`) has an algorithmic blind spot on intersecting cycles (multi-branch feedback graphs), omitting cycles that traverse through completed DFS nodes (`state = 2`) and misclassifying participating tasks as unstarted or unsound-completed rather than `cycle` (H1). +2. The decision to make ordinary `tskflwctl lint` fail with exit code 11 on the six resolvable legacy `blocked_by` fields breaks repo CI while simultaneously prohibiting all CLI tools from repairing those fields until slice 2 ships the guarded migration (M1). + +Findings are classified as **[merge-blocking]**, **[pre-mutation prerequisite]**, or **[tracked follow-up]**. + +--- + +## Findings + +### High + +#### H1. Cycle detection in `deterministicCycles` misses cycles in multi-cycle or intersecting graphs, leaving cycle members misattributed · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:795-835 | **Component:** core / graph analysis +**Effort:** M · **Urgency:** acute + +**[pre-mutation prerequisite / merge-blocking for cycle correctness]** + +`deterministicCycles` implements a standard 3-color DFS (`state 0: unvisited, 1: visiting, 2: finished`) to extract attributable cycle paths. When a vertex finishes its post-order traversal, it is marked `state = 2`. The inner neighbor loop (`lines 807-820`) only handles `case 0` (unvisited) and `case 1` (back-edge to a node in the current recursion stack); it contains no handler for `case 2`. + +When a directed graph contains intersecting cycles (for example, two parallel branches `A -> B -> C -> A` and `A -> D -> C -> A` sharing node `C` and `A`, or any figure-8 feedback topology), the first cycle path marks node `C` as `state = 2`. When DFS subsequently traverses branch `D`, neighbor `C` is in `state = 2` and is skipped without further traversal. + +Consequently: +1. `structure.Cycles` records only `[A, B, C, A]` and misses `[A, D, C, A]`. +2. `g.cycleMembers[D]` remains `false`. +3. `g.Problems()` omits the cycle involving `D`. +4. When queried via `g.State(D)` or `g.Blockers(...)`, `g.blockerReason(D)` returns `BlockerNotStarted`, `BlockerInFlight`, or `BlockerUnsoundCompleted` instead of `BlockerCycle`. +5. In slice 2 (guarded dependency operations), an agent or developer diagnosing why task `D` is blocked will be told it is unstarted rather than cyclic. If the user breaks the edge `B -> C` to fix the first cycle, the second cycle `A -> D -> C -> A` will suddenly emerge on the next run, violating the guarantee of deterministic and complete cycle attribution. + +**Why tests missed it:** Existing contract and stress tests (`TestOwnedDAGAnalyzerContract`, `TestTaskGraphCycleBlockerReason`) only tested single, isolated 2-node or 3-node cycles (`a <-> b` and `c -> a -> b -> c`), never multi-cycle or intersecting graphs sharing vertices. + +**Recommendation:** Adopt Tarjan's Strongly Connected Components (SCC) algorithm (which is strictly O(V+E)) to identify all cyclic subgraphs. Every node in an SCC of size $> 1$ (or with a self-edge) is unconditionally marked in `g.cycleMembers`. Extract canonical cycle paths within each SCC. + +--- + +### Medium + +#### M1. Intentionally failing ordinary `lint` on resolvable legacy `blocked_by` fields breaks repository CI and leaves users without a CLI remediation path · **Status:** fixed 2026-08-27 + +**File:** internal/core/service.go:346-363, internal/cli/lint.go:62, planning/tasks/6g3q4rst78qy-establish-canonical-task-dependencies-and-strict-graph-reads.md:70-74 | **Component:** core / cli / lint +**Effort:** S · **Urgency:** acute + +**[merge-blocking / operational]** + +`dependencyLintIssues` (`service.go:346-363`) maps every `LegacyDependencyDiagnostic` into a `domain.Issue`. In `runLint` (`internal/cli/lint.go:62-65`), any issue causes `tskflwctl lint` to exit with status code 11 (`validation failed: 6 item(s) with issues`). Running `go run ./cmd/tskflwctl lint --json` against this repository confirms exit status 11 on the six existing `blocked_by` tasks. + +At the same time, `task set`, `task set --force`, and `task edit` strictly reject removing or modifying `blocked_by` (with error `"legacy dependency fields are removed only by the guarded migration"`). `lint --fix` also does not migrate or remove them. + +This creates an operational deadlock: +- Merging slice 1 to `main` immediately breaks `tskflwctl lint` in CI and developer pre-commit hooks. +- Developers cannot fix the legacy fields using any supported CLI command. +- The only options are disabling CI lint checks, making manual regex/editor filesystem edits (which the ADR and task documentation explicitly warn against), or waiting for slice 2 (`6g3q4rt7mgjn`) to land and execute the guarded migration. + +**Why tests missed it:** `internal/cli/lint_test.go:88-90` asserts that `lint` exits with 11 on legacy fields, treating this as a test pass rather than evaluating its impact on live repository workflows. + +**Recommendation:** Either: +1. Treat resolvable legacy references as non-fatal warnings in `lint` (reporting them on stderr/JSON while exiting 0, reserving exit 11 for unresolvable/ambiguous legacy references or broken graphs) until slice 2 ships the guarded migration; OR +2. Bundle the guarded migration into this slice or merge slice 1 and slice 2 in immediate sequence before tagging or enforcing CI lint gates. + +#### M2. Unreadable task files lose identity association, reporting downstream blockers as `missing` rather than `unreadable` · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:269-274, 349-352, 703-705 | **Component:** core / graph snapshot +**Effort:** S · **Urgency:** soon + +**[pre-mutation prerequisite]** + +`NewTaskGraph` accepts `unreadable []domain.FileProblem`. Because an unreadable task failed YAML parsing, it is not present in `g.tasks`. `ProblemUnreadable` is recorded in `g.problems` with `Path` populated but `TaskID: ""` (`lines 270-274`). + +When a valid downstream task declares `depends_on: [6fjangd7kvh0]`, and `tasks/6fjangd7kvh0-alpha.md` is the unreadable file, `!taskExists(g.tasks, prerequisite)` triggers `ProblemMissingDependency` (`"task beta depends on missing task 6fjangd7kvh0"`), and `g.Blockers(...)` reports `BlockerMissing` rather than an unreadable or corrupted blocker. + +If a developer introduces a YAML indentation error in a prerequisite task file, downstream tasks will claim that the prerequisite was deleted or never existed (`missing`), misleading users who see the file on disk. + +**Why tests missed it:** Test fixtures tested unreadable files and missing dependencies as disjoint scenarios, never asserting blocker reason fidelity for a valid task depending on a file that exists on disk but failed to parse. + +**Recommendation:** Extract the filename ID from `FileProblem.Path` (via `id.Extract` or `splitFlatName`) and record unreadable task IDs in a `map[string]domain.FileProblem` within `TaskGraph`. When `prerequisite` is in this unreadable map, emit a specific `ProblemUnreadableDependency` and assign `BlockerInvalidStatus` (or a dedicated `BlockerUnreadable` token) with the path to the unreadable file. + +--- + +### Low + +#### L1. Minimal `dagcontract.Run` suite provides insufficient validation for alternative DAG analyzers · **Status:** fixed 2026-08-27 + +**File:** internal/core/testdata/dagcontract/contract.go:14-43 | **Component:** core / testdata / dagcontract +**Effort:** S · **Urgency:** eventually + +**[tracked follow-up]** + +`dagcontract.Run` contains only three micro-fixtures: a 4-node wave, a 3-node cycle, and a 1-node self cycle. It omits multi-cycle graphs, disconnected frontiers, diamond reconvergences, and deep dependency chains. + +Any future spike or developer evaluating an external graph library (such as an updated `dominikbraun/graph` or `gonum`) against `dagcontract.Run` could observe 100% pass rates while the library actually fails on complex topological wave ties, deep recursion, or multi-cycle attribution. + +**Why tests missed it:** The contract was created as a minimal structural interface check rather than an exhaustive property-based benchmark. + +**Recommendation:** Expand `dagcontract.Run` to include the deep/wide and reconvergent stress tests currently present in `dependency_graph_test.go`. + +#### L2. Self-dependencies generate redundant dual diagnostics in lint output · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:335-342, 374-376, internal/core/service.go:329-345 | **Component:** core / graph analysis / lint +**Effort:** XS · **Urgency:** eventually + +**[tracked follow-up]** + +When a task declares `depends_on: [self_id]`, `newTaskGraph` emits `ProblemSelfDependency` (`"task X cannot depend on itself"`) and also appends the self-edge to `edges`. The analyzer then detects `[X, X]` as a cycle and emits `ProblemCycle` (`"dependency cycle: X -> X"`). In `dependencyLintIssues`, both issues are attached to task `X` under field `depends_on`. + +A user who accidentally adds a self-dependency sees two separate lint lines describing the identical 1-node defect. + +**Why tests missed it:** `TestLintReportsLegacyAndCanonicalDependencyDefects` explicitly asserted that both error messages were emitted simultaneously. + +**Recommendation:** Suppress the generic `ProblemCycle` when `ProblemSelfDependency` is already recorded for that task ID, or filter 1-node self-cycles from the cycle problem list. + +#### L3. Recursive DFS in `computeSound` creates unneeded stack frame allocation for deep graphs · **Status:** tracked by 6g3q4rt7mgjn + +**File:** internal/core/dependency_graph.go:503-536 | **Component:** core / graph algorithms +**Effort:** S · **Urgency:** eventually + +**[monitor / tracked follow-up]** + +`OwnedDAGAnalyzer.Analyze` uses iterative Kahn's algorithm for topological sorting, but `computeSound` uses recursive DFS across prerequisites. While memoization prevents exponential re-traversal, stack depth equals graph diameter. + +In an extreme artificial or generated graph with 20,000 linear chain tasks, `computeSound` will allocate 20,000 stack frames, placing unnecessary load on goroutine stack allocation. + +**Why tests missed it:** Deep chain tests evaluated `Analyze` at 2048 nodes, but `TestTaskGraphSoundCompletionMemoizesReconvergentDiamonds` only tested diamond chains up to depth 120. + +**Recommendation:** Replace recursive DFS in `computeSound` with an iterative post-order traversal or wave-based dynamic programming evaluation in a future optimization pass if planning graph diameters grow significantly. + +--- + +## Assumptions That Survived Adversarial Review + +These claims were independently checked, tested with counterexamples, and found sound: + +1. **`depends_on` Set Semantics and Stable Serialization:** + - Tasks store `DependsOn` as a stable task ID slice (`domain/task.go:41`). + - `CreateTask` sorts IDs stably in YAML (`store/create.go:83-87`). + - Domain `Task` reads raw values to preserve diagnostic fidelity for lint. + - Wire contract (`TaskJSON.DependsOn`) sorts IDs and omits when empty (`wire/dto.go:41, 51`). +2. **Reconvergent Diamond Memoization & O(V+E) Complexity:** + - Verified with visit-count assertions: `TestTaskGraphSoundCompletionMemoizesReconvergentDiamonds` proves that for a 120-layer reconvergent diamond (361 tasks, 480 edges), every task is computed exactly once in `computeSound` (`soundVisits == 1` for all nodes). + - `Blockers` and `Downstream` are memoized behind mutexes and return deep copies (`dependency_graph.go:647, 730`). +3. **Hard Gating on Generic Mutators (`task set` and `task edit`):** + - Verified: `task set` and `task set --force` strictly reject `depends_on`, `blocked_by`, `dependencies`, and `blocks` at both core (`Service.SetFields`) and store (`FS.SetFields`) layers. + - Verified: `task edit` rejects any dependency delta (`depends_on` or legacy fields), while correctly permitting formatting / order-only adjustments and non-graph frontmatter edits (`store/edit.go:158-177`). +4. **Precedence Rules and Blocker Explanations:** + - Broken precedence over blocked is strictly honored (`dependency_graph.go:565, 101`). + - `domain.StatusDeferred` (parked) is correctly evaluated as `GateBlocked` / `BlockerParked` (not broken). + - `domain.StatusDeprecated` (withdrawn) is correctly evaluated as `GateBroken` / `BlockerWithdrawn`. + - `unsound-completed` accurately identifies completed tasks whose prerequisites are incomplete. + - Shortest path BFS in `Blockers` produces deterministic, shortest explanatory paths with lexicographic tie-breaking (`dependency_graph.go:658-698`). +5. **Reopen Invalidation Semantics:** + - Changing an upstream completed task to `ready-to-start` immediately causes downstream completed tasks to derive `SoundlyCompleted: false`, `Drained: false`, `Gate: GateBlocked`, `Inconsistent: true` without rewriting frontmatter on disk (`dependency_graph_test.go:291-304`). +6. **Legacy Field Direction and Slug Resolution:** + - Inversion of `blocks` (`From: this_task, To: candidate`) vs `blocked_by` (`From: candidate, To: this_task`) is mathematically correct (`dependency_graph.go:484-488`). + - Exact ID resolution takes precedence over slug resolution. + - Ambiguous slugs (>1 task matching slug) and missing slugs/IDs are correctly flagged as `ProblemLegacyAmbiguous` / `ProblemLegacyMissing` and cause `GraphBroken` (`dependency_graph.go:477-494`). +7. **Thread Safety and Query Immutability:** + - All slice and map returns from `TaskGraph` (`Blockers`, `Downstream`, `TaskIDs`, `TopologicalWaves`, `Problems`, `LegacyDiagnostics`) return cloned copies, preventing caller mutation from corrupting graph caches. Mutex locks protect memoized caches. +8. **Wire Version Bump and Backward Compatibility:** + - `SchemaVersion` bumped to `"1.49"`. + - `depends_on` field in JSON DTO is `omitempty` and sorted. + - No breaking changes to existing fields or JSON envelopes. +9. **Multi-Repository Planning Space Compatibility:** + - Tasks reference other tasks by their 12-character stable task ID within the planning space. The graph engine operates purely on task IDs and statuses within the planning repository, regardless of how many implementation checkouts are coordinated. + +--- + +## Sequencing & Architectural Critique + +1. **Store vs Core Separation:** The analysis interface correctly lives in `internal/core/dependency_graph.go` as pure in-memory algorithms over `domain.Task` records. It does not touch the filesystem or hold locks. +2. **Reentrancy Preparation for Slice 2:** `Service.ReadTaskGraph()` provides the strict snapshot factory. In slice 2 (`6g3q4rt0wzkq`), `WithGraphMutation` in store will take the lock, obtain `ReadTaskGraph()`, invoke the pure planner, apply writes, and release. This split cleanly prepares for slice 2 without coupling core to filesystem locking. +3. **Migration Boundary:** Slice 1 correctly scopes migration to *diagnosis only*. However, coupling this diagnosis to fatal exit 11 in `lint` (M1) before the mutation command exists causes an operational bind. + +--- + +## Traceability Table + +| Finding | Severity | Classification | Action / Target Destination | +|---|---|---|---| +| **H1** (Intersecting cycle detection) | High | Fixed | `6g3q4rst78qy`: Tarjan SCC membership and representative-cycle tests | +| **M1** (Fatal lint on degraded legacy fields) | Medium | Fixed | safe legacy references are advisories with exit zero; unsafe projections fail | +| **M2** (Unreadable task identity loss) | Medium | Fixed | recover filename ID and expose the `unreadable` blocker reason | +| **L1** (Minimal DAG contract suite) | Low | Fixed | remove the one-implementation seam and retain direct adversarial graph tests | +| **L2** (Duplicate self-dependency diagnostics) | Low | Fixed | self-edge emits one specific diagnostic, not a second generic cycle issue | +| **L3** (Recursive DFS in `computeSound`) | Low | Tracked | depth-envelope criterion on `6g3q4rt7mgjn` | + +--- + +## Validation Commands and Results + +All checks executed in worktree `/Users/andyeschbacher/git/andy-esch/taskflow-canonical-task-dependency-reads` on branch `feat/canonical-task-dependency-reads`: + +1. **Full Unit Test Suite:** + ```bash + go test ./... + ``` + *Result:* Passed (25 test packages ok, ~21s total execution time). + +2. **Race Detection Test Suite:** + ```bash + go test -race ./... + ``` + *Result:* Passed (all test packages passed with zero race conditions detected). + +3. **CLI Lint Check against Live Planning Data:** + ```bash + go run ./cmd/tskflwctl lint --json + ``` + *Result:* Exit code 11 (validation failed). Emitted 6 legacy `blocked_by` issues on live tasks: + - `color-and-design-overhaul-one-coherent-palette-across-every-surface` + - `theme-config-table-and-selection-plumbing` + - `route-the-interactive-picker-theme-through-the-palette` + - `route-tui-chrome-through-the-palette` + - `theme-discovery-commands-glamour-polish-and-a-second-theme` + - `route-progress-bars-and-the-cli-ansi-map-through-the-palette` + +4. **Schema Wire Output:** + ```bash + go run ./cmd/tskflwctl schema --json + ``` + *Result:* Passed. Correctly reports `schema_version: "1.49"` and includes `depends_on` in `task_fields`. + +5. **Audit Lint Validation:** + ```bash + tskflwctl audit lint 2026-08-26-canonical-task-dependency-read-foundation + ``` + *Result:* Passed. + +## Remediation disposition (2026-08-27) + +The current slice absorbed H1, M1, M2, L1, and L2. L3 remains a measured optimization trigger, +tracked by the explicit deep-chain envelope criterion on `6g3q4rt7mgjn`; it is not treated as a +present correctness failure because the audit's 100,000-node counterexample completed correctly. +Final post-remediation validation is recorded in task `6g3q4rst78qy`. diff --git a/planning/audits/6g41amrnje2j-2026-08-26-canonical-task-dependency-read-foundation-claude.md b/planning/audits/6g41amrnje2j-2026-08-26-canonical-task-dependency-read-foundation-claude.md new file mode 100644 index 00000000..c8610c43 --- /dev/null +++ b/planning/audits/6g41amrnje2j-2026-08-26-canonical-task-dependency-read-foundation-claude.md @@ -0,0 +1,836 @@ +--- +schema: 1 +id: 6g41amrnje2j +bucket: closed +area: canonical-task-dependency-read-foundation-claude +date: "2026-08-26" +--- + +# Audit: canonical-task-dependency-read-foundation-claude — 2026-08-26 + +> Edit findings in place and flip each `**Status:**` as you work it. + +Adversarial implementation review of the canonical task-dependency read foundation +(task `6g3q4rst78qy`, epic `30-threads-and-task-dependency-graphs`, branch +`feat/canonical-task-dependency-reads`, uncommitted working tree vs `main` at +`43b3044`). Every claim below was checked against code and executed against the +compiled package; counterexamples were constructed with `go test -overlay` so that +no production or test file in the repository was modified. This audit is a second, +independent pass — a sibling audit `6g417v97bx8s` exists for the same slice; where +we agree that is noted as corroboration, and where its "survived adversarial +review" list is wrong that is stated explicitly. + +## Executive verdict + +**Not ready as-is; safe with amendments.** The architecture is right and the +fail-closed posture genuinely holds: no exercised path let an unsound graph state +reach a supported command, `-race` is clean, snapshot immutability is real, and the +blocker path projection is not merely deterministic but lexicographically minimal +and insertion-order independent. Sound completion really is memoized once per task. + +But three classes of defect should not merge into a foundation that seven downstream +tasks will build on: + +1. **Two exported read APIs are misleading in the exact way a future authorization + check will consume them.** `Blockers()` returns an empty slice for a task whose + own record is hard-broken (H2), and `TopologicalWaves()` returns + `complete = true` over a silently truncated edge set on a `broken` snapshot (H3). + Both are "no news" answers that mean "damaged", and the natural slice-3 predicate + (`len(Blockers(id)) == 0` → allow start) reads them as permission. +2. **Two write paths still change graph-owned fields.** `lint --fix` normalizes + `depends_on` at the text level and can *create* edges that did not exist + (H4, verified end-to-end), and the `task edit` guard is inverted on malformed + frontmatter: repairing a file while *preserving* its dependency is rejected while + repairing it by *deleting* the dependency succeeds silently (H5). +3. **The legacy edge plan slice 2 is meant to consume is not validated.** Legacy + references that resolve to a self-edge or to a mutual cycle are reported as + cleanly `resolved` at `degraded` health (M1). + +Cycle diagnostics are also incomplete (H1) — corroborating the sibling audit's H1 +with an executable counterexample, and adding that lint attributes a cycle to +exactly one member (M4). + +The overbuilt/underbuilt call: the `DAGAnalyzer` seam is **overbuilt** for what +shipped (one implementation, one three-case contract, and the comparison adapter is +not in the repository at all — L2), while cycle attribution, blocker honesty, and +the legacy plan's own validity are **underbuilt** relative to what ADR-0006 promises. +Nothing here is founded on an incorrect assumption; the `depends_on`-as-stable-ID-set +model and the strict/resilient split are sound and worth keeping. + +Findings are classified as **[merge-blocking]**, **[pre-mutation prerequisite]**, or +**[tracked follow-up]**. + +--- + +## Findings + +#### H1. `deterministicCycles` misses cycle members, so `blocker.Reason` lies about tasks that can never start · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:806-821 | **Component:** core / graph analysis +**Effort:** M · **Urgency:** acute + +**[pre-mutation prerequisite]** + +The DFS at `dependency_graph.go:806-821` handles only `state[dependent] == 0` +(unvisited) and `== 1` (on stack). There is no `case 2`: an edge into an already +finished node is dropped. Every *cycle* still yields at least one back edge, but not +every *node on a cycle* is reached by one, so `g.cycleMembers` is incomplete. + +Executed counterexample (three tasks, prerequisite→dependent edges +`A→B, A→C, B→A, C→B`, i.e. `A.depends_on=[B]`, `B.depends_on=[A,C]`, +`C.depends_on=[A]`, IDs chosen so `A < B < C`): + +``` +health=broken +problem code=cycle task=000000000001 cycle=[000000000001 000000000002 000000000001] +cycleMembers=map[000000000001:true 000000000002:true] +GAP: 000000000003 is on cycle A->C->B->A but is not a recorded cycle member +blocker: id=000000000003 reason=not-started path=[Z 000000000003] +``` + +`C` sits on the cycle `A→C→B→A` and is reported to consumers with reason +`not-started`. ADR-0006 (as amended by this slice) states that `cycle` is forensic +vocabulary that lets diagnostics "explain damage … without collapsing it into a +vague missing/blocked result" — this is precisely that collapse, in the opposite +direction: an unstartable task is described as merely unstarted. + +Fail-closed is preserved (`gate(C)` still returns `broken`, because `C`'s +prerequisite `A` *is* a flagged cycle member and `computeSound` propagates +`broken`), so this is a diagnostic-honesty defect, not a safety hole. That is why it +is a pre-mutation prerequisite rather than merge-blocking. + +**Why current tests miss it:** `TestTaskGraphCycleBlockerReason` +(`dependency_graph_test.go:249-264`) uses a two-node cycle, the one shape where a +single back edge covers every member. Worse, its assertion body is +`if (blocker.TaskID == a.ID || blocker.TaskID == b.ID) && blocker.Reason != BlockerCycle` +inside a `range` — it passes vacuously if `Blockers` returns nothing at all. +`dagcontract.Run`'s "attributable cycle" case is a single 3-cycle with no chords. + +**Recommendation:** replace the ad-hoc DFS with Tarjan SCC to mark membership +(`cycleMembers` = every node in a non-trivial SCC, plus self-loops), and keep the +existing back-edge walk only to render one representative path per SCC. That is a +contained change inside `deterministicCycles` and does not widen the analyzer +surface. + +#### H2. `Blockers()` reports nothing for a task whose own record is broken, and a lifecycle check will read that as permission · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:646-698 | **Component:** core / graph read API +**Effort:** S · **Urgency:** acute + +**[merge-blocking]** + +`Blockers` explains *prerequisites*. It never inspects the queried task's own +validity, so a task that is `hardBroken` for a reason local to itself — invalid +status, duplicate `depends_on` entry, ID drift, missing frontmatter ID — returns an +empty slice: + +``` +state(B)={TaskID:…02 Role:unknown Gate:broken …} Blockers(B)=[] (len=0) +state(C)={TaskID:…03 Role:candidate Gate:broken …} Blockers(C)=[] (len=0) +``` + +(`B` has status `nonsense`; `C` has `depends_on: [A, A]`. Both are `GateBroken`.) + +**Realistic failure:** epic 30's next slices are +`enforce-dependency-eligibility-across-every-task-start-path` and +`ship-guarded-dependency-mutations-and-graph-queries`. The obvious implementation of +"may this task start?" is `blockers := graph.Blockers(id); if len(blockers) == 0 { … }`, +and `task why-blocked ` is the obvious CLI. Both would authorize / report +"nothing blocking" for a task the snapshot has already classified as broken. The +correct predicate exists (`State(id).Gate == GateClear`, or +`state.Eligible`), but nothing in the type system, the doc comment +("returns every reachable unsound prerequisite"), or the tests steers a caller to it. + +**Why current tests miss it:** every `Blockers` test queries a *well-formed* task +whose prerequisites are the damaged ones. No test queries a hard-broken task. + +**Recommendation:** smallest fix — emit a self-blocker +(`Blocker{TaskID: taskID, Reason: , Path: []string{taskID}, Direct: true}`) +when `g.hardBroken[taskID]` or the task's own status is invalid, so an empty slice +means exactly one thing. Alternatively rename to `PrerequisiteBlockers` and document +that `State().Gate` is the authorization predicate. Do this before the API is +consumed, not after. + +#### H3. `TopologicalWaves()` returns `complete = true` on a `broken` snapshot, over an edge set that silently dropped the broken edges · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:331-366, 758-760 | **Component:** core / graph read API +**Effort:** S · **Urgency:** acute + +**[merge-blocking]** + +Edges are appended to the analyzer input only in the `default` arm of the switch at +`dependency_graph.go:331-357`; duplicate, invalid-ID, and missing prerequisites are +recorded as problems and then *omitted* from `edges`. `Analyze` therefore sees a +strictly smaller graph, finds no cycle, and returns `TopologicalComplete: true`. +`TopologicalWaves()` hands that straight out with no health qualifier: + +``` +cyclic snapshot: waves=[] complete=false health=broken +broken-but-acyclic: waves=[[…0a …0b]] complete=true health=broken +``` + +In the second case task `…0a` declares `depends_on: []`. The returned +wave plan places it in wave 0 alongside an unrelated task, and the boolean the API +offers as its trust signal says the ordering is complete. + +**Realistic failure:** `generate-deterministic-thread-graph-views` and +`add-usage-informed-thread-views-to-the-tui` are the natural consumers of waves. A +rendered Thread diagram on a partially migrated or hand-edited repository would show +a confidently wrong ordering, with the missing prerequisite invisible rather than +marked. + +**Why current tests miss it:** `TestTaskGraphTopologicalWavesAndDownstream` uses a +fully healthy graph. No test calls `TopologicalWaves` on a snapshot with a +non-cycle problem. + +**Recommendation:** return `complete = false` whenever `g.health != GraphHealthy` +(one line at `dependency_graph.go:758`), or drop the bool and require callers to +check `Health()` first. `complete` must never mean "no cycle among the edges I +happened to keep". + +#### H4. `lint --fix` rewrites `depends_on` and can create dependency edges through an unguarded path · **Status:** fixed 2026-08-27 + +**File:** internal/store/fix.go:353; internal/domain/fields.go:33 | **Component:** store / repair +**Effort:** S · **Urgency:** acute + +**[merge-blocking]** + +Registering `depends_on` as a `"list"` field (`fields.go:33`) enrols it in +`domain.IsListField`, which is exactly the predicate `fixValue` uses at +`fix.go:353` to rewrite a scalar into a YAML flow list. `FixFrontmatter` consults +`domain.IsGraphOwnedTaskField` nowhere. Verified end-to-end on a scratch planning +repo: + +``` +$ tskflwctl -C $R lint +! …/tasks/000000000001-alpha.md + validation failed: malformed frontmatter: field "depends_on" must be a YAML list, + but it is a string ("000000000002, 000000000003") + +$ tskflwctl -C $R task show 000000000001 --json # before: unreadable, ZERO edges +{"schema_version":"1.49","error":{"code":"validation",…}} + +$ tskflwctl -C $R lint --fix + - depends_on: normalized to a YAML list + +$ tskflwctl -C $R task show 000000000001 --json # after: TWO edges now exist +{…,"depends_on":["000000000002","000000000003"]} +``` + +`lint --fix` therefore performed a repository-global graph mutation — with no +referential check, no self-edge check, and no cycle check — via the one write path +the slice never guarded. The same mechanism applies to the legacy `dependencies` and +`blocks` fields (both already `"list"`), though not to `blocked_by`, which is +registered as `"list"` too but is the field this repo actually uses. This is also +the *only* CLI way to touch these fields today, which interacts badly with M2. + +**Why current tests miss it:** `fix_test.go` predates the field and tests +normalization on `tags`/`dependencies` as generic list behaviour, never as a graph +write. No test asserts that `FixFrontmatter` leaves graph-owned fields alone. + +**Recommendation:** exclude `domain.IsGraphOwnedTaskField(key)` from the +list-normalizing branch in `fixValue`, and have `FixFrontmatter` report those files +as `Skipped` with the reason, exactly as `repairInvalidID` already does for a +referenced id. If normalization is genuinely wanted, it belongs in the guarded +migration where the result can be validated. + +#### H5. The `task edit` dependency guard is inverted on malformed frontmatter: preserving an edge is rejected, deleting it is accepted · **Status:** fixed 2026-08-27 + +**File:** internal/store/edit.go:158-178, 228-252 | **Component:** store / interactive edit +**Effort:** M · **Urgency:** acute + +**[pre-mutation prerequisite]** + +`dependencyValues` (`edit.go:228`) recovers the pre-edit baseline with a narrow YAML +decode. Its doc comment claims this "can recover the graph baseline even when an +unrelated typed field (for example tier) is malformed" — true, and a good idea. But +the narrow struct still declares `DependsOn []string`, so it is defeated by +malformation *of the guarded field itself*, which is the case that matters. When +`dependenciesReadable` is false, `edit.go:167-176` accepts the edit only if the +result has **no** dependency fields at all. + +Verified against `store.FS.EditTask` directly (the CLI requires a TTY): + +``` +dependencyValues readable=false fields={dependsOn:[] …} +repair-keeping-deps: changed=false err=validation failed: cannot verify the original + graph-owned fields while repairing malformed frontmatter… +repair-deleting-deps: changed=true err= <-- edge silently deleted via task edit +resulting DependsOn=[] +``` + +So on a file with `depends_on: A, B` (scalar, the realistic hand-edit): repairing it +into `depends_on: [A, B]` is **refused**, and repairing it by **deleting the line +entirely is accepted with no diagnostic**. The guard permits exactly the destructive +outcome and forbids the corrective one. The error text ("repair them directly, run +lint") then points the operator at a raw filesystem edit — the behaviour the slice +set out to discourage — or at `lint --fix`, which is H4. + +**Why current tests miss it:** +`TestEditTaskRejectsDependencyDeltaButAllowsReordering` and its legacy twin only +exercise well-formed originals, where `dependenciesReadable` is always true. The +`!dependenciesReadable` branch has no test at all. + +**Recommendation:** make the narrow decode tolerant — decode into +`map[string]yaml.Node` (or `[]string`-or-`string`) so a scalar/duplicated value is +still captured verbatim as the baseline, and compare raw text when it cannot be +typed. Failing that, invert the escape hatch: accept an edit that leaves the +dependency text **byte-identical**, and reject one that removes it. + +#### M1. The resolved legacy edge plan is never validated — self-edges and mutual cycles resolve "clean" at `degraded` health · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:436-499 | **Component:** core / legacy migration diagnosis +**Effort:** M · **Urgency:** soon + +**[pre-mutation prerequisite]** + +`resolveLegacyDiagnostics` classifies each reference as `resolved` / `missing` / +`ambiguous` purely on *cardinality of the slug/ID match* (`dependency_graph.go:475-491`). +Nothing checks whether the resulting `ref.Edge` is legal, or whether the union of all +resolved edges with the existing canonical edges is still a DAG. Verified: + +``` +# legacy blocked_by naming the task's own slug +health=degraded problems=0 +legacy blocked_by ref="alpha" resolution=resolved edge={From:000000000001 To:000000000001} + +# two tasks each blocked_by the other's slug +health=degraded problems=0 +legacy blocked_by on …01 ref="beta" -> resolved edge=…02->…01 +legacy blocked_by on …02 ref="alpha" -> resolved edge=…01->…02 +``` + +Both snapshots report zero problems and `degraded` — the health state whose stated +meaning is "every legacy reference resolves exactly but has not yet been migrated". + +**Realistic failure:** `6g3q4rt7mgjn` is specified to consume this plan. A guarded +migration that trusts `Resolution == LegacyResolved` and writes +`ref.Edge` would produce a self-dependency or a cycle in one atomic batch, taking the +repository from `degraded` straight to `broken` — and, because migration is a bulk +write, potentially after an arbitrary write prefix has already landed. + +**Why current tests miss it:** +`TestTaskGraphLegacyResolutionHealthAndDirection` uses one prerequisite and one +dependent with no feedback edge; +`TestTaskGraphLegacyMissingAndAmbiguousAreBroken` covers only cardinality 0 and >1. + +**Recommendation:** after building `g.legacy`, run the *projected* edge set +(canonical edges ∪ resolved legacy edges) through the same `analyzer.Analyze`, and +downgrade to `broken` with a distinct problem code (e.g. +`legacy-reference-unsafe`) when the projection self-loops or cycles. This is the +cheapest possible pre-flight for slice 2 and reuses machinery already present. + +#### M2. Ordinary `lint` now fails permanently on this repository, and the guard removed the only CLI way to fix it · **Status:** fixed 2026-08-27 + +**File:** internal/core/service.go:327-380 | **Component:** core / lint · operations +**Effort:** S · **Urgency:** acute + +**[merge-blocking — operational]** + +Verified on the working tree: + +``` +$ ./bin/tskflwctl lint +… 6 item(s) with issues +error: validation failed: 6 item(s) with issues, 0 unreadable file(s) # exit 11 +``` + +All six are *resolvable* `blocked_by` occurrences — health is `degraded`, structural +problems are zero. This is deliberate and documented in the task's stress-test +criterion, but three consequences were not weighed: + +1. `CLAUDE.md` states "Keep `planning/` lint-clean" as a standing invariant. This + slice makes that impossible until `6g3q4rt7mgjn` ships, so the invariant and any + CI gate built on it are now permanently red. +2. Once lint is expected to fail, it stops functioning as a signal. A genuine new + defect lands inside a known-failing command whose exit code nobody reads. +3. The same slice made `task set`, `task set --force`, `task set --unset` and + `task edit` refuse these fields (correctly). The remaining CLI path is + `lint --fix`, which is H4 — an unguarded text rewrite. The operator's only + sanctioned option is a raw filesystem edit. + +This corroborates the sibling audit's M1; the CLAUDE.md conflict and the +`lint --fix` interaction are additional. + +**Recommendation:** make a *fully resolved* legacy diagnostic advisory — reported in +`lint` output and in `--json`, but not exit-code-bearing — until the migration lands, +and keep exit 11 for `legacy-reference-missing` / `legacy-reference-ambiguous`, which +are genuinely actionable today. That preserves the diagnostic without spending the +repository's lint-clean invariant on a state the tool refuses to let anyone fix. + +#### M3. `Blockers` is superlinear in chain depth and makes the lint pass roughly cubic; the "O(V+E)" acceptance criterion over-claims · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:669; internal/core/service.go:346-360 | **Component:** core / performance +**Effort:** M · **Urgency:** soon + +**[pre-mutation prerequisite]** + +`dependency_graph.go:669` copies the whole accumulated path for every edge examined +(`path := append(append([]string(nil), current.path...), prerequisite)`), so one +`Blockers` call over a chain of depth *d* does Θ(d²) work even when it returns no +blockers. `dependencyLintIssues` (`service.go:346`) then calls `Blockers` once per +`Inconsistent` task, which on a reopened chain is every task on it. + +Measured (single reopened thread, all downstream tasks `completed`): + +| chain depth | `dependencyLintIssues` | +|---|---| +| 100 | 10 ms | +| 200 | 40 ms | +| 300 | 84 ms | +| 500 | 345 ms | +| 1000 | 3.0 s | +| 2000 | **84 s** | + +`Blockers` alone: n=1000 → 6.4 ms, n=8000 → 139 ms. + +To be fair about when this bites: it is driven by **depth, not task count**. 2000 +tasks arranged as 200 threads of depth 10 cost **8 ms**, and this repository's +current `lint` over 279 tasks takes **0.09 s**. The shape that hurts is one long +Thread whose root is reopened — which is both a listed future failure scenario and +the literal purpose of epic 30. + +The acceptance criterion "Sound completion and derived graph state memoize one +result per task per snapshot and are O(V+E)" is true of `computeSound` +(independently confirmed) but not of the blocker projection, which is inherently +Ω(Σ path lengths) because ADR-0006 requires a path per blocker. + +**Why current tests miss it:** +`TestTaskGraphSoundCompletionMemoizesReconvergentDiamonds` asserts visit counts for +`computeSound` only, and the deep-chain test +(`TestOwnedDAGAnalyzerDeepWideAndDisconnected`) exercises `Analyze`, never +`Blockers`. There is no benchmark and no test that calls `dependencyLintIssues` at +scale. + +**Recommendation:** store a parent pointer per visited node and materialize the path +once per emitted blocker, not per edge. Then amend the acceptance criterion (or +ADR-0006) to say "state derivation is O(V+E); blocker path projection is O(V + Σ +path lengths)" rather than leaving an unachievable claim ticked. + +#### M4. A cycle is attributed to exactly one task; every other member is unattributed in `lint` · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:374; internal/core/service.go:340-346 | **Component:** core / lint attribution +**Effort:** S · **Urgency:** soon + +**[pre-mutation prerequisite]** + +`GraphProblem{Code: ProblemCycle, TaskID: cycle[0], …}` records one problem per +cycle, keyed to the lexicographically smallest member. +`dependencyLintIssues` buckets by `problem.TaskID`, so `lint` prints the cycle under +one slug only. Verified on a plain two-node cycle plus the H1 chorded shape: + +``` +lint issue for 000000000001: [depends_on] dependency cycle: …01 -> …02 -> …01 +GAP: task 000000000002 participates in a cycle but ordinary lint attributes nothing to it +GAP: task 000000000003 participates in a cycle but ordinary lint attributes nothing to it +``` + +ADR-0006 as amended by this slice requires that "every structural defect and legacy +occurrence must also appear during an ordinary `lint` call with deterministic +attribution". An operator inspecting the *other* task in the cycle — or a filter +like `lint --json` keyed by slug — sees a clean record. Combined with H1, a cycle +member can be both unattributed *and* mislabelled `not-started`. + +**Recommendation:** emit one `ProblemCycle` per member (same `Cycle` payload, each +with its own `TaskID` and `Path`), and let the message name the representative path. +Deduplication for human output belongs in the renderer, not in the attribution. + +#### M5. A duplicate stable task ID silently discards the second file's edges and mirrors the first file's issues onto both slugs · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:302-307; internal/core/service.go:272 | **Component:** core / identity +**Effort:** M · **Urgency:** soon + +**[pre-mutation prerequisite]** + +On a duplicate id the second record is dropped with `continue` +(`dependency_graph.go:306`) — its `depends_on` never becomes edges and its own +defects are never diagnosed. Meanwhile `service.go:272` appends +`graphIssues[canonicalTaskID(t)]` for **every** task, and both files share that key, +so the first file's issues are reprinted under the second file's slug. Verified: + +``` +problem duplicate-task-id task=…01 path=tasks/…01-second.md +problem missing-dependency task=…01 path=tasks/…01-first.md +dependencies[…01] = [000000000009] # second file's …07 edge is gone +lint issue attributed to BOTH slugs: [id] duplicate stable task id … +lint issue attributed to BOTH slugs: [depends_on] task …01 depends on missing task …09 +``` + +`second` is told it depends on a missing task it never referenced, and its real +reference to `…07` is reported nowhere. This is simultaneously the "silently +dropping" and "misleadingly duplicating" failure the review set out to test for. +Duplicate task ids are not hypothetical: `domain.DuplicateIDIssues` exists precisely +because they occur, and it is currently wired to research only, not tasks. + +**Recommendation:** two small changes — (a) key `graphIssues` by file path rather +than by task id in `service.go`, so a duplicate's issues land on the file that owns +them; (b) keep the dropped record in a `duplicates []domain.Task` side list and emit +its edge-level problems too, so the operator sees both files' claims before choosing +which to rename. + +#### M6. Unreadable task files carry no identity, so a blocker on one is reported as `missing` · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:269-273; internal/core/service.go:333 | **Component:** core / diagnostics +**Effort:** S · **Urgency:** soon + +**[tracked follow-up]** + +`ProblemUnreadable` is constructed with `Path` only; `TaskID` stays empty even though +task filenames are id-led (`tasks/-.md`) and `store.splitFlatName` already +recovers the id. `dependencyLintIssues` then skips the code entirely +(`service.go:333`), so the two halves are never joined: + +``` +problem unreadable-task task="" path=tasks/000000000002-broken.md +problem missing-dependency task="…01" msg=task …01 depends on missing task 000000000002 +blocker id=000000000002 reason=missing +lint issues for A: [{Field:depends_on Message:task …01 depends on missing task …02}] +``` + +The prerequisite is not missing — its file is right there and unparseable. A human +can join the two lines by eye because the id leads the filename; a machine consumer, +and any Thread view, cannot. Corroborates the sibling audit's M2; the concrete +remedy below is additional. + +**Recommendation:** populate `GraphProblem.TaskID` from the filename in the +`unreadable` loop, register those ids in a `g.unreadable` set, and add a +`BlockerUnreadable` reason so `blockerReason` distinguishes "file absent" from "file +present but unparseable". Both are broken; only one is fixed by re-creating the task. + +#### M7. `Blockers` recurses through a withdrawn prerequisite and attributes its upstream to the querying task · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:678-687 | **Component:** core / graph read API +**Effort:** S · **Urgency:** eventually + +**[tracked follow-up]** + +Traversal continues past any node that exists, including one whose reason is +`withdrawn`. Verified: + +``` +blocker …0u reason=not-started path=[T …0w …0u] direct=false +blocker …0w reason=withdrawn path=[T …0w] direct=true +``` + +`T` depends on the deprecated `W`, which depends on `U`. Once `W` is withdrawn, `U` +is irrelevant to `T`: finishing `U` changes nothing. A "what must I finish to unblock +this?" surface would list work that cannot help. The same applies to recursing past +an `invalid-status` node. + +**Recommendation:** stop traversal at a terminal blocker (`withdrawn`, +`invalid-status`, `cycle`, `missing`) and keep only the direct one, or add a +`Terminal bool` to `Blocker` so consumers can prune. Decide before slice 3 consumes +the list. + +#### L1. `invalid-dependency-id` has a problem code but no blocker reason, so it degrades to `missing` · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph.go:701-705 | **Component:** core / vocabulary +**Effort:** XS · **Urgency:** eventually + +**[tracked follow-up]** + +`blockerReason` returns `BlockerMissing` for anything absent from `g.tasks`, which +includes values that are not stable ids at all: + +``` +blocker id="not-a-stable-id" reason=missing path=[000000000001 not-a-stable-id] +``` + +The snapshot already knows better — it emitted `ProblemInvalidDependencyID` for the +same value. The blocker vocabulary the ADR now pins (eight tokens) has no way to say +so. **Recommendation:** add `BlockerInvalidReference` and check `id.Valid` in +`blockerReason` before falling back to `missing`; amend the ADR's token list in the +same change. + +#### L2. The "shared contract" exercises one implementation, and lives where `vet`/`golangci-lint` cannot see it · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph_contract_test.go:10; internal/core/testdata/dagcontract/contract.go | **Component:** testing / architecture +**Effort:** S · **Urgency:** eventually + +**[tracked follow-up]** + +The acceptance criterion "One shared contract suite covers the owned analyzer and +the bounded library adapter" is ticked, but: + +- `dagcontract.Run` has exactly one call site, with `core.OwnedDAGAnalyzer{}`. +- `dominikbraun/graph` appears nowhere in `go.mod`, and the implementation record + states the adapter "remained under `/tmp`". The comparison is therefore not + reproducible, not re-runnable when the analyzer changes, and not auditable. +- The contract itself is three cases: a 4-node diamond, an unchorded 3-cycle, and a + self-loop. It does not cover multiple cycles, chorded cycles (H1's shape), edges to + unknown nodes, duplicate edges, or empty input. +- `go list ./... | grep -c dagcontract` → `0`. A package under `testdata/` is + excluded from `./...`, so `go vet ./...` and `golangci-lint run ./...` never + analyze it. It is type-checked only as a transitive dependency of `core_test`. + +The verdict to retain the owned analyzer is defensible on the reasoning given +(deterministic attributable cycle paths, owned wave derivation, owned tie-breaking, +pre-v1 upstream API). Nothing important is being reimplemented poorly, and deferring +critical-path/weighted features is correct for V1. The problem is that the +`DAGAnalyzer` seam and the `testdata` package are abstraction cost paid for a +comparison the repository cannot demonstrate. + +**Recommendation:** either commit the adapter behind a build tag so the contract has +two implementations and the AC is true, or delete the `DAGAnalyzer` interface and the +`testdata` package and call `deterministicCycles` directly — then record the bake-off +as prose in the ADR. Meanwhile move the contract into `internal/core/dagcontract` +(no `testdata`) so it is linted, and grow it with the shapes above. + +#### L3. `schema --json` advertises all four graph-owned fields with no machine-readable "not settable" marker · **Status:** tracked by 6g3q4rt7mgjn + +**File:** internal/domain/fields.go:33, 89-97 | **Component:** wire / agent contract +**Effort:** S · **Urgency:** eventually + +**[tracked follow-up]** + +`schema --json` `task_fields` now lists `depends_on`, `blocked_by`, `dependencies`, +and `blocks` alongside genuinely settable fields, while all four are rejected: + +``` +$ tskflwctl task set 6g3q4rst78qy --set depends_on=6fjangd7kvh0 +error: validation failed: depends_on is graph-owned and cannot be changed with `task set` … +``` + +The 1.49 note states the contract "recognizes the persisted list while generic +mutation remains forbidden" — the recognizing half is machine-readable, the +forbidding half is not. In fairness there is precedent: `status` has always been in +`task_fields` and has never been settable, so the registry has never meant +"settable". But this slice quadruples the exception set, and `schema` is explicitly +the surface agents route on. + +**Recommendation:** add a `writable` (or `owner: graph|lifecycle`) boolean to the +`task_fields` entries and bump to 1.50 when convenient. Low urgency; the error +message is clear and names the future command. + +#### L4. `ReadTaskGraph()` has no production caller, and `Service.Lint` re-implements the scan it wraps · **Status:** tracked by 6g3q4rt0wzkq + +**File:** internal/core/service_task.go:84-90; internal/core/service.go:246-250 | **Component:** core / architecture +**Effort:** XS · **Urgency:** eventually + +**[tracked follow-up]** + +`ReadTaskGraph` is the documented snapshot factory, but the only production consumer +of the graph — `Service.Lint` — calls `s.store.ListTasks()` and `NewTaskGraph` +itself. `ReadTaskGraph` has zero callers and zero direct tests +(`grep -rn ReadTaskGraph internal` returns only its own definition), and +`TaskGraph.Task()` shows 0% coverage. + +The core/store split is otherwise well prepared for slice 2: the graph is pure +in-memory analysis over `domain.Task` with no filesystem or lock dependency, so a +store-owned `WithGraphMutation` critical section can take the write lock, build a +snapshot, plan, and write without core learning about locking. That part needs no +undoing. **Recommendation:** have `Lint` call `ReadTaskGraph` (it needs +`[]TaskWithBody` for other checks, so extract the projection) so the seam is +exercised by the one command that uses it. + +#### L5. `store.FS.CreateTask` serializes `depends_on` with no validation · **Status:** fixed 2026-08-27 + +**File:** internal/store/create.go:83-87, 112 | **Component:** store / create +**Effort:** XS · **Urgency:** eventually + +**[tracked follow-up]** + +`taskFields` writes `t.DependsOn` sorted whenever it is non-empty, with no +`id.Valid` check, no existence check, and no self-edge check. No CLI reaches it +today (`task new` has no dependency flag and `NewTaskParams` has no such field), so +this is not currently exploitable — but the create path is exactly where +`task new --depends-on` will land, and the guard that exists on `SetFields` and +`EditTask` has no counterpart here. **Recommendation:** reject a non-empty +`DependsOn` in `CreateTask` until the guarded create arrives, mirroring +`FS.SetFields` (`fsstore.go:262-266`). + +#### L6. Test-suite gaps that create false confidence · **Status:** fixed 2026-08-27 + +**File:** internal/core/dependency_graph_test.go | **Component:** testing +**Effort:** M · **Urgency:** soon + +**[tracked follow-up]** + +Concrete weaknesses, beyond the per-finding notes above: + +- **Vacuous pass.** `TestTaskGraphCycleBlockerReason:259-263` asserts inside a + `range` over `Blockers`; an empty result passes. Given H2, that is not theoretical. +- **Containment instead of equality.** Both problem-code tests use + `slices.Contains` over a `wantCodes` list (`:45-49`, `:81-85`). Neither asserts the + exact set, the count, or the attribution, so a dropped, duplicated, or + misattributed problem is invisible. +- **Unrepresentative ids.** `testutil.TaskID` hashes a seed into the Crockford + alphabet, producing ids with no shared prefix. Real ids are time-ordered + (`id.New`), so tasks minted in one session share a long prefix and their + lexicographic order *is* creation order. Every tie-break and sort assertion is + exercised against a distribution the product never produces. +- **White-box coupling.** `TestTaskGraphSoundCompletionMemoizesReconvergentDiamonds` + reaches into the unexported `graph.soundVisits`, which is incremented only on cache + miss — so `visits == 1` is close to tautological given the early return. It does + demonstrate memoization; it does not demonstrate a complexity bound. +- **Untested seam.** No test injects a non-owned `DAGAnalyzer` into `newTaskGraph`, + so the abstraction's only justification is untested (see L2). +- **Missing shapes.** No fuzz or property test over `NewTaskGraph`; no concurrency + test on `TaskGraph` (the suite is `-race`-clean but nothing exercises parallel + readers — I added one ad hoc and it passed); no benchmark; no test of + `dependencyLintIssues` on a real repository fixture; no `-o csv` / TUI coverage of + the new field; `TaskGraph.Task()` uncovered. + +**Recommendation:** convert the two containment assertions to exact +`[]GraphProblem` comparisons, restructure the cycle-reason test to assert a +count first, add a `testutil.SequentialTaskID` helper for order-sensitive tests, and +add one property test (random DAG → `TopologicalWaves` respects every edge; random +graph with a planted cycle → every planted member is a `cycleMembers` entry). The last +one is the regression test for H1. + +--- + +## Claims I tried to falsify and found sound + +Each of these was attacked with a constructed counterexample or a direct +measurement, not read off the implementation record. + +1. **`depends_on` really is a duplicate-free stable-ID set at the boundaries.** + The reader deliberately retains raw duplicates (`store` round-trip preserves + `[second, first]` unsorted), `CreateTask` writes sorted, `ToTaskJSON` sorts a + *copy* and provably does not mutate the domain record, and the graph dedupes via + `sortedUnique` while still detecting duplicates — the `seen` map at + `dependency_graph.go:325` runs over the raw list, so assigning the deduped set + first does not hide them. Verified. +2. **Generic mutation is genuinely closed.** All four fields, both spellings + (`--set` and `--unset`), with and without `--force`, at both the core + (`Service.SetFields`) and store (`FS.SetFields`) layers. Checked through the real + CLI, not just the unit tests. The TUI inline editor uses an explicit field list + that excludes them. +3. **Concurrent reads are safe.** 16 goroutines × 200 tasks × 8 accessors under + `-race`: clean. The reason is structural, not accidental — `computeSound` is + driven to completion during construction and is never reached again from an + exported method, so `g.sound` is frozen; `Blockers`/`Downstream` guard their + caches with `g.mu`; and returned slices are copies (I mutated a returned + `Task().Tags` and the snapshot was unaffected). +4. **Blocker paths are shortest, deterministic, *and* lexicographically minimal.** + Not just "stable for the happy shape": on a graph with two equal-length competing + routes through different intermediates, 50 randomized input permutations produced + byte-identical `[]Blocker`. This follows from FIFO BFS over sorted adjacency, which + keeps each frontier in lexicographic path order. +5. **Legacy edge direction is right, including the `blocks` inversion.** + `blocked_by`/`dependencies` produce `From: candidate, To: task`; `blocks` produces + `From: task, To: candidate`. Exact-ID match takes precedence over slug match, and + resolution is exact-only (no prefix/fuzzy), so it is deterministic. Duplicate + slugs — legal in this repo — correctly go `ambiguous`. +6. **The one existing path that could change graph identity already fails closed.** + `repairInvalidID` refuses to canonicalize a misspelled id that is referenced + anywhere else in the planning tree, and its `referencesTo` substring scan covers + `depends_on` values for free. `lint --fix` cannot orphan an inbound edge by + renaming. (Contrast H4, which is the *content* path, not the identity path.) +7. **Reopen invalidation works without touching disk.** Flipping an upstream task + back to `ready-to-start` immediately makes the completed downstream + `SoundlyCompleted: false`, `Drained: false`, `Gate: blocked`, `Inconsistent: true`, + with its frontmatter unchanged. Broken-over-blocked precedence holds; `deferred` + is `parked`/blocked and `deprecated` is `withdrawn`/broken, as the ADR says. +8. **Deep graphs do not blow the stack.** A 100 000-node chain builds and answers + correctly (`sound=true broken=false health=healthy`). The recursion in + `computeSound` and `deterministicCycles` is bounded by depth and Go's growable + stacks absorb it; the sibling audit's L3 is a real but very low-priority concern. + Depth costs time (M3), not correctness. +9. **The 1.49 wire change is additive and correctly versioned.** `depends_on` is + `omitempty`, sorted, present in the JSON Schema and the schema-comment map, and no + existing field or envelope changed. A 1.48 validator will reject 1.49 payloads + because `TaskJSON` is `additionalProperties: false` — that is inherent to any + additive change under a closed schema and is exactly what the version bump is for. +10. **The omitted surfaces are consistent, not accidental.** `depends_on` is absent + from `render.TaskColumns()`, so `-c depends_on` is unavailable in `-o table/csv` + and in `--json -c` projections — but `tags` has always been absent for the same + reason (list-valued fields are not string columns). Full `--json` carries it. This + is a deliberate deferral matching existing precedent, not a gap. +11. **Multi-repository planning spaces are not broken by this design.** Edges are + plain stable ids resolved within one planning root, and ids are globally unique by + construction, so a planning space coordinating several implementation checkouts + works unchanged. The real limitation is that an edge cannot *name* another + planning space — but nothing in this slice forecloses adding a qualifier later, + and adding one now would be speculative complexity. Correctly not built. +12. **The core/store split is the right preparation for slice 2.** The graph is pure + analysis over `domain.Task` with no I/O and no locking, so the store-owned + mutation critical section can wrap it without inverting the dependency. Nothing + here will need undoing (see L4 for the one loose thread). + +Two claims from the sibling audit's own "survived adversarial review" list do **not** +survive: its item 2 extends the O(V+E) finding to `Blockers` (falsified by M3), and +its item 3 states `task edit` "correctly permits formatting / order-only adjustments" +without qualifying the malformed-frontmatter branch (falsified by H5). Its item 6 +covers legacy direction and cardinality correctly but does not test edge *legality* +(M1). + +--- + +## Traceability table + +| Finding | Severity | Classification | Destination | +|---|---|---|---| +| H1 cycle members under-reported | High | Fixed | `6g3q4rst78qy`: Tarjan SCC membership + representative-cycle tests | +| H2 `Blockers` silent on self-brokenness | High | Fixed | `ExplainGate` couples derived authorization state, local problems, and frontier | +| H3 `TopologicalWaves` complete on broken snapshot | High | Fixed | completeness is true only at healthy graph health | +| H4 `lint --fix` writes graph-owned fields | High | Fixed | graph-owned normalization is skipped noisily without writing | +| H5 `task edit` guard inverted on malformed YAML | High | Fixed | an unreadable graph baseline rejects every edited candidate | +| M1 legacy edge plan unvalidated | Medium | Fixed | canonical ∪ resolved-legacy SCC validation + ADR amendment | +| M2 permanent `lint` failure on this repo | Medium | Fixed | safe legacy debt is an advisory with exit zero; unsafe debt remains fatal | +| M3 blocker/lint cost cubic in chain depth | Medium | Fixed | parent-pointer traversal, action-frontier lint, honest output-sensitive contract | +| M4 cycle attributed to one member only | Medium | Fixed | one deterministic problem per SCC member | +| M5 duplicate id drops + mirrors issues | Medium | Fixed | every record is validated and lint attribution is keyed by source path | +| M6 unreadable files lose identity | Medium | Fixed | stable ID recovery from id-led filenames + `unreadable` blocker reason | +| M7 traversal past withdrawn prerequisites | Medium | Fixed | causal closure traverses; action frontier stops at withdrawn/terminal damage | +| L1 no `invalid-reference` blocker token | Low | Fixed | distinct `invalid-reference`, `unreadable`, and `invalid-task` reasons | +| L2 contract covers one implementation | Low | Fixed | speculative analyzer interface/testdata package removed; direct adversarial tests retained | +| L3 `schema` cannot express "not settable" | Low | Tracked | acceptance criterion on `6g3q4rt7mgjn` | +| L4 `ReadTaskGraph` unused, scan duplicated | Low | Tracked | acceptance criterion on `6g3q4rt0wzkq` | +| L5 `CreateTask` writes unvalidated edges | Low | Fixed | ordinary create rejects non-empty graph-owned fields | +| L6 test-suite gaps | Low | Fixed | non-vacuous exact SCC, projection, identity, repair, and legacy counterexamples | + +ADR-0006 amendments required: **M1** (define `degraded` to require a *legal* projected +edge set), **M2** (state which legacy states are exit-code-bearing), **M3** (state the +blocker-path complexity honestly), **L1** (extend the blocker token list). + +Current-task reopening recommended for: **H1–H5, M2, M4** (all in-scope defects of +`6g3q4rst78qy`, not new scope). + +--- + +## Validation commands and results + +Run from `/Users/andyeschbacher/git/andy-esch/taskflow-canonical-task-dependency-reads` +on branch `feat/canonical-task-dependency-reads` (uncommitted tree, merge-base +`43b3044`). All counterexamples were compiled with `go test -overlay=…` against files +held outside the repository, so no production or test file was modified — verified by +`git status --porcelain` before and after (48 entries both times; the only new entry +is this audit). + +| Command | Result | +|---|---| +| `go build ./...` | pass | +| `go test ./...` | pass, 23 packages | +| `go test -race ./...` | **pass, exit 0**, no race reports | +| `just lint` (`golangci-lint run ./...`) | **0 issues** | +| `go vet ./...` | pass | +| `go list ./... \| grep -c dagcontract` | **0** — contract package excluded from `./...` (L2) | +| `./bin/tskflwctl lint` | **exit 11**, 6 legacy `blocked_by` issues, 0 unreadable (M2) | +| `time ./bin/tskflwctl lint` | 0.09 s over 279 tasks | +| `./bin/tskflwctl task set … --set/--unset depends_on\|blocked_by\|dependencies\|blocks [--force]` | all rejected, exit 11 (sound) | +| `go test ./internal/core -run 'TestTaskGraph\|TestOwnedDAG' -cover` | 26.1% package; `TaskGraph.Task` 0%, `TaskIDs` 0% | +| Overlay: cycle-member coverage counterexample | **GAP confirmed** — H1, M4 | +| Overlay: `Blockers` on hard-broken task | **`[]` returned** — H2 | +| Overlay: `TopologicalWaves` on broken-but-acyclic snapshot | **`complete=true`** — H3 | +| Overlay: legacy self-edge / mutual-cycle resolution | **`resolved`, `degraded`, 0 problems** — M1 | +| Overlay: duplicate task id | **second file's edges dropped; issues mirrored** — M5 | +| Overlay: unreadable file vs dependency id | **`TaskID=""`, blocker `missing`** — M6 | +| Overlay: `EditTask` on malformed frontmatter | **preserve rejected, delete accepted** — H5 | +| CLI: `lint --fix` on scalar `depends_on` in a scratch repo | **0 edges → 2 edges**, unguarded — H4 | +| Overlay: 50-permutation blocker determinism on competing equal-length paths | byte-identical (sound) | +| Overlay: 16-goroutine concurrent reader under `-race` | clean (sound) | +| Overlay: 100 000-node chain | builds, correct, no stack overflow (sound) | +| Overlay: `dependencyLintIssues` depth scaling | 100→10 ms · 500→345 ms · 1000→3.0 s · **2000→84 s** — M3 | +| Overlay: 2000 tasks / 200 threads of depth 10 | 8 ms (M3 is depth-driven, not size-driven) | + +--- + +## Remediation disposition (2026-08-27) + +The current slice absorbed every merge-blocking and pre-mutation correctness finding. L3 is +tracked by an explicit machine-schema ownership criterion on `6g3q4rt7mgjn`; L4 is tracked by an +explicit canonical-snapshot-loader criterion on `6g3q4rt0wzkq`. No speculative standalone task or +graph-library abstraction was created. Final post-remediation validation is recorded in task +`6g3q4rst78qy`. diff --git a/planning/tasks/6g3q4rst78qy-establish-canonical-task-dependencies-and-strict-graph-reads.md b/planning/tasks/6g3q4rst78qy-establish-canonical-task-dependencies-and-strict-graph-reads.md index 8ea2cf42..e193280d 100644 --- a/planning/tasks/6g3q4rst78qy-establish-canonical-task-dependencies-and-strict-graph-reads.md +++ b/planning/tasks/6g3q4rst78qy-establish-canonical-task-dependencies-and-strict-graph-reads.md @@ -1,7 +1,7 @@ --- schema: 1 id: 6g3q4rst78qy -status: in-progress +status: completed epic: 30-threads-and-task-dependency-graphs description: Model canonical depends_on data and deterministic strict graph snapshots, derived gate state, legacy diagnostics, and bounded library comparison. effort: 3-5 days @@ -10,8 +10,9 @@ priority: high autonomy_level: 3 tags: [threads, graph, storage, migration] created: "2026-08-25" -updated_at: "2026-08-25" -started_at: "2026-08-25" +updated_at: "2026-08-27" +started_at: "2026-08-26" +completed_at: "2026-08-27" --- # Establish canonical task dependencies and strict graph reads @@ -22,32 +23,112 @@ Introduce the production read foundation for one planning-repository task DAG wi ## Scope - Model `depends_on` as a sorted, duplicate-free set of stable task IDs on `domain.Task` and in the task field/schema contracts. -- Define a narrow taskflow-owned graph analysis interface and strict repository snapshot distinct from resilient repair-oriented listing. +- Define a small taskflow-owned graph analyzer and strict repository snapshot distinct from resilient repair-oriented listing. - Diagnose—but do not rewrite—the live legacy `blocked_by`, `dependencies`, and `blocks` vocabulary, including slug-to-ID resolution failures and ambiguity. - Implement deterministic validation, cycle paths, memoized sound completion/gate derivation, lifecycle roles, blocker/downstream traversal, and topological waves. - Define blocker records with stable reason tokens and one deterministic shortest explanatory path. - Prevent generic `task set`, including `--force`, and unguarded `task edit` from introducing or changing `depends_on`. -- Run the bounded owned-versus-`dominikbraun/graph` contract-test bake-off and record the decision; do not expand V1 algorithms. +- Run the bounded owned-versus-`dominikbraun/graph` bake-off and record the decision; retain only the implementation surface justified by that decision. ## Acceptance criteria -- [ ] Valid task frontmatter round-trips `depends_on` in stable ID order through domain, store, schema, and wire-facing task representations where applicable. -- [ ] A strict snapshot identifies malformed/unreadable tasks, ID drift, unknown status, duplicate/self/missing dependencies, and attributable cycles for fail-closed consumers while diagnostic consumers retain the problem list. -- [ ] Existing resilient list/lint repair behavior remains available and reports every graph problem deterministically. -- [ ] Legacy dependency diagnostics report each resolvable target ID and actionable missing/ambiguous slug failures; no migration write occurs in this task. -- [ ] Gate derivation implements broken-over-blocked precedence, treats deferred prerequisites as blocked, and explains unfinished, parked, withdrawn, missing, and unsound-completed blockers. -- [ ] Sound completion and derived graph state memoize one result per task per snapshot and are O(V+E). -- [ ] Generic set always rejects `depends_on`, including under `--force`; interactive edit cannot land a dependency delta before guarded validation exists. -- [ ] One shared contract suite covers the owned analyzer and the bounded library adapter; the retained choice and rationale are recorded. -- [ ] No public dependency or Thread mutation is introduced by this task. +- [x] Valid task frontmatter round-trips `depends_on` in stable ID order through domain, store, schema, and wire-facing task representations where applicable. +- [x] A strict snapshot identifies malformed/unreadable tasks, ID drift, unknown status, duplicate/self/missing dependencies, and attributable cycles for fail-closed consumers while diagnostic consumers retain the problem list. +- [x] Existing resilient list/lint repair behavior remains available and reports every graph problem deterministically. +- [x] Legacy dependency diagnostics report each resolvable target ID and actionable missing/ambiguous slug failures; no migration write occurs in this task. +- [x] Gate derivation implements broken-over-blocked precedence, treats deferred prerequisites as blocked, and explains unfinished, parked, withdrawn, missing, and unsound-completed blockers. +- [x] Sound completion and derived graph state memoize one result per task per snapshot and are O(V+E). +- [x] Generic set always rejects `depends_on`, including under `--force`; interactive edit cannot land a dependency delta before guarded validation exists. +- [x] The bounded library comparison exercises the same taskflow-owned cases as the owned analyzer; the retained choice and rationale are recorded without preserving a speculative adapter abstraction. +- [x] No public dependency or Thread mutation is introduced by this task. +- [x] Every first-party write path, including lint --fix, task creation, and + malformed-frontmatter edit, refuses an unvalidated graph-owned field change. +- [x] Cycle analysis uses SCC membership, attributes every cyclic task + deterministically, and emits one representative path per component without + duplicate self-edge noise. +- [x] Topological completeness is false on degraded or broken snapshots, and + resolved legacy edges produce degraded health only when their projected union + is a legal DAG. +- [x] Unreadable files, invalid references, and duplicate task IDs retain + path-faithful identity and actionable lint/blocker diagnostics without + silently dropping a duplicate record's defects. +- [x] Safely resolvable legacy references remain visible as advisory + ordinary-lint findings with exit zero; unresolved, ambiguous, or unsafe legacy + references remain validation errors. +- [x] Separate causal and action-frontier blocker projections are explicit; + eligibility uses derived state, and ordinary lint avoids expanding every + causal path on deep inconsistent chains. +- [x] The retained analyzer implementation and its tests are reproducible + in-repository, with non-vacuous exact assertions and adversarial graph shapes. +- [x] Every finding in both 2026-08-26 dependency-foundation audits is marked + fixed, tracked with a concrete destination, or rejected with recorded + evidence. ## Stress tests - Randomized input/map order produces byte-for-byte stable diagnostics and plans. - Deep chains, wide frontiers, disconnected tasks, duplicate edges, missing IDs, self-edges, and exact cycle paths are covered. - Reconvergent diamond chains assert bounded visit counts, not merely acceptable wall-clock time. -- Ordinary repository lint and the full test, race, formatting, schema, and diff checks pass. +- Ordinary repository lint reports exactly the six expected degraded legacy-field diagnostics and + no unaccounted graph defects; the full test, race, formatting, schema, and diff checks pass. ## Sequencing First production slice. It unlocks guarded dependency writes and supplies the pure role/gate/sound-completion analysis consumed independently by eligibility enforcement and Threads. + +## Implementation record (2026-08-26) + +- Added canonical `depends_on` persistence and schema/wire projection without adding a public graph + mutation. Readers retain malformed evidence for lint; valid serialization and outward projection + use stable ID order. +- Added one immutable strict snapshot with `healthy`, `degraded`, and `broken` health. Deterministic + problems cover unreadable tasks, missing/drifted/duplicate IDs, invalid status, duplicate/self/ + invalid/missing edges, cycles, and unresolved legacy references. Derived role, gate, sound + completion, blockers, downstream impact, and topological waves stay behind taskflow-owned types. +- Generic set and interactive edit now reject changes to all graph-owned fields (`depends_on`, + `blocked_by`, `dependencies`, and `blocks`), including force/unset paths. This prevents supported + commands from manufacturing invalid graph states; ordinary lint remains the noisy guard for raw + edits and older binaries. +- Real-repository dogfood loaded 279 tasks in 12–29 ms across two observed runs. Health was + `degraded`, with zero structural problems and exactly six resolvable legacy `blocked_by` field + occurrences. Ordinary human and JSON lint now emit those six grouped, stable-ID/edge advisories + and no unreadable files while exiting zero. Mutation and dispatch remain closed until guarded + migration in `6g3q4rt7mgjn`. +- The isolated `dominikbraun/graph` v0.23.0 adapter passed the same bounded taskflow-owned cases as + the owned analyzer. It still required owned deterministic cycle attribution, custom + wave derivation, and taskflow-controlled shortest-path tie breaking. That is no material code + reduction, while the upstream API remains explicitly unstable before v1; retain the owned + O(V+E) analyzer for V1 and revisit libraries only if the analysis surface grows. The temporary + bake-off module added no project dependency; the repository keeps direct adversarial tests rather + than a one-implementation adapter interface. +- Stress coverage includes randomized ordering, deep chains, wide/disconnected frontiers, + duplicate/self/missing/invalid edges, exact and self cycles, all blocker reason tokens, + deterministic shortest paths, legacy direction/resolution, reopen invalidation, immutable query + results, and a 120-layer reconvergent diamond with exact visit-count assertions. + +## Adversarial hardening record (2026-08-27) + +- Replaced back-edge cycle discovery with SCC membership: every cyclic task receives deterministic + attribution, one edge-following representative path is retained per component, and self-edges do + not also emit generic cycle noise. Canonical and resolved legacy edges are analyzed as one + structural union, so a legacy self-edge or cycle is broken rather than degraded. +- Replaced the ambiguous blocker API with `CausalBlockers`, `BlockingFrontier`, and `ExplainGate`. + Authorization remains `State.Eligible`; lint uses the bounded action frontier. Traversal keeps + predecessor links and materializes only emitted paths, making the complexity contract explicitly + output-sensitive. +- Closed the remaining first-party write gaps: task creation rejects dependency-bearing records, + lint repair skips graph-owned normalization noisily, and an unreadable edit baseline rejects every + candidate rather than accepting deletion. Safe legacy debt is an advisory in human/JSON lint with + exit zero; unsafe/unresolved debt remains a validation error. +- Preserved stable-ID identity for unreadable filenames, distinguished missing/unreadable/invalid + references, and keyed graph lint attribution by source path so both duplicate-ID records retain + their own defects. The speculative one-implementation analyzer interface and hidden testdata + contract were removed in favor of direct adversarial tests. +- Both independent audits now have no open findings. Machine-readable field ownership is tracked by + `6g3q4rt7mgjn`; canonical snapshot-loader consolidation is tracked by `6g3q4rt0wzkq`; the measured + deep-chain optimization trigger is also explicit on `6g3q4rt7mgjn`. + +Validation: full `go test ./...` and `go test -race ./...`; golangci-lint (zero issues); `go vet +./...`; module tidy diff; generated CLI docs; schema-comment freshness; golden machine contracts; +`git diff --check`; both audit lints; and live planning lint all pass. Live planning lint emits +exactly six advisory legacy-field findings, zero unreadable files, and exits zero. diff --git a/planning/tasks/6g3q4rt0wzkq-make-repository-graph-mutations-portable-and-serializable.md b/planning/tasks/6g3q4rt0wzkq-make-repository-graph-mutations-portable-and-serializable.md index b09c7e87..3ed6a570 100644 --- a/planning/tasks/6g3q4rt0wzkq-make-repository-graph-mutations-portable-and-serializable.md +++ b/planning/tasks/6g3q4rt0wzkq-make-repository-graph-mutations-portable-and-serializable.md @@ -10,6 +10,7 @@ priority: high autonomy_level: 2 tags: [threads, graph, storage, concurrency] created: "2026-08-25" +updated_at: "2026-08-26" --- # Make repository graph mutations portable and serializable @@ -31,6 +32,9 @@ Provide one store-owned repository mutation boundary that makes final graph read - [ ] The callback contract accepts and returns taskflow-owned snapshot/planned-write values, permits no nested Store calls, and detects invalid nesting without hanging. - [ ] Lock acquisition/release errors are attributable and process termination does not leave unrecoverable stale state. - [ ] Existing optimistic concurrency and ordinary write behavior remain compatible. +- [ ] The store boundary consumes one canonical strict-snapshot loader; remove + or fold any duplicate or otherwise unused ReadTaskGraph scan seam so lint and + mutation cannot drift. ## Stress tests diff --git a/planning/tasks/6g3q4rt7mgjn-ship-guarded-dependency-mutations-and-graph-queries.md b/planning/tasks/6g3q4rt7mgjn-ship-guarded-dependency-mutations-and-graph-queries.md index b415a84f..bc8aa7d9 100644 --- a/planning/tasks/6g3q4rt7mgjn-ship-guarded-dependency-mutations-and-graph-queries.md +++ b/planning/tasks/6g3q4rt7mgjn-ship-guarded-dependency-mutations-and-graph-queries.md @@ -10,6 +10,7 @@ priority: high autonomy_level: 3 tags: [threads, graph, cli, storage] created: "2026-08-25" +updated_at: "2026-08-26" --- # Ship guarded dependency mutations and graph queries @@ -34,6 +35,14 @@ Expose safe repository-global dependency operations and deterministic read queri - [ ] Diagnostic queries degrade with explicit problems and graph health; frontier/unblocked selectors return no eligible work on an unsound relevant graph; mutation fails closed. - [ ] The six live legacy `blocked_by` values migrate from resolvable slugs to stable `depends_on` IDs with atomic-frontmatter/body preservation; missing or ambiguous values write nothing. - [ ] `task set` cannot mutate `depends_on` even with `--force`, and `task edit` cannot bypass guarded graph validation. +- [ ] Public blocker commands expose separately named causal-closure and + action-frontier projections, while every authorization path uses derived + eligibility rather than blocker-list emptiness. +- [ ] Machine-readable schema marks graph-owned dependency fields as unavailable + to generic set/unset and directs callers to guarded dependency operations. +- [ ] Deep-chain stress establishes a supported graph-depth envelope; replace + recursive sound derivation if measured repository shapes approach unsafe stack + or latency bounds. ## Stress tests From 145281066a02a61f313fd3e1ea9c89c85075c7d0 Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Thu, 27 Aug 2026 05:34:37 -0400 Subject: [PATCH 2/2] docs: clarify dependency read foundation boundaries Document the preservation-only depends_on rollout, graph-aware lint behavior, generic mutation guardrails, and the core/store ownership model. Regenerate the affected CLI reference and schema golden. --- README.md | 11 +++++++++++ docs/ARCHITECTURE.md | 13 ++++++++++++- docs/cli/tskflwctl.md | 2 +- docs/cli/tskflwctl_lint.md | 14 ++++++++++++-- docs/cli/tskflwctl_task_edit.md | 6 +++++- docs/cli/tskflwctl_task_set.md | 7 +++++++ internal/cli/edit.go | 5 ++++- internal/cli/lint.go | 12 +++++++++--- internal/cli/task.go | 8 ++++++-- .../cli/testdata/golden/schema_task_json.golden | 2 +- internal/domain/entity.go | 2 +- 11 files changed, 69 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 146f4a32..980f0b08 100644 --- a/README.md +++ b/README.md @@ -240,6 +240,17 @@ field in place and stamp the dates atomically — no file moves (`lint --fix` re-normalizes a hand-edited drift). Errors carry semantic exit codes — `10` not-found, `11` validation, `13` ambiguous, `14` conflict (e.g. a name already taken). +**Task-dependency read foundation.** Task frontmatter and JSON may carry `depends_on`, +a sorted set of stable task IDs representing repository-global prerequisites. This +release reads, validates, and explains that graph but intentionally exposes no public +dependency mutation command yet. Generic task creation, `task set` (even `--force`), +`task edit`, and `lint --fix` cannot add, remove, or reinterpret dependency fields; +guarded `task depend add/remove` operations are the next slice. Ordinary `lint` reports +all graph defects. Exactly resolved legacy `blocked_by`/`dependencies`/`blocks` values +are visible JSON/human advisories with exit zero, while missing, ambiguous, cyclic, or +self-referential legacy projections remain validation errors. See +[`ADR-0006`](./planning/adrs/0006-adopt-threads-as-task-dags.md) for the model and rollout. + **Research** is the thinnest kind, and the omissions are the point: no status and no lifecycle verbs (a later doc supersedes an earlier one — a decision that needs a lifecycle is an [ADR](./planning/adrs/)), and no `epic:` field, so provenance is diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d4a1f6da..154e29c3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -143,6 +143,13 @@ adapter capabilities rather than leaked persistence. adapter needs a complete entity service and watcher layout for an explicit start path. Its `WorkspaceStore` port returns neutral capabilities; registry labels are carried as presentation context and never influence discovery. + `TaskGraph` is an immutable read projection over one repository scan. It owns graph + health (`healthy`/`degraded`/`broken`), SCC-based cycle attribution, derived lifecycle + role and gate state, sound completion, topology, downstream impact, and separately + named causal-blocker and action-frontier projections. The analyzer uses only taskflow + types and owned deterministic algorithms; a graph package cannot leak into domain, + persistence, or wire contracts. Eligibility is read from derived state, never inferred + from an empty blocker list. Per-space failures remain data in the projection; the CLI renders the complete sweep before applying its partial-failure exit policy. Pure; unit-testable without fs. - **`internal/store`** — the secondary adapter: tasks as @@ -152,7 +159,11 @@ adapter capabilities rather than leaked persistence. the use-case `Store`; CLI lint and the TUI watcher get the narrow `Fixer`/`Linter`/`Layout` wired directly. It owns the *layout* knowledge — `WatchPaths()` hands the TUI watcher its dir set so the path convention isn't reconstructed - outside the store. Concurrency is **version-CAS** (epic 24): every write, just + outside the store. Task dependency fields are graph-owned: generic create/set/edit + paths cannot introduce a semantic delta, and text-level lint repair skips a would-be + dependency normalization instead of manufacturing unchecked edges. The future guarded + dependency port will own the repository-wide read/validate/write critical section. + Concurrency is **version-CAS** (epic 24): every write, just before committing, re-resolves the file by its **id** and re-hashes it against the content read at the start of the op (`verifyUnchanged` in `cas.go` — a strong whole-file SHA-256 computed on read, **never stored**), so a concurrent diff --git a/docs/cli/tskflwctl.md b/docs/cli/tskflwctl.md index 83f767f8..b88aac0a 100644 --- a/docs/cli/tskflwctl.md +++ b/docs/cli/tskflwctl.md @@ -25,7 +25,7 @@ Local-first planning CLI (tasks, epics, audits, research) over markdown * [tskflwctl config](tskflwctl_config.md) - Inspect, migrate, diagnose, and edit configuration * [tskflwctl epic](tskflwctl_epic.md) - Work with epics * [tskflwctl init](tskflwctl_init.md) - Scaffold a planning tree here, or point at an external planning repo -* [tskflwctl lint](tskflwctl_lint.md) - Validate active task, epic, and research frontmatter (--fix repairs tasks/audits/research and assigns missing ids) +* [tskflwctl lint](tskflwctl_lint.md) - Validate entity frontmatter and task-dependency graph integrity * [tskflwctl research](tskflwctl_research.md) - Work with research docs * [tskflwctl schema](tskflwctl_schema.md) - Describe the tool's contract + per-kind authoring guidance (for agents) * [tskflwctl space](tskflwctl_space.md) - Manage planning spaces and their registered entry points diff --git a/docs/cli/tskflwctl_lint.md b/docs/cli/tskflwctl_lint.md index 41ee113f..b8416772 100644 --- a/docs/cli/tskflwctl_lint.md +++ b/docs/cli/tskflwctl_lint.md @@ -1,6 +1,16 @@ ## tskflwctl lint -Validate active task, epic, and research frontmatter (--fix repairs tasks/audits/research and assigns missing ids) +Validate entity frontmatter and task-dependency graph integrity + +### Synopsis + +Validate task, epic, and research frontmatter, then validate the repository-global +task-dependency graph. Exactly resolved legacy dependency fields are visible +advisories; missing, ambiguous, or structurally unsafe references are errors. + +--fix repairs ordinary frontmatter and missing ids. It never normalizes or changes +graph-owned task fields (depends_on, blocked_by, dependencies, or blocks); a +would-be graph repair is skipped and reported for deliberate remediation. ``` tskflwctl lint [flags] @@ -18,7 +28,7 @@ tskflwctl lint [flags] ### Options ``` - --fix auto-repair frontmatter: quote ':' values, normalize lists, backfill missing task/audit/research ids; epics are text-only + --fix auto-repair ordinary frontmatter and missing ids; graph-owned task fields are skipped -h, --help help for lint --links also check body cross-links: flag any [..](path.md) whose target file is missing (opt-in — a tree can carry pre-existing danglers) ``` diff --git a/docs/cli/tskflwctl_task_edit.md b/docs/cli/tskflwctl_task_edit.md index da7946e4..7dd21d6d 100644 --- a/docs/cli/tskflwctl_task_edit.md +++ b/docs/cli/tskflwctl_task_edit.md @@ -7,7 +7,11 @@ Open a task in your editor (whole file; re-validated on save) Open the task's markdown file in $VISUAL/$EDITOR (falling back to vi). On save the file is re-parsed: a frontmatter break (or a value the loader can't read) reopens the editor with the error rather than landing on disk — deeper -field checks remain `lint`'s job. The human counterpart to `task set`; agents +field checks remain `lint`'s job. Graph-owned dependency fields are preservation- +only here: a semantic change is rejected, and a malformed dependency baseline +must be repaired deliberately before any edited candidate can land. + +The human counterpart to `task set`; agents and scripts should drive `set` (deterministic) instead. ``` diff --git a/docs/cli/tskflwctl_task_set.md b/docs/cli/tskflwctl_task_set.md index f8036246..b93f6ba2 100644 --- a/docs/cli/tskflwctl_task_set.md +++ b/docs/cli/tskflwctl_task_set.md @@ -2,6 +2,13 @@ Set one or more frontmatter fields (validated, single atomic write) +### Synopsis + +Set one or more task frontmatter fields in a single validated atomic write. +Graph-owned dependency fields (depends_on and the legacy blocked_by, dependencies, +and blocks fields) cannot be changed or removed here, including with --force. +They require the guarded dependency operations introduced by the dependency roadmap. + ``` tskflwctl task set [flags] ``` diff --git a/internal/cli/edit.go b/internal/cli/edit.go index c08f7612..1be1b2b6 100644 --- a/internal/cli/edit.go +++ b/internal/cli/edit.go @@ -23,7 +23,10 @@ func newTaskEditCmd(app *App) *cobra.Command { Long: "Open the task's markdown file in $VISUAL/$EDITOR (falling back to vi). On\n" + "save the file is re-parsed: a frontmatter break (or a value the loader can't\n" + "read) reopens the editor with the error rather than landing on disk — deeper\n" + - "field checks remain `lint`'s job. The human counterpart to `task set`; agents\n" + + "field checks remain `lint`'s job. Graph-owned dependency fields are preservation-\n" + + "only here: a semantic change is rejected, and a malformed dependency baseline\n" + + "must be repaired deliberately before any edited candidate can land.\n\n" + + "The human counterpart to `task set`; agents\n" + "and scripts should drive `set` (deterministic) instead.", Example: " tskflwctl task edit add-retry-backoff\n tskflwctl task edit # pick from a list", Args: cobra.MaximumNArgs(1), // bare → picker on a TTY; non-interactive needs the slug diff --git a/internal/cli/lint.go b/internal/cli/lint.go index cfa7d269..f0ce4ed3 100644 --- a/internal/cli/lint.go +++ b/internal/cli/lint.go @@ -13,8 +13,14 @@ import ( func newLintCmd(app *App) *cobra.Command { var fix, links bool cmd := &cobra.Command{ - Use: "lint", - Short: "Validate active task, epic, and research frontmatter (--fix repairs tasks/audits/research and assigns missing ids)", + Use: "lint", + Short: "Validate entity frontmatter and task-dependency graph integrity", + Long: "Validate task, epic, and research frontmatter, then validate the repository-global\n" + + "task-dependency graph. Exactly resolved legacy dependency fields are visible\n" + + "advisories; missing, ambiguous, or structurally unsafe references are errors.\n\n" + + "--fix repairs ordinary frontmatter and missing ids. It never normalizes or changes\n" + + "graph-owned task fields (depends_on, blocked_by, dependencies, or blocks); a\n" + + "would-be graph repair is skipped and reported for deliberate remediation.", Example: " tskflwctl lint\n tskflwctl lint --fix --dry-run\n tskflwctl lint --links\n tskflwctl lint --json", Args: cobra.NoArgs, // Read-only by default; --fix opts into mutation explicitly. @@ -26,7 +32,7 @@ func newLintCmd(app *App) *cobra.Command { return runLint(app, links) }, } - cmd.Flags().BoolVar(&fix, "fix", false, "auto-repair frontmatter: quote ':' values, normalize lists, backfill missing task/audit/research ids; epics are text-only") + cmd.Flags().BoolVar(&fix, "fix", false, "auto-repair ordinary frontmatter and missing ids; graph-owned task fields are skipped") cmd.Flags().BoolVar(&links, "links", false, "also check body cross-links: flag any [..](path.md) whose target file is missing (opt-in — a tree can carry pre-existing danglers)") return cmd } diff --git a/internal/cli/task.go b/internal/cli/task.go index f3aeac54..3a60a34f 100644 --- a/internal/cli/task.go +++ b/internal/cli/task.go @@ -507,8 +507,12 @@ func newTaskSetCmd(app *App) *cobra.Command { force bool ) cmd := &cobra.Command{ - Use: "set ", - Short: "Set one or more frontmatter fields (validated, single atomic write)", + Use: "set ", + Short: "Set one or more frontmatter fields (validated, single atomic write)", + Long: "Set one or more task frontmatter fields in a single validated atomic write.\n" + + "Graph-owned dependency fields (depends_on and the legacy blocked_by, dependencies,\n" + + "and blocks fields) cannot be changed or removed here, including with --force.\n" + + "They require the guarded dependency operations introduced by the dependency roadmap.", Example: " tskflwctl task set add-retry-backoff --priority high\n tskflwctl task set --priority high # pick the task from a list", Args: cobra.MaximumNArgs(1), // bare → picker on a TTY; non-interactive needs the slug Annotations: map[string]string{"safety": "mutating"}, diff --git a/internal/cli/testdata/golden/schema_task_json.golden b/internal/cli/testdata/golden/schema_task_json.golden index b671cb99..f48e478b 100644 --- a/internal/cli/testdata/golden/schema_task_json.golden +++ b/internal/cli/testdata/golden/schema_task_json.golden @@ -1 +1 @@ -{"schema_version":"1.49","kind":"task","sections":["Objective","Acceptance criteria","Out of scope","Related"],"body_template":"\n# \u003ctitle\u003e\n\n## Objective\n\n\u003cwhy / what — one short paragraph\u003e\n\n## Acceptance criteria\n\n- [ ] \u003cobservable outcome\u003e\n\n## Out of scope\n\n- \u003cexplicitly excluded\u003e\n\n## Related\n\n- Epic [\u003cepic-id\u003e](../epics/\u003cepic-id\u003e.md)\n","fields":[{"name":"epic","type":"string","required":true,"description":"ID of the epic this task belongs to; must already exist.","example":"17-pm-go-cli"},{"name":"description","type":"string","required":false,"description":"One line summarizing the task (≤200 chars); required once next-up/in-progress.","example":"Add retry backoff to the Strava webhook"},{"name":"effort","type":"string","required":false,"description":"Rough size estimate (free-form).","example":"1-2 hours"},{"name":"tier","type":"int","required":false,"description":"Importance, 1 (highest) – 5 (lowest).","example":"2"},{"name":"priority","type":"string","required":false,"description":"One of: high | medium | low.","example":"medium"},{"name":"autonomy_level","type":"int","required":false,"description":"How autonomously this can be done, 1–5.","example":"3"},{"name":"tags","type":"list","required":true,"description":"At least one topical tag (required at creation).","example":"[cli, core]"}],"conventions":["status lives in frontmatter (authoritative) and is changed only via the lifecycle verbs (start/next/complete/…), which edit it in place — don't edit it directly.","depends_on is a sorted set of stable task IDs owned by the repository-global DAG; use `task depend add/remove` once available — generic `task set` and `task edit` cannot change it.","description is a single line, ≤200 characters.","at least one tag is required at creation.","the filename slug is derived from the title; any title is accepted (colons, dashes, arrows, …) and the full title is kept as the body H1.","an acceptance criterion is `- [x]` (met) or `- [ ]` (not met). A not-met criterion may say WHY with a trailing `· **deferred:** reason` — one of: deferred | n/a | tracked | wontfix. A reason is required for those, and a checked criterion takes no suffix.","never hand-edit the acceptance criteria — `task ac` owns them: --check/--uncheck \u003cn\u003e, --defer/--wontfix/--tracked/--na \u003cn\u003e --reason \u003cwhy\u003e for a state, and --add/--remove/--replace \u003cn\u003e to change which criteria exist."],"templates":[{"kind":"task","name":"default","description":"Standard task scaffold: objective, acceptance criteria, out-of-scope, related epic."}]} +{"schema_version":"1.49","kind":"task","sections":["Objective","Acceptance criteria","Out of scope","Related"],"body_template":"\n# \u003ctitle\u003e\n\n## Objective\n\n\u003cwhy / what — one short paragraph\u003e\n\n## Acceptance criteria\n\n- [ ] \u003cobservable outcome\u003e\n\n## Out of scope\n\n- \u003cexplicitly excluded\u003e\n\n## Related\n\n- Epic [\u003cepic-id\u003e](../epics/\u003cepic-id\u003e.md)\n","fields":[{"name":"epic","type":"string","required":true,"description":"ID of the epic this task belongs to; must already exist.","example":"17-pm-go-cli"},{"name":"description","type":"string","required":false,"description":"One line summarizing the task (≤200 chars); required once next-up/in-progress.","example":"Add retry backoff to the Strava webhook"},{"name":"effort","type":"string","required":false,"description":"Rough size estimate (free-form).","example":"1-2 hours"},{"name":"tier","type":"int","required":false,"description":"Importance, 1 (highest) – 5 (lowest).","example":"2"},{"name":"priority","type":"string","required":false,"description":"One of: high | medium | low.","example":"medium"},{"name":"autonomy_level","type":"int","required":false,"description":"How autonomously this can be done, 1–5.","example":"3"},{"name":"tags","type":"list","required":true,"description":"At least one topical tag (required at creation).","example":"[cli, core]"}],"conventions":["status lives in frontmatter (authoritative) and is changed only via the lifecycle verbs (start/next/complete/…), which edit it in place — don't edit it directly.","depends_on is a sorted set of stable task IDs owned by the repository-global DAG. It is preservation-only until guarded `task depend add/remove` commands land: generic task creation, `task set`, `task edit`, and `lint --fix` cannot add, remove, or reinterpret it.","description is a single line, ≤200 characters.","at least one tag is required at creation.","the filename slug is derived from the title; any title is accepted (colons, dashes, arrows, …) and the full title is kept as the body H1.","an acceptance criterion is `- [x]` (met) or `- [ ]` (not met). A not-met criterion may say WHY with a trailing `· **deferred:** reason` — one of: deferred | n/a | tracked | wontfix. A reason is required for those, and a checked criterion takes no suffix.","never hand-edit the acceptance criteria — `task ac` owns them: --check/--uncheck \u003cn\u003e, --defer/--wontfix/--tracked/--na \u003cn\u003e --reason \u003cwhy\u003e for a state, and --add/--remove/--replace \u003cn\u003e to change which criteria exist."],"templates":[{"kind":"task","name":"default","description":"Standard task scaffold: objective, acceptance criteria, out-of-scope, related epic."}]} diff --git a/internal/domain/entity.go b/internal/domain/entity.go index b096040a..74147107 100644 --- a/internal/domain/entity.go +++ b/internal/domain/entity.go @@ -71,7 +71,7 @@ var entities = []Descriptor{ }, Conventions: []string{ "status lives in frontmatter (authoritative) and is changed only via the lifecycle verbs (start/next/complete/…), which edit it in place — don't edit it directly.", - "depends_on is a sorted set of stable task IDs owned by the repository-global DAG; use `task depend add/remove` once available — generic `task set` and `task edit` cannot change it.", + "depends_on is a sorted set of stable task IDs owned by the repository-global DAG. It is preservation-only until guarded `task depend add/remove` commands land: generic task creation, `task set`, `task edit`, and `lint --fix` cannot add, remove, or reinterpret it.", fmt.Sprintf("description is a single line, ≤%d characters.", MaxDescriptionLen), "at least one tag is required at creation.", "the filename slug is derived from the title; any title is accepted (colons, dashes, arrows, …) and the full title is kept as the body H1.",