Merge branch 'main' into feat/poweruse-list-handler
Kollision mit #264 (Handoff-Budget im Brief sichtbar machen): beide Seiten haben HandoffRoundTracker, HandoffMcpTools und InteractiveLaunchSpecService angefasst. Der Konflikt war inhaltlich, nicht nur textuell. #264 liest den Cap an zwei Stellen aus der Konstante (GetStatus und das roundInfo-Rendering), waehrend dieser Branch ihn zum Parameter aus app_settings gemacht hat. Ein reiner Text-Merge haette kompiliert und trotzdem gelogen: Register haette den konfigurierten Cap durchgesetzt, waehrend der Brief dem Modell weiter "2 Runden uebrig" gemeldet haette, obwohl es 39 hat. Aufgeloest so, dass der Cap eine einzige Quelle hat: - GetStatus(taskId, maxHandoffs) nimmt den Cap als Parameter, mit demselben Clamp wie Register. Die Rechnerei liegt damit an einer Stelle. - GetHandoffStatus liest das Setting und gibt es weiter. - Das roundInfo-Rendering benutzt GetStatus statt eigener Arithmetik. Zwei Tests nageln genau diesen Drift fest, damit er beim naechsten Merge nicht unbemerkt zurueckkommt: einer auf GetHandoffStatus, einer auf den Brief. MaxHandoffs -> DefaultMaxHandoffs in #264s vier Test-Referenzen; die laufen alle mit Default-Settings, die Semantik bleibt.
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