docs(claude-do): plan for multi-dependency depends_on relation table
This commit is contained in:
Mika Kuns
@@ -0,0 +1,147 @@
|
|||||||
|
# Mehrfach-Abhängigkeiten (`depends_on` als Menge) — Implementation Plan
|
||||||
|
|
||||||
|
**Goal:** A task can declare *n* predecessors instead of exactly one. The queue picker
|
||||||
|
claims it only once **all** of them are `Done`.
|
||||||
|
|
||||||
|
**Architecture:** Replace the single `tasks.depends_on_task_id` column with a
|
||||||
|
`task_dependencies(task_id, depends_on_task_id)` relation table (composite PK, both FKs to
|
||||||
|
`tasks.id`, `ON DELETE CASCADE`). Cascade reproduces today's `ON DELETE SET NULL` semantics
|
||||||
|
on a join table: deleting a predecessor drops the edge, so the dependent becomes pickable
|
||||||
|
instead of blocked forever. `BlockedByTaskId` (the internal planning chain) is untouched —
|
||||||
|
the split stays deliberate.
|
||||||
|
|
||||||
|
**Tech Stack:** .NET 8, EF Core (SQLite), SignalR, Avalonia 12, xUnit.
|
||||||
|
|
||||||
|
**Not in scope:** `BlockedByTaskId`; rewriting the list-handler prompt to *use* multi-deps;
|
||||||
|
a graph visualizer.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## File Structure
|
||||||
|
|
||||||
|
**Create:**
|
||||||
|
- `src/ClaudeDo.Data/Models/TaskDependencyEntity.cs`
|
||||||
|
- `src/ClaudeDo.Data/Configuration/TaskDependencyEntityConfiguration.cs`
|
||||||
|
- `src/ClaudeDo.Data/Migrations/<ts>_AddTaskDependencies.cs` — scaffolded
|
||||||
|
- `src/ClaudeDo.Worker/External/TaskBlocking.cs` — the shared `blockedReason` builder
|
||||||
|
(two consumers: `ExternalMcpService`, `TaskWaitMcpTools`)
|
||||||
|
- `tests/ClaudeDo.Data.Tests/TaskDependencyMigrationTests.cs` — real `Migrate()` on a DB
|
||||||
|
seeded with legacy `depends_on_task_id` rows
|
||||||
|
|
||||||
|
**Modify:**
|
||||||
|
- `src/ClaudeDo.Data/Models/TaskEntity.cs` — `DependsOnTaskId` → `Dependencies` nav +
|
||||||
|
`[NotMapped] DependsOnTaskIds`
|
||||||
|
- `src/ClaudeDo.Data/Configuration/TaskEntityConfiguration.cs` — drop column/FK/index
|
||||||
|
- `src/ClaudeDo.Data/ClaudeDoDbContext.cs` — `DbSet<TaskDependencyEntity>`
|
||||||
|
- `src/ClaudeDo.Data/Repositories/TaskRepository.cs` — `.Include(t => t.Dependencies)` on
|
||||||
|
the three read paths; `UpdateAsync` must not push the nav graph
|
||||||
|
- `src/ClaudeDo.Worker/Queue/QueuePicker.cs` — `NOT EXISTS` over the relation table
|
||||||
|
- `src/ClaudeDo.Worker/State/TaskStateService.cs` — `SetDependsOnAsync` takes a set,
|
||||||
|
graph-wide cycle check
|
||||||
|
- `src/ClaudeDo.Worker/State/Interfaces/ITaskStateService.cs`
|
||||||
|
- `src/ClaudeDo.Worker/External/ExternalMcpService.cs` — DTOs, `add_task`, `update_task`,
|
||||||
|
`ComputeBlockedInfoAsync`
|
||||||
|
- `src/ClaudeDo.Worker/External/TaskWaitMcpTools.cs`
|
||||||
|
- `src/ClaudeDo.Ui/ViewModels/Islands/TaskRowViewModel.cs`,
|
||||||
|
`TasksIslandViewModel.cs` — display of *n* predecessors
|
||||||
|
- `src/ClaudeDo.Data/CLAUDE.md`, `src/ClaudeDo.Worker/CLAUDE.md`
|
||||||
|
- test files listed per task below
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 1 — Schema + migration
|
||||||
|
|
||||||
|
- [ ] `TaskDependencyEntity { string TaskId; string DependsOnTaskId; }`, table
|
||||||
|
`task_dependencies`, composite PK, both FKs cascade, index on `depends_on_task_id`.
|
||||||
|
- [ ] `TaskEntity.Dependencies` (`ICollection<TaskDependencyEntity>`), `[NotMapped]
|
||||||
|
DependsOnTaskIds` projection. `DependsOnTaskId` and its FK/index are removed.
|
||||||
|
- [ ] Scaffold the migration, then **hand-edit the Up body** so the data copy sits between
|
||||||
|
create and drop:
|
||||||
|
`CreateTable` → `INSERT INTO task_dependencies (task_id, depends_on_task_id) SELECT
|
||||||
|
id, depends_on_task_id FROM tasks WHERE depends_on_task_id IS NOT NULL` →
|
||||||
|
`DropColumn`. Down reverses it (add column, `UPDATE … FROM` the first edge, drop
|
||||||
|
table) — lossy for >1 edge, which is inherent and documented in the migration.
|
||||||
|
- [ ] Verify: `dotnet ef database update` against a temp DB seeded with legacy rows;
|
||||||
|
`pragma_table_info` diff. **`DropColumn` forces a SQLite table rebuild** — the
|
||||||
|
`ef-migration-parallel-designer-collision` trap applies: if another migration lands
|
||||||
|
in parallel, hand-check both `.Designer.cs` snapshots.
|
||||||
|
|
||||||
|
**Test:** `TaskDependencyMigrationTests` — migrate to the previous migration, seed a task
|
||||||
|
with `depends_on_task_id` via raw SQL, migrate to head, assert the edge is in
|
||||||
|
`task_dependencies` and nothing else changed. (Acceptance #2.)
|
||||||
|
|
||||||
|
## Task 2 — Queue gating
|
||||||
|
|
||||||
|
- [ ] `QueuePicker` replaces the `depends_on_task_id IS NULL OR EXISTS(...)` clause with
|
||||||
|
a doubly-negated `NOT EXISTS` over `task_dependencies`, so a dangling edge keeps
|
||||||
|
blocking exactly as today:
|
||||||
|
```sql
|
||||||
|
AND NOT EXISTS (
|
||||||
|
SELECT 1 FROM task_dependencies td
|
||||||
|
WHERE td.task_id = t.id
|
||||||
|
AND NOT EXISTS (SELECT 1 FROM tasks d
|
||||||
|
WHERE d.id = td.depends_on_task_id AND d.status = 'done')
|
||||||
|
)
|
||||||
|
```
|
||||||
|
|
||||||
|
**Tests** (`QueuePickerTests`, real SQLite): n=2 both `Done` → claimed; one `Done` one
|
||||||
|
`Running` → skipped; zero deps → claimed (regression); one dep `Failed` → still skipped,
|
||||||
|
no cascade (acceptance #6); one dep `Cancelled` → still skipped.
|
||||||
|
|
||||||
|
## Task 3 — Write path + cycle detection
|
||||||
|
|
||||||
|
- [ ] `ITaskStateService.SetDependsOnAsync(string taskId, IReadOnlyList<string>
|
||||||
|
dependsOnTaskIds, CancellationToken)` — empty list clears. Rejects self-reference,
|
||||||
|
unknown id, duplicate handling by de-dup.
|
||||||
|
- [ ] Cycle check walks the **graph**: load the whole edge set once (it only ever holds
|
||||||
|
*declared* dependencies, so it stays small — `ponytail:` note, switch to a recursive
|
||||||
|
CTE if it ever isn't), BFS from every proposed predecessor; reaching `taskId` is a
|
||||||
|
cycle. Visited set guards pre-existing bad data.
|
||||||
|
- [ ] Replace rows transactionally (`ExecuteDelete` for `taskId` + `AddRange`), wake the
|
||||||
|
queue when the set shrank.
|
||||||
|
|
||||||
|
**Tests** (`TaskStateServiceTests`): sets n edges; empty list clears + wakes; self-ref
|
||||||
|
rejected; unknown id rejected; **A→B→C→A rejected** (acceptance #3); adding a second edge
|
||||||
|
leaves the first intact.
|
||||||
|
|
||||||
|
## Task 4 — MCP surface
|
||||||
|
|
||||||
|
- [ ] `add_task` / `update_task` keep `dependsOnTaskId` (string) **and** gain
|
||||||
|
`dependsOnTaskIds` (`string[]?`). Precedence: `dependsOnTaskIds` non-null wins;
|
||||||
|
otherwise the legacy scalar is treated as a one-element set, with `""` still the
|
||||||
|
clear sentinel. Both run through `TaskIdResolver` so `#123` keeps working.
|
||||||
|
- [ ] `TaskDto`/`TaskRefDto`: `DependsOnTaskId` → `DependsOnTaskIds`
|
||||||
|
(`IReadOnlyList<string>`). Only the *input* form is contractually frozen.
|
||||||
|
- [ ] `TaskBlocking.Describe` builds one reason naming **every** unmet predecessor
|
||||||
|
(acceptance #4); `ComputeBlockedInfoAsync` and `TaskWaitMcpTools.CheckOnceAsync`
|
||||||
|
both use it.
|
||||||
|
|
||||||
|
**Tests** (`ExternalMcpServiceTests`, `TaskWaitMcpToolsTests`, `BatchMcpToolsTests`):
|
||||||
|
legacy scalar still sets + clears the link (acceptance #5, explicit back-compat test);
|
||||||
|
`dependsOnTaskIds` sets n; cycle via the list form throws; `blockedReason` names both
|
||||||
|
open predecessors.
|
||||||
|
|
||||||
|
## Task 5 — UI
|
||||||
|
|
||||||
|
- [ ] `TaskRowViewModel.DependsOnTaskIds`; `DependsOnTaskId` becomes a computed
|
||||||
|
"single predecessor or null" so the existing linear-chain rail is unchanged for the
|
||||||
|
1-dependency case.
|
||||||
|
- [ ] `BuildAfterLabel` joins all resolvable predecessors (`#3, #7`); a row with >1
|
||||||
|
predecessor gets the label, never a rail (a rail can only render a line).
|
||||||
|
- [ ] `GroupingKey` folds the joined ids so the reconcile tick still detects a change.
|
||||||
|
- [ ] `.Include(t => t.Dependencies)` on the `TasksIslandViewModel` query sites that
|
||||||
|
hydrate rows.
|
||||||
|
|
||||||
|
**Tests** (`TasksIslandChainTests`, `TasksIslandClassifyItemsTests`,
|
||||||
|
`TasksIslandReconcileTickTests`): existing chain tests keep passing via the 1-dep path;
|
||||||
|
a 2-dep row shows a joined after-label and no rail.
|
||||||
|
|
||||||
|
## Task 6 — Docs + verify
|
||||||
|
|
||||||
|
- [ ] `src/ClaudeDo.Data/CLAUDE.md` (Models, Schema) and
|
||||||
|
`src/ClaudeDo.Worker/CLAUDE.md` (Status Model table, picker bullet,
|
||||||
|
`TaskStateService` bullet) describe the relation table (acceptance #7).
|
||||||
|
- [ ] VERIFY (acceptance #8): `dotnet build src/ClaudeDo.App -c Release`,
|
||||||
|
`dotnet build src/ClaudeDo.Worker -c Release`, `dotnet test` for
|
||||||
|
`ClaudeDo.Data.Tests`, `ClaudeDo.Worker.Tests`, `ClaudeDo.Ui.Tests` (`-c Release`),
|
||||||
|
report counts.
|
||||||
Reference in New Issue
Block a user