From 44376f07091569c640509c964c3a3e3d35689874 Mon Sep 17 00:00:00 2001 From: kami Date: Mon, 27 Jul 2026 23:26:24 +0400 Subject: [PATCH] fix(store): TaskAmended applies description/priority/due, not just title (S7) Amendments to due/description/inherent_priority were accepted and durably logged but silently discarded by the projection since store.apply only ever handled the title key. Also added the missing Task.Description field (TaskCreated never populated it either). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01W1rkJ2hBMybnJctPbcy4tT --- AUDIT.md | 15 +++++++++++++++ internal/domain/domain.go | 1 + internal/store/store.go | 18 +++++++++++++++--- internal/store/store_test.go | 31 +++++++++++++++++++++++++++++++ progress.md | 9 ++++++++- 5 files changed, 70 insertions(+), 4 deletions(-) diff --git a/AUDIT.md b/AUDIT.md index 8cf6711..3b1cc6a 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -867,6 +867,21 @@ both come through in the brief. `go build ./...`, `go vet ./...`, `go test ./...` all pass. +### S7 — closed, 2026-07-27 + +`store.apply`'s `TaskAmended` case only ever applied `title` from the +amendment payload — `due`, `description`, and `inherent_priority` amendments +were accepted by `ValidatePayload` (which only checks the payload is +non-empty) and durably logged, but silently dropped by the projection, so a +client reading back the task would never see them take effect. Also found in +passing: `domain.Task` had no `Description` field at all, so a description +couldn't be amended onto a task even if the projection had handled it — +`TaskCreated` discarded it too. Added `Task.Description`; both `TaskCreated` +and `TaskAmended` in `store.apply` now populate/update all four fields +(`title`, `description`, `inherent_priority`, `due`), matching what §4 lists +as amendable. Covered by `TestTaskAmendedAppliesAllFields` +(`internal/store/store_test.go`). + ### Design consequences (not yet implemented) 1. **Percentages are a level, not a delta.** diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 74b6251..a061cf3 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -61,6 +61,7 @@ type Task struct { Lease *Lease `json:"lease,omitempty"` Version int `json:"version"` Title string `json:"title,omitempty"` + Description string `json:"description,omitempty"` } type Event struct { diff --git a/internal/store/store.go b/internal/store/store.go index 167c26d..79d7066 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -135,6 +135,9 @@ func (s *Store) apply(e domain.Event) error { if v, ok := p["title"].(string); ok { t.Title = v } + if v, ok := p["description"].(string); ok { + t.Description = v + } s.external[t.Source+"\x00"+t.ExternalID] = t.ID case "TaskLeased": t.State = domain.StateLeased @@ -152,9 +155,18 @@ func (s *Store) apply(e domain.Event) error { t.State = domain.StateBlocked t.Lease = nil case "TaskAmended": - for k, v := range p { - if k == "title" { - t.Title, v = v.(string) + if v, ok := p["title"].(string); ok { + t.Title = v + } + if v, ok := p["description"].(string); ok { + t.Description = v + } + if v, ok := p["inherent_priority"].(float64); ok { + t.InherentPriority = int(v) + } + if v, ok := p["due"].(string); ok { + if d, err := time.Parse(time.RFC3339, v); err == nil { + t.Due = &d } } } diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 605afac..9024852 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -55,6 +55,37 @@ func TestAppendReplayAndDeduplicate(t *testing.T) { } } +// TestTaskAmendedAppliesAllFields guards S7: the projection previously only +// applied "title" from a TaskAmended payload, silently discarding due, +// description, and inherent_priority amendments even though they were +// accepted and logged. +func TestTaskAmendedAppliesAllFields(t *testing.T) { + dir := t.TempDir() + s, err := Open(dir) + if err != nil { + t.Fatal(err) + } + if err := s.Append(created("e1")); err != nil { + t.Fatal(err) + } + amend, _ := json.Marshal(map[string]any{ + "title": "new title", + "description": "new description", + "inherent_priority": 5.0, + "due": "2026-08-01T00:00:00Z", + }) + if err := s.Append(domain.Event{Type: "TaskAmended", TaskID: "task-1", Version: 2, Payload: amend, Surface: string(authz.System)}); err != nil { + t.Fatal(err) + } + tk := s.Tasks()[0] + if tk.Title != "new title" || tk.Description != "new description" || tk.InherentPriority != 5 { + t.Fatalf("unexpected task after amendment: %+v", tk) + } + if tk.Due == nil || !tk.Due.Equal(time.Date(2026, 8, 1, 0, 0, 0, 0, time.UTC)) { + t.Fatalf("unexpected due after amendment: %+v", tk.Due) + } +} + // TestLeaseAndExpireEventIDsAreUnique guards S5: Event.ID was set to the // task id in both Lease and ExpireLeases, so every lease of the same task // produced a TaskLeased/TaskReleased event with a colliding ID — unsound diff --git a/progress.md b/progress.md index 4b2b0d4..4c30a62 100644 --- a/progress.md +++ b/progress.md @@ -219,7 +219,14 @@ Fixed so far: `receipt` straight out of each `TaskCompleted` event's existing payload. Covered by an updated `TestBuildBrief`. -Not yet started: Codex/opencode completion producers, S7–S11. See +- **S7** — `TaskAmended` only ever applied `title` from the amendment + payload; `due`/`description`/`inherent_priority` amendments were accepted + and logged but silently dropped by the projection. Also added the missing + `Task.Description` field (it didn't exist at all, so `TaskCreated` dropped + it too). Both `TaskCreated` and `TaskAmended` now populate all four fields. + Covered by `TestTaskAmendedAppliesAllFields` (`internal/store`). + +Not yet started: Codex/opencode completion producers, S8–S11. See `AUDIT.md` for the full plan. **Phase 0 done (2026-07-27):** this box has live TCP reachability to the real