diff --git a/cmd/codeaf/appworkstreams.go b/cmd/codeaf/appworkstreams.go index 138e9753de..4be96ab2d6 100644 --- a/cmd/codeaf/appworkstreams.go +++ b/cmd/codeaf/appworkstreams.go @@ -480,11 +480,11 @@ func workstreamSource(p *appwire.WorkstreamSource) *workstream.Source { return &workstream.Source{Kind: p.Kind, ID: p.ID, ChatID: workstreamValue(p.ChatID), Factory: workstreamValue(p.Factory), Label: workstreamValue(p.Label)} } func appWorkstreamPageNode(p workstream.PageNode) appwire.WorkstreamPageNode { - return appwire.WorkstreamPageNode{ID: p.ID, WorkstreamID: p.WorkstreamID, ParentID: workstreamOptional(p.ParentID), Title: p.Title, Owner: p.Owner, ReadByJobs: p.ReadByJobs, Position: p.Position, Version: p.Version, UpdatedAt: p.UpdatedAt, UpdatedBy: p.UpdatedBy, UpdatedFrom: appWorkstreamSource(p.UpdatedFrom)} + return appwire.WorkstreamPageNode{ID: p.ID, WorkstreamID: p.WorkstreamID, ParentID: workstreamOptional(p.ParentID), Title: p.Title, Owner: p.Owner, ReadByJobs: p.ReadByJobs, Position: p.Position, Version: p.Version, UpdatedAt: p.UpdatedAt, UpdatedBy: p.UpdatedBy, UpdatedFrom: appWorkstreamSource(p.UpdatedFrom), DeletedAt: p.DeletedAt} } func appWorkstreamPage(p workstream.Page) appwire.WorkstreamPage { n := appWorkstreamPageNode(p.PageNode) - return appwire.WorkstreamPage{ID: n.ID, WorkstreamID: n.WorkstreamID, ParentID: n.ParentID, Title: n.Title, Owner: n.Owner, ReadByJobs: n.ReadByJobs, Position: n.Position, Version: n.Version, UpdatedAt: n.UpdatedAt, UpdatedBy: n.UpdatedBy, UpdatedFrom: n.UpdatedFrom, Doc: p.Doc, Markdown: p.Markdown} + return appwire.WorkstreamPage{ID: n.ID, WorkstreamID: n.WorkstreamID, ParentID: n.ParentID, Title: n.Title, Owner: n.Owner, ReadByJobs: n.ReadByJobs, Position: n.Position, Version: n.Version, UpdatedAt: n.UpdatedAt, UpdatedBy: n.UpdatedBy, UpdatedFrom: n.UpdatedFrom, DeletedAt: n.DeletedAt, Doc: p.Doc, Markdown: p.Markdown} } func (b *appBackend) WorkstreamPageList(p appwire.WorkstreamPageListParams) (appwire.WorkstreamPageListResult, error) { return workstreamDo(b, p, func(s *workstream.Store) (appwire.WorkstreamPageListResult, workstream.Change, error) { @@ -493,12 +493,26 @@ func (b *appBackend) WorkstreamPageList(p appwire.WorkstreamPageListParams) (app for _, page := range pages { out.Pages = append(out.Pages, appWorkstreamPageNode(page)) } + if err == nil && workstreamValue(p.IncludeDeleted) { + var deleted []workstream.PageNode + deleted, err = s.DeletedPages(context.Background(), p.WorkstreamID) + out.Deleted = []appwire.WorkstreamPageNode{} + for _, page := range deleted { + out.Deleted = append(out.Deleted, appWorkstreamPageNode(page)) + } + } return out, workstream.Change{}, err }) } func (b *appBackend) WorkstreamPageGet(p appwire.WorkstreamPageGetParams) (appwire.WorkstreamPageGetResult, error) { return workstreamDo(b, p, func(s *workstream.Store) (appwire.WorkstreamPageGetResult, workstream.Change, error) { - v, err := s.Page(context.Background(), p.ID, workstreamValue(p.Markdown)) + var v workstream.Page + var err error + if p.Version != nil { + v, err = s.PageAtVersion(context.Background(), p.ID, *p.Version, workstreamValue(p.Markdown)) + } else { + v, err = s.Page(context.Background(), p.ID, workstreamValue(p.Markdown)) + } return appwire.WorkstreamPageGetResult{Page: appWorkstreamPage(v)}, workstream.Change{}, err }) } @@ -510,7 +524,11 @@ func (b *appBackend) WorkstreamPageSave(p appwire.WorkstreamPageSaveParams) (app } func (b *appBackend) WorkstreamPageCreate(p appwire.WorkstreamPageCreateParams) (appwire.WorkstreamPageCreateResult, error) { return workstreamDo(b, p, func(s *workstream.Store) (appwire.WorkstreamPageCreateResult, workstream.Change, error) { - v, c, err := s.CreatePage(context.Background(), workstream.PageCreate{WorkstreamID: p.WorkstreamID, ParentID: workstreamValue(p.ParentID), AfterID: workstreamValue(p.AfterID), Title: p.Title, Owner: workstreamValue(p.Owner), ReadByJobs: workstreamValue(p.ReadByJobs), Doc: p.Doc}) + read := workstream.ReadByJobsDefault(p.Title) + if p.ReadByJobs != nil { + read = *p.ReadByJobs + } + v, c, err := s.CreatePage(context.Background(), workstream.PageCreate{WorkstreamID: p.WorkstreamID, ParentID: workstreamValue(p.ParentID), AfterID: workstreamValue(p.AfterID), Title: p.Title, Owner: workstreamValue(p.Owner), ReadByJobs: read, Doc: p.Doc}) return appwire.WorkstreamPageCreateResult{Page: appWorkstreamPage(v)}, c, err }) } @@ -548,7 +566,14 @@ func (b *appBackend) WorkstreamPageHistory(p appwire.WorkstreamPageHistoryParams } func (b *appBackend) WorkstreamPageRestore(p appwire.WorkstreamPageRestoreParams) (appwire.WorkstreamPageRestoreResult, error) { return workstreamDo(b, p, func(s *workstream.Store) (appwire.WorkstreamPageRestoreResult, workstream.Change, error) { - v, c, err := s.RestorePage(context.Background(), p.ID, p.Version, p.BaseVersion, workstream.Attribution{}) + var v workstream.Page + var c workstream.Change + var err error + if p.Version == nil { + v, c, err = s.UndeletePage(context.Background(), p.ID) + } else { + v, c, err = s.RestorePage(context.Background(), p.ID, *p.Version, *p.BaseVersion, workstream.Attribution{}) + } return appwire.WorkstreamPageRestoreResult{Page: appWorkstreamPage(v)}, c, err }) } diff --git a/cmd/codeaf/appworkstreams_conformance_test.go b/cmd/codeaf/appworkstreams_conformance_test.go index f39b158764..b022e5e27f 100644 --- a/cmd/codeaf/appworkstreams_conformance_test.go +++ b/cmd/codeaf/appworkstreams_conformance_test.go @@ -578,8 +578,24 @@ func workstreamWireSeed(t *testing.T, name string, situation int) *workstreamWir if name == "page.delete" { fixture.page(t, "conformance-descendant", fixture.IDs["pg_32e1"]) } - case "page.history", "page.restore": + case "page.history": fixture.saveByAgent(t) + // WP-F1 (g): prepare all three additive example_1 pairs with the exact + // optional keys illustrated by the wire contract. + case "page.list": + if situation == 1 { + fixture.deletedPage(t) + } + case "page.get": + if situation == 1 { + fixture.saveByAgent(t) + } + case "page.restore": + if situation == 1 { + fixture.deletedPage(t) + } else { + fixture.saveByAgent(t) + } } if name == "record.list" || name == "record.keep" || name == "record.drop" || name == "record.supersede" || name == "workstream.changed" { fixture.record(t) @@ -611,6 +627,18 @@ func (fixture *workstreamWireFixture) page(t *testing.T, alias, parent string) { fixture.IDs[alias] = page.Page.ID } +func (fixture *workstreamWireFixture) deletedPage(t *testing.T) { + t.Helper() + fixture.page(t, "pg_33f2", fixture.IDs["pg_31d0"]) + id := fixture.IDs["pg_33f2"] + if _, err := fixture.Backend.WorkstreamPageSave(appwire.WorkstreamPageSaveParams{ID: id, BaseVersion: 1, Doc: json.RawMessage(`{"type":"doc","content":[]}`)}); err != nil { + t.Fatal(err) + } + if _, err := fixture.Backend.WorkstreamPageDelete(appwire.WorkstreamPageDeleteParams{ID: id}); err != nil { + t.Fatal(err) + } +} + func (fixture *workstreamWireFixture) source() *appwire.WorkstreamSource { return &appwire.WorkstreamSource{Kind: "message", ID: "msg_19a2", ChatID: workstreamPtr(fixture.IDs["conv_5b9e"]), Label: workstreamPtr("Billing scope")} } @@ -746,10 +774,11 @@ func (contract *workstreamWireContract) checkError(t *testing.T, path string, va } type workstreamWireErrorCase struct { - Name string - Method string - Code string - Example string + Name string + Method string + Code string + Example string + ParamsExample int } // TestAppWorkstreamWireConformance keeps the table independent of the fixtures: @@ -767,6 +796,12 @@ func TestAppWorkstreamWireConformance(t *testing.T) { errorCases := []workstreamWireErrorCase{ {Name: "bad_params", Method: "workstream.get", Code: "bad_params"}, {Name: "not_found", Method: "workstream.get", Code: "not_found", Example: "workstream.get"}, + // WP-F1 (a), (b), (g): new version and restore error doors use real dispatch. + {Name: "get_unknown_version", Method: "page.get", Code: "not_found", ParamsExample: 1}, + {Name: "restore_version_only", Method: "page.restore", Code: "bad_params"}, + {Name: "restore_base_only", Method: "page.restore", Code: "bad_params"}, + {Name: "restore_live_id_only", Method: "page.restore", Code: "bad_params", ParamsExample: 1}, + {Name: "restore_deleted_version", Method: "page.restore", Code: "bad_params"}, {Name: "conflict", Method: "workstream.create", Code: "conflict"}, {Name: "collection_owner", Method: "collections.add", Code: "in_other_workstream", Example: "collections.add"}, {Name: "create_owner", Method: "workstream.create", Code: "in_other_workstream", Example: "collections.add"}, @@ -1155,14 +1190,29 @@ func workstreamWireError(t *testing.T, contract *workstreamWireContract, files m fixture := workstreamWireSeed(t, "workstream.get", 0) backend := fixture.Backend examples := files[errorCase.Method] - params := fixture.prepare(t, examples.Examples.Params[0]) + params := fixture.prepare(t, examples.Examples.Params[errorCase.ParamsExample]) var engine appwire.Engine = &workstreamWireBackend{appBackend: backend} release := func() {} + changedShape := false switch errorCase.Name { case "bad_params": delete(params, "id") + changedShape = true case "not_found": params["id"] = "missing-workstream" + case "get_unknown_version": + params["version"] = json.Number("99") + case "restore_version_only": + delete(params, "base_version") + changedShape = true + case "restore_base_only": + delete(params, "version") + changedShape = true + case "restore_live_id_only": + params["id"] = fixture.IDs["pg_31d0"] + case "restore_deleted_version": + fixture.deletedPage(t) + params["id"] = fixture.IDs["pg_33f2"] case "conflict": params["name"] = fixture.Stream.Name case "collection_owner": @@ -1180,6 +1230,7 @@ func workstreamWireError(t *testing.T, contract *workstreamWireContract, files m params["id"] = fixture.IDs["pg_32e1"] params["parent_id"] = fixture.IDs["conformance-descendant"] case "too_large": + changedShape = true params["doc"] = map[string]any{"type": "doc", "content": []any{ map[string]any{"type": "paragraph", "content": []any{ map[string]any{"type": "text", "text": strings.Repeat("x", (1<<20)+1)}, @@ -1192,9 +1243,9 @@ func workstreamWireError(t *testing.T, contract *workstreamWireContract, files m default: t.Fatalf("unimplemented error case %s", errorCase.Name) } - if errorCase.Name != "bad_params" && errorCase.Name != "too_large" { + if !changedShape { comparison := &workstreamWireComparison{} - workstreamWireCheck(t, comparison.shape(errorCase.Method+" error params ", params, workstreamWireDecode(t, examples.Examples.Params[0]), workstreamWirePreparedShape)) + workstreamWireCheck(t, comparison.shape(errorCase.Method+" error params ", params, workstreamWireDecode(t, examples.Examples.Params[errorCase.ParamsExample]), workstreamWirePreparedShape)) } connection := workstreamWireConnect(t, engine) frame := connection.call(t, errorCase.Method, params) diff --git a/cmd/codeaf/appworkstreams_pages_test.go b/cmd/codeaf/appworkstreams_pages_test.go new file mode 100644 index 0000000000..6d89ba1d6f --- /dev/null +++ b/cmd/codeaf/appworkstreams_pages_test.go @@ -0,0 +1,192 @@ +package main + +import ( + "context" + "encoding/json" + "reflect" + "testing" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/workstream" +) + +// WP-F1 (c): creation alone chooses the title default; explicit booleans win, +// renames retain the existing choice, and the initial Overview remains off. +func TestAppWorkstreamPageReadByJobsDefaults(t *testing.T) { + b, _, _, _ := workstreamBackend(t) + w := workstreamCreateForTest(t, b, "Read defaults") + pages, err := b.WorkstreamPageList(appwire.WorkstreamPageListParams{WorkstreamID: w.ID}) + if err != nil || len(pages.Pages) != 1 || pages.Pages[0].ReadByJobs { + t.Fatalf("initial Overview: %+v %v", pages, err) + } + for _, tc := range []struct { + title string + value *bool + want bool + }{ + {"Spec", nil, true}, {" decisions ", nil, true}, {"DECISIONS", nil, true}, + {"Spec notes", nil, false}, {"Overview", nil, false}, {"Notes", nil, false}, + {"Spec", workstreamPtr(false), false}, {"Notes", workstreamPtr(true), true}, + } { + t.Run(tc.title, func(t *testing.T) { + made, err := b.WorkstreamPageCreate(appwire.WorkstreamPageCreateParams{WorkstreamID: w.ID, Title: tc.title, ReadByJobs: tc.value}) + if err != nil || made.Page.ReadByJobs != tc.want { + t.Fatalf("created: %+v %v, want read_by_jobs=%v", made, err, tc.want) + } + rename := "Spec" + if tc.want { + rename = "Notes" + } + updated, err := b.WorkstreamPageUpdate(appwire.WorkstreamPageUpdateParams{ID: made.Page.ID, Title: &rename}) + if err != nil || updated.Page.ReadByJobs != tc.want { + t.Fatalf("rename changed read_by_jobs: %+v %v", updated, err) + } + }) + } +} + +// WP-F1 (a): handlers expose historical content and attribution, optional +// markdown and current metadata, including deleted_at for a deleted page. +func TestAppWorkstreamPageGetVersion(t *testing.T) { + b, rec, _, _ := workstreamBackend(t) + w := workstreamCreateForTest(t, b, "Version handler") + store, err := b.workstreamStore(false) + if err != nil || store == nil { + t.Fatalf("store: %v", err) + } + doc := json.RawMessage(`{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"Earlier"}]}]}`) + first, _, err := store.CreatePage(context.Background(), workstream.PageCreate{WorkstreamID: w.ID, Title: "Before", Doc: doc, + Attribution: workstream.Attribution{By: "agent", From: &workstream.Source{Kind: "chat", ID: "old-source"}}}) + if err != nil { + t.Fatal(err) + } + if _, err := b.WorkstreamPageSave(appwire.WorkstreamPageSaveParams{ID: first.ID, BaseVersion: 1, Doc: json.RawMessage(`{"type":"doc","content":[]}`)}); err != nil { + t.Fatal(err) + } + if _, err := b.WorkstreamPageUpdate(appwire.WorkstreamPageUpdateParams{ID: first.ID, Title: workstreamPtr("After"), ReadByJobs: workstreamPtr(true)}); err != nil { + t.Fatal(err) + } + before := len(workstreamEvents(rec)) + current, err := b.WorkstreamPageGet(appwire.WorkstreamPageGetParams{ID: first.ID}) + if err != nil { + t.Fatal(err) + } + selected, err := b.WorkstreamPageGet(appwire.WorkstreamPageGetParams{ID: first.ID, Version: workstreamPtr(2)}) + if err != nil || !reflect.DeepEqual(selected, current) { + t.Fatalf("current version: %+v, want %+v: %v", selected, current, err) + } + old, err := b.WorkstreamPageGet(appwire.WorkstreamPageGetParams{ID: first.ID, Version: workstreamPtr(1), Markdown: workstreamPtr(true)}) + if err != nil || old.Page.Title != "Before" || string(old.Page.Doc) != string(doc) || old.Page.Version != 1 || old.Page.UpdatedBy != "agent" || !old.Page.UpdatedAt.Equal(first.UpdatedAt) || old.Page.UpdatedFrom == nil || old.Page.UpdatedFrom.ID != "old-source" || !old.Page.ReadByJobs || old.Page.Markdown == nil || *old.Page.Markdown != "Earlier\n" { + t.Fatalf("historical page: %+v %v", old, err) + } + _, err = b.WorkstreamPageGet(appwire.WorkstreamPageGetParams{ID: first.ID, Version: workstreamPtr(99)}) + workstreamCheckError(t, err, appwire.CodeNotFound) + for _, version := range []int{0, -1} { + _, err := b.WorkstreamPageGet(appwire.WorkstreamPageGetParams{ID: first.ID, Version: &version}) + workstreamCheckError(t, err, appwire.CodeBadParams) + } + if len(workstreamEvents(rec)) != before { + t.Fatal("version reads emitted changes") + } + if _, err := b.WorkstreamPageDelete(appwire.WorkstreamPageDeleteParams{ID: first.ID}); err != nil { + t.Fatal(err) + } + old, err = b.WorkstreamPageGet(appwire.WorkstreamPageGetParams{ID: first.ID, Version: workstreamPtr(1)}) + if err != nil || old.Page.DeletedAt.IsZero() || old.Page.Markdown != nil || old.Page.Title != "Before" { + t.Fatalf("deleted historical page: %+v %v", old, err) + } +} + +// WP-F1 (b): list presence, deleted write refusals, restore parameter dispatch +// and the committed pages event are visible through the backend handlers. +func TestAppWorkstreamDeletedPageHandlers(t *testing.T) { + b, rec, _, _ := workstreamBackend(t) + w := workstreamCreateForTest(t, b, "Deleted handler") + list := func(include *bool) map[string]json.RawMessage { + t.Helper() + result, err := b.WorkstreamPageList(appwire.WorkstreamPageListParams{WorkstreamID: w.ID, IncludeDeleted: include}) + if err != nil { + t.Fatal(err) + } + raw, err := json.Marshal(result) + if err != nil { + t.Fatal(err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(raw, &fields); err != nil { + t.Fatal(err) + } + return fields + } + for _, include := range []*bool{nil, workstreamPtr(false)} { + if _, present := list(include)["deleted"]; present { + t.Fatal("deleted present without include_deleted=true") + } + } + if raw := string(list(workstreamPtr(true))["deleted"]); raw != "[]" { + t.Fatalf("empty deleted list = %s", raw) + } + made, err := b.WorkstreamPageCreate(appwire.WorkstreamPageCreateParams{WorkstreamID: w.ID, Title: "Spec"}) + if err != nil { + t.Fatal(err) + } + id := made.Page.ID + _, err = b.WorkstreamPageRestore(appwire.WorkstreamPageRestoreParams{ID: id}) + workstreamCheckError(t, err, appwire.CodeBadParams) + if _, err := b.WorkstreamPageDelete(appwire.WorkstreamPageDeleteParams{ID: id}); err != nil { + t.Fatal(err) + } + fields := list(workstreamPtr(true)) + var nodes []appwire.WorkstreamPageNode + if err := json.Unmarshal(fields["deleted"], &nodes); err != nil || len(nodes) != 1 || nodes[0].ID != id || nodes[0].DeletedAt.IsZero() { + t.Fatalf("deleted list: %s %v", fields["deleted"], err) + } + for name, call := range map[string]func() error{ + "save": func() error { + _, err := b.WorkstreamPageSave(appwire.WorkstreamPageSaveParams{ID: id, BaseVersion: 1, Doc: made.Page.Doc}) + return err + }, + "update": func() error { _, err := b.WorkstreamPageUpdate(appwire.WorkstreamPageUpdateParams{ID: id}); return err }, + "move": func() error { _, err := b.WorkstreamPageMove(appwire.WorkstreamPageMoveParams{ID: id}); return err }, + "delete": func() error { _, err := b.WorkstreamPageDelete(appwire.WorkstreamPageDeleteParams{ID: id}); return err }, + "history": func() error { + _, err := b.WorkstreamPageHistory(appwire.WorkstreamPageHistoryParams{ID: id}) + return err + }, + "create parent": func() error { + _, err := b.WorkstreamPageCreate(appwire.WorkstreamPageCreateParams{WorkstreamID: w.ID, ParentID: &id, Title: "No"}) + return err + }, + "create after": func() error { + _, err := b.WorkstreamPageCreate(appwire.WorkstreamPageCreateParams{WorkstreamID: w.ID, AfterID: &id, Title: "No"}) + return err + }, + } { + t.Run(name, func(t *testing.T) { workstreamCheckError(t, call(), appwire.CodeNotFound) }) + } + for _, p := range []appwire.WorkstreamPageRestoreParams{ + {ID: id, Version: workstreamPtr(1)}, {ID: id, BaseVersion: workstreamPtr(1)}, + {ID: id, Version: workstreamPtr(1), BaseVersion: workstreamPtr(1)}, + } { + _, err := b.WorkstreamPageRestore(p) + workstreamCheckError(t, err, appwire.CodeBadParams) + } + _, err = b.WorkstreamPageRestore(appwire.WorkstreamPageRestoreParams{ID: "missing"}) + workstreamCheckError(t, err, appwire.CodeNotFound) + before := len(workstreamEvents(rec)) + restored, err := b.WorkstreamPageRestore(appwire.WorkstreamPageRestoreParams{ID: id}) + if err != nil || restored.Page.Version != 1 || !restored.Page.DeletedAt.IsZero() || restored.Page.UpdatedBy != "you" || restored.Page.UpdatedFrom != nil { + t.Fatalf("undelete: %+v %v", restored, err) + } + events := workstreamEvents(rec) + if len(events) != before+1 || events[before].ID != w.ID || !reflect.DeepEqual(events[before].Parts, []string{"workstream", "pages"}) { + t.Fatalf("undelete event: %+v", events[before:]) + } + history, err := b.WorkstreamPageHistory(appwire.WorkstreamPageHistoryParams{ID: id}) + if err != nil || len(history.Versions) != 1 { + t.Fatalf("undelete added history: %+v %v", history, err) + } + if raw := string(list(workstreamPtr(true))["deleted"]); raw != "[]" { + t.Fatalf("deleted list after restore = %s", raw) + } +} diff --git a/cmd/codeaf/appworkstreams_test.go b/cmd/codeaf/appworkstreams_test.go index 7351dc9d41..2e8baf8a64 100644 --- a/cmd/codeaf/appworkstreams_test.go +++ b/cmd/codeaf/appworkstreams_test.go @@ -464,7 +464,7 @@ func TestAppWorkstreamPageVersions(t *testing.T) { if len(workstreamEvents(rec)) != before { t.Fatal("stale save or reads emitted") } - restored, err := b.WorkstreamPageRestore(appwire.WorkstreamPageRestoreParams{ID: id, Version: 1, BaseVersion: 2}) + restored, err := b.WorkstreamPageRestore(appwire.WorkstreamPageRestoreParams{ID: id, Version: workstreamPtr(1), BaseVersion: workstreamPtr(2)}) if err != nil || restored.Page.Version != 3 || string(restored.Page.Doc) != `{"type":"doc","content":[]}` { t.Fatalf("restore %+v, %v", restored, err) } @@ -781,7 +781,7 @@ func TestAppWorkstreamEveryWriteEmitsOnce(t *testing.T) { return err }}, {"page.restore", []string{"workstream", "pages"}, false, func(f *fixture) error { - _, err := f.b.WorkstreamPageRestore(appwire.WorkstreamPageRestoreParams{ID: f.page, Version: 1, BaseVersion: f.base}) + _, err := f.b.WorkstreamPageRestore(appwire.WorkstreamPageRestoreParams{ID: f.page, Version: workstreamPtr(1), BaseVersion: workstreamPtr(f.base)}) return err }}, {"record.propose", []string{"workstream", "records"}, false, func(f *fixture) error { diff --git a/cmd/codeaf/testdata/workstreams-wire/contract.json b/cmd/codeaf/testdata/workstreams-wire/contract.json index df9349aa4f..6dd50cef8e 100644 --- a/cmd/codeaf/testdata/workstreams-wire/contract.json +++ b/cmd/codeaf/testdata/workstreams-wire/contract.json @@ -443,6 +443,10 @@ "updated_from": { "type": "WorkstreamSource", "required": false + }, + "deleted_at": { + "type": "string (RFC 3339)", + "required": false } }, "WorkstreamPage": { @@ -490,6 +494,10 @@ "type": "WorkstreamSource", "required": false }, + "deleted_at": { + "type": "string (RFC 3339)", + "required": false + }, "doc": { "type": "object", "required": true @@ -836,12 +844,20 @@ "workstream_id": { "type": "string", "required": true + }, + "include_deleted": { + "type": "boolean", + "required": false } }, "WorkstreamPageListResult": { "pages": { "type": "WorkstreamPageNode[]", "required": true + }, + "deleted": { + "type": "WorkstreamPageNode[]", + "required": false } }, "WorkstreamPageGetParams": { @@ -852,6 +868,10 @@ "markdown": { "type": "boolean", "required": false + }, + "version": { + "type": "integer", + "required": false } }, "WorkstreamPageGetResult": { @@ -995,11 +1015,11 @@ }, "version": { "type": "integer", - "required": true + "required": false }, "base_version": { "type": "integer", - "required": true + "required": false } }, "WorkstreamPageRestoreResult": { diff --git a/cmd/codeaf/testdata/workstreams-wire/page.get.json b/cmd/codeaf/testdata/workstreams-wire/page.get.json index fdcf420091..85cc68240a 100644 --- a/cmd/codeaf/testdata/workstreams-wire/page.get.json +++ b/cmd/codeaf/testdata/workstreams-wire/page.get.json @@ -13,6 +13,10 @@ { "id": "pg_31d0", "markdown": true + }, + { + "id": "pg_31d0", + "version": 1 } ], "result": [ @@ -33,6 +37,23 @@ }, "markdown": "" } + }, + { + "page": { + "id": "pg_31d0", + "workstream_id": "ws_8f2c1a", + "title": "Overview", + "owner": "you", + "read_by_jobs": false, + "position": 0, + "version": 1, + "updated_at": "2026-11-02T14:05:00Z", + "updated_by": "you", + "doc": { + "type": "doc", + "content": [] + } + } } ] } diff --git a/cmd/codeaf/testdata/workstreams-wire/page.list.json b/cmd/codeaf/testdata/workstreams-wire/page.list.json index 37550b606a..6238f1105b 100644 --- a/cmd/codeaf/testdata/workstreams-wire/page.list.json +++ b/cmd/codeaf/testdata/workstreams-wire/page.list.json @@ -12,6 +12,10 @@ "params": [ { "workstream_id": "ws_8f2c1a" + }, + { + "workstream_id": "ws_8f2c1a", + "include_deleted": true } ], "result": [ @@ -29,6 +33,36 @@ "updated_by": "you" } ] + }, + { + "pages": [ + { + "id": "pg_31d0", + "workstream_id": "ws_8f2c1a", + "title": "Overview", + "owner": "you", + "read_by_jobs": false, + "position": 0, + "version": 1, + "updated_at": "2026-11-02T14:05:00Z", + "updated_by": "you" + } + ], + "deleted": [ + { + "id": "pg_33f2", + "workstream_id": "ws_8f2c1a", + "parent_id": "pg_31d0", + "title": "Old metering notes", + "owner": "you", + "read_by_jobs": false, + "position": 0, + "version": 2, + "updated_at": "2026-11-02T15:10:00Z", + "updated_by": "you", + "deleted_at": "2026-11-02T15:20:00Z" + } + ] } ] } diff --git a/cmd/codeaf/testdata/workstreams-wire/page.restore.json b/cmd/codeaf/testdata/workstreams-wire/page.restore.json index 9cdf549c70..37da1c2804 100644 --- a/cmd/codeaf/testdata/workstreams-wire/page.restore.json +++ b/cmd/codeaf/testdata/workstreams-wire/page.restore.json @@ -16,6 +16,9 @@ "id": "pg_31d0", "version": 1, "base_version": 2 + }, + { + "id": "pg_33f2" } ], "result": [ @@ -35,6 +38,24 @@ "content": [] } } + }, + { + "page": { + "id": "pg_33f2", + "workstream_id": "ws_8f2c1a", + "parent_id": "pg_31d0", + "title": "Old metering notes", + "owner": "you", + "read_by_jobs": false, + "position": 0, + "version": 2, + "updated_at": "2026-11-02T15:30:00Z", + "updated_by": "you", + "doc": { + "type": "doc", + "content": [] + } + } } ] } diff --git a/docs/changes/unreleased/2290-app-workstream-page-versions-restore.md b/docs/changes/unreleased/2290-app-workstream-page-versions-restore.md new file mode 100644 index 0000000000..7a5cda50b7 --- /dev/null +++ b/docs/changes/unreleased/2290-app-workstream-page-versions-restore.md @@ -0,0 +1,24 @@ +--- +kind: added +title: Workstream pages support version reads, restorable deletion and title-based read defaults +pr: 2290 +surface: [app] +invalidates: + - page.delete used to destroy pages and their history; it now marks pages deleted and restorable while retaining saved versions. + - page.create used to default read_by_jobs to false for every title; it now defaults to true for Spec and Decisions unless explicitly overridden. +--- + +`page.get` accepts a saved version and renders that version's document when +markdown is requested. Deleted pages remain readable, but leave the live tree +and refuse edits and history reads. `page.list` with `include_deleted: true` +adds deletion roots, newest first, including an empty list when none remain. + +`page.restore` with only an id restores a deleted page and the descendants +removed by the same deletion, preserving saved versions, nesting and order. +It appends under its live old parent or at the root. Restoring an old version +of a live page still requires both version parameters and creates a new version. + +Creation compares trimmed titles without regard to case when choosing Read by +jobs. Explicit choices win and renames keep the existing setting; the initial +Overview remains off. Deleted pages remain stored until their collection is +deleted. diff --git a/internal/appwire/workstreams.go b/internal/appwire/workstreams.go index 6a7191bb17..498cb655a6 100644 --- a/internal/appwire/workstreams.go +++ b/internal/appwire/workstreams.go @@ -146,6 +146,7 @@ type WorkstreamPageNode struct { UpdatedAt time.Time `json:"updated_at"` UpdatedBy string `json:"updated_by" enum:"you,agent"` UpdatedFrom *WorkstreamSource `json:"updated_from,omitempty"` + DeletedAt time.Time `json:"deleted_at,omitzero"` } type WorkstreamPage struct { @@ -160,6 +161,7 @@ type WorkstreamPage struct { UpdatedAt time.Time `json:"updated_at"` UpdatedBy string `json:"updated_by" enum:"you,agent"` UpdatedFrom *WorkstreamSource `json:"updated_from,omitempty"` + DeletedAt time.Time `json:"deleted_at,omitzero"` Doc json.RawMessage `json:"doc"` Markdown *string `json:"markdown,omitempty"` } @@ -305,16 +307,19 @@ type WorkstreamPlanSetResult struct { } type WorkstreamPageListParams struct { - WorkstreamID string `json:"workstream_id"` + WorkstreamID string `json:"workstream_id"` + IncludeDeleted *bool `json:"include_deleted,omitempty"` } type WorkstreamPageListResult struct { - Pages []WorkstreamPageNode `json:"pages"` + Pages []WorkstreamPageNode `json:"pages"` + Deleted []WorkstreamPageNode `json:"deleted,omitzero"` } type WorkstreamPageGetParams struct { ID string `json:"id"` Markdown *bool `json:"markdown,omitempty"` + Version *int `json:"version,omitempty"` } type WorkstreamPageGetResult struct { @@ -385,8 +390,8 @@ type WorkstreamPageHistoryResult struct { type WorkstreamPageRestoreParams struct { ID string `json:"id"` - Version int `json:"version"` - BaseVersion int `json:"base_version"` + Version *int `json:"version,omitempty"` + BaseVersion *int `json:"base_version,omitempty"` } type WorkstreamPageRestoreResult struct { diff --git a/internal/appwire/workstreams_check.go b/internal/appwire/workstreams_check.go index c549a4312e..c9418fdda0 100644 --- a/internal/appwire/workstreams_check.go +++ b/internal/appwire/workstreams_check.go @@ -197,12 +197,17 @@ func workstreamSource(p *WorkstreamSource) error { return p.Check() } -func (p WorkstreamListParams) Check() error { return nil } -func (p WorkstreamGetParams) Check() error { return workstreamID(p.ID) } -func (p WorkstreamReopenParams) Check() error { return workstreamID(p.ID) } -func (p WorkstreamPlanGetParams) Check() error { return workstreamID(p.ID) } -func (p WorkstreamPageListParams) Check() error { return workstreamID(p.WorkstreamID) } -func (p WorkstreamPageGetParams) Check() error { return workstreamID(p.ID) } +func (p WorkstreamListParams) Check() error { return nil } +func (p WorkstreamGetParams) Check() error { return workstreamID(p.ID) } +func (p WorkstreamReopenParams) Check() error { return workstreamID(p.ID) } +func (p WorkstreamPlanGetParams) Check() error { return workstreamID(p.ID) } +func (p WorkstreamPageListParams) Check() error { return workstreamID(p.WorkstreamID) } +func (p WorkstreamPageGetParams) Check() error { + if p.Version != nil && *p.Version < 1 { + return Errorf(CodeBadParams, "version must be positive") + } + return workstreamID(p.ID) +} func (p WorkstreamPageDeleteParams) Check() error { return workstreamID(p.ID) } func (p WorkstreamRecordKeepParams) Check() error { return workstreamID(p.ID) } func (p WorkstreamRecordDropParams) Check() error { return workstreamID(p.ID) } @@ -329,7 +334,10 @@ func (p WorkstreamPageHistoryParams) Check() error { return workstreamChecks(workstreamID(p.ID), err) } func (p WorkstreamPageRestoreParams) Check() error { - if p.Version < 1 || p.BaseVersion < 0 { + if (p.Version == nil) != (p.BaseVersion == nil) { + return Errorf(CodeBadParams, "version and base_version must be supplied together") + } + if p.Version != nil && (*p.Version < 1 || *p.BaseVersion < 0) { return Errorf(CodeBadParams, "restore requires positive version and nonnegative base_version") } return workstreamID(p.ID) diff --git a/internal/appwire/workstreams_check_test.go b/internal/appwire/workstreams_check_test.go index 117f128e14..2da32a2a7c 100644 --- a/internal/appwire/workstreams_check_test.go +++ b/internal/appwire/workstreams_check_test.go @@ -34,6 +34,12 @@ func TestWorkstreamBoundsBeforeDelegation(t *testing.T) { {"page.history", `{"id":"x","limit":0}`, CodeBadParams}, {"page.history", `{"id":"x","limit":201}`, CodeBadParams}, {"page.restore", `{"id":"x","version":0,"base_version":1}`, CodeBadParams}, + // WP-F1 (a), (b): version reads are positive and restore versions are paired. + {"page.get", `{"id":"x","version":0}`, CodeBadParams}, + {"page.get", `{"id":"x","version":-1}`, CodeBadParams}, + {"page.restore", `{"id":"x","version":1}`, CodeBadParams}, + {"page.restore", `{"id":"x","base_version":1}`, CodeBadParams}, + {"page.restore", `{"id":"x","version":1,"base_version":-1}`, CodeBadParams}, {"record.list", `{"workstream_id":"x","state":"active"}`, CodeBadParams}, {"record.propose", `{"workstream_id":"x","kind":"decision","text":""}`, CodeBadParams}, {"record.propose", `{"workstream_id":"x","kind":"decision","text":"` + strings.Repeat("t", 2001) + `"}`, CodeBadParams}, @@ -60,6 +66,10 @@ func TestWorkstreamBoundsPreserveExplicitZeroAndFalse(t *testing.T) { "workstream.plan_set": `{"id":"x","base_version":0,"milestones":[],"waves":[]}`, "workstream.update": `{"id":"x","patch":{"why":"","budget_usd":0,"wake":{"morning":false,"related_issues":false}}}`, "page.create": `{"workstream_id":"x","title":"x","read_by_jobs":false}`, + // WP-F1 (a), (b): additive params, including id-only restore, reach dispatch. + "page.get": `{"id":"x","version":1}`, + "page.list": `{"workstream_id":"x","include_deleted":true}`, + "page.restore": `{"id":"x"}`, } { _, err := methods[method].call(fake, json.RawMessage(params)) if err != ErrWorkstreamsNotYet { diff --git a/internal/workspace/schema3_test.go b/internal/workspace/schema3_test.go index f61626e0d5..ac38029234 100644 --- a/internal/workspace/schema3_test.go +++ b/internal/workspace/schema3_test.go @@ -210,3 +210,39 @@ DELETE FROM memberships WHERE ref_id='gone';`); err != nil { }) } } + +// WP-F1 (b), Storage: schema 3 is amended in place; either missing deletion +// column identifies an obsolete schema-3 file and must fail at Open. +func TestWorkstreamSchemaThreeRequiresDeletionColumns(t *testing.T) { + for _, column := range []string{"deleted_at", "deleted_root"} { + t.Run(column, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "obsolete.db") + s := openTestStore(t, path) + createTestCollection(t, s, "Schema three") + if userVersion(t, s) != 3 { + t.Fatal("fixture is not schema 3") + } + if _, err := s.db.Exec("ALTER TABLE workstream_pages DROP COLUMN " + column); err != nil { + t.Fatal(err) + } + if err := s.Close(); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + opened, err := Open(path) + if opened != nil { + _ = opened.Close() + } + if err == nil || !strings.Contains(err.Error(), column) { + t.Fatalf("obsolete schema 3 opened without naming %s: %v", column, err) + } + after, err := os.ReadFile(path) + if err != nil || string(after) != string(before) { + t.Fatalf("refusal changed obsolete schema 3: %v", err) + } + }) + } +} diff --git a/internal/workspace/workstreams.go b/internal/workspace/workstreams.go index 46de35b2f5..3a4619608d 100644 --- a/internal/workspace/workstreams.go +++ b/internal/workspace/workstreams.go @@ -180,7 +180,8 @@ CREATE TABLE workstream_pages ( read_by_jobs INTEGER NOT NULL DEFAULT 0, position INTEGER NOT NULL CHECK(position>=0), version INTEGER NOT NULL CHECK(version>0), updated_at INTEGER NOT NULL, updated_by TEXT NOT NULL CHECK(updated_by IN ('you','agent')), updated_from TEXT, - doc TEXT NOT NULL, overview INTEGER NOT NULL DEFAULT 0 CHECK(overview IN (0,1)) + doc TEXT NOT NULL, overview INTEGER NOT NULL DEFAULT 0 CHECK(overview IN (0,1)), + deleted_at INTEGER, deleted_root TEXT ); CREATE INDEX workstream_page_tree ON workstream_pages(workstream_id,parent_id,position); CREATE UNIQUE INDEX workstream_overview ON workstream_pages(workstream_id) WHERE overview=1; @@ -225,7 +226,7 @@ CREATE TABLE workstream_plans ( var workstreamSchemaReads = []string{ "SELECT id,why,origin_chat_id,agent_id,autonomy,wake_morning,wake_related_issues,budget_usd,created_at,updated_at,started_at,done_at,how_it_went,client_id FROM workstreams LIMIT 0", "SELECT member_seq,workstream_id,factory,job FROM workstream_jobs LIMIT 0", - "SELECT id,workstream_id,parent_id,title,owner,read_by_jobs,position,version,updated_at,updated_by,updated_from,doc,overview FROM workstream_pages LIMIT 0", + "SELECT id,workstream_id,parent_id,title,owner,read_by_jobs,position,version,updated_at,updated_by,updated_from,doc,overview,deleted_at,deleted_root FROM workstream_pages LIMIT 0", "SELECT page_id,version,saved_at,saved_by,source,title,doc,bytes FROM workstream_page_versions LIMIT 0", "SELECT id,workstream_id,kind,text,state,actor,source,created_at,decided_at,supersedes,superseded_by FROM workstream_records LIMIT 0", "SELECT id,workstream_id,scope,factory,job,wave,milestone_id,cap_usd,ends_at,state,end_reason,granted_at,ended_at FROM workstream_goaheads LIMIT 0", diff --git a/internal/workstream/pages.go b/internal/workstream/pages.go index cc3a238bb2..c856f975ef 100644 --- a/internal/workstream/pages.go +++ b/internal/workstream/pages.go @@ -4,53 +4,89 @@ import ( "context" "database/sql" "encoding/json" + "errors" "strings" "unicode/utf8" "github.com/Agent-Field/codeaf/internal/workstream/pagedoc" ) -const pageSelect = `SELECT id,workstream_id,coalesce(parent_id,''),title,owner,read_by_jobs,position,version,updated_at,updated_by,updated_from,doc,overview FROM workstream_pages WHERE id=?` +const pageSelect = `SELECT id,workstream_id,coalesce(parent_id,''),title,owner,read_by_jobs,position,version,updated_at,updated_by,updated_from,doc,overview,deleted_at FROM workstream_pages WHERE id=?` + +// ReadByJobsDefault chooses the creation default; later title changes do not +// alter an existing page's explicit Read by jobs setting. +func ReadByJobsDefault(title string) bool { + title = strings.TrimSpace(title) + return strings.EqualFold(title, "Spec") || strings.EqualFold(title, "Decisions") +} func pageIn(ctx context.Context, tx *sql.Tx, id string) (Page, bool, error) { var p Page var at int64 var source sql.NullString + var deletedAt sql.NullInt64 var overview bool var doc string - err := tx.QueryRowContext(ctx, pageSelect, id).Scan(&p.ID, &p.WorkstreamID, &p.ParentID, &p.Title, &p.Owner, &p.ReadByJobs, &p.Position, &p.Version, &at, &p.UpdatedBy, &source, &doc, &overview) + err := tx.QueryRowContext(ctx, pageSelect, id).Scan(&p.ID, &p.WorkstreamID, &p.ParentID, &p.Title, &p.Owner, &p.ReadByJobs, &p.Position, &p.Version, &at, &p.UpdatedBy, &source, &doc, &overview, &deletedAt) if err != nil { return p, false, found(err) } p.UpdatedAt = instant(at) + if deletedAt.Valid { + p.DeletedAt = instant(deletedAt.Int64) + } p.Doc = json.RawMessage(doc) p.UpdatedFrom, err = readSource(source) return p, overview, err } -func pageNodesIn(ctx context.Context, tx *sql.Tx, id string) ([]PageNode, error) { - rows, err := tx.QueryContext(ctx, `SELECT id,workstream_id,coalesce(parent_id,''),title,owner,read_by_jobs,position,version,updated_at,updated_by,updated_from FROM workstream_pages WHERE workstream_id=? ORDER BY position,id`, id) - if err != nil { - return nil, err +func livePageIn(ctx context.Context, tx *sql.Tx, id string) (Page, bool, error) { + p, overview, err := pageIn(ctx, tx, id) + if err == nil && !p.DeletedAt.IsZero() { + err = ErrNotFound } + return p, overview, err +} + +const pageNodeColumns = `id,workstream_id,coalesce(parent_id,''),title,owner,read_by_jobs,position,version,updated_at,updated_by,updated_from,deleted_at` + +func readPageNodes(rows *sql.Rows) ([]PageNode, error) { defer rows.Close() - siblings := map[string][]PageNode{} + out := make([]PageNode, 0) for rows.Next() { var p PageNode var at int64 var source sql.NullString - if err := rows.Scan(&p.ID, &p.WorkstreamID, &p.ParentID, &p.Title, &p.Owner, &p.ReadByJobs, &p.Position, &p.Version, &at, &p.UpdatedBy, &source); err != nil { + var deletedAt sql.NullInt64 + if err := rows.Scan(&p.ID, &p.WorkstreamID, &p.ParentID, &p.Title, &p.Owner, &p.ReadByJobs, &p.Position, &p.Version, &at, &p.UpdatedBy, &source, &deletedAt); err != nil { return nil, err } p.UpdatedAt = instant(at) + if deletedAt.Valid { + p.DeletedAt = instant(deletedAt.Int64) + } + var err error p.UpdatedFrom, err = readSource(source) if err != nil { return nil, err } - siblings[p.ParentID] = append(siblings[p.ParentID], p) + out = append(out, p) } - if err := rows.Err(); err != nil { + return out, rows.Err() +} + +func pageNodesIn(ctx context.Context, tx *sql.Tx, id string) ([]PageNode, error) { + rows, err := tx.QueryContext(ctx, `SELECT `+pageNodeColumns+` FROM workstream_pages WHERE workstream_id=? AND deleted_at IS NULL ORDER BY position,id`, id) + if err != nil { return nil, err } + nodes, err := readPageNodes(rows) + if err != nil { + return nil, err + } + siblings := map[string][]PageNode{} + for _, p := range nodes { + siblings[p.ParentID] = append(siblings[p.ParentID], p) + } out := make([]PageNode, 0) seen := map[string]bool{} // An explicit stack keeps exports safe for a very deep tree. @@ -89,7 +125,40 @@ func (s *Store) Pages(ctx context.Context, id string) ([]PageNode, error) { }) return out, err } + +// DeletedPages lists delete roots; descendants remain readable but are not +// separate entries unless they were deleted by their own delete operation. +func (s *Store) DeletedPages(ctx context.Context, id string) ([]PageNode, error) { + out := make([]PageNode, 0) + err := s.workspace.Read(ctx, func(tx *sql.Tx, v int) error { + if v < 3 { + return ErrNotFound + } + if _, err := getIn(ctx, tx, id); err != nil { + return err + } + rows, err := tx.QueryContext(ctx, `SELECT `+pageNodeColumns+` FROM workstream_pages WHERE workstream_id=? AND deleted_at IS NOT NULL AND deleted_root=id ORDER BY deleted_at DESC,id`, id) + if err != nil { + return err + } + out, err = readPageNodes(rows) + return err + }) + return out, err +} func (s *Store) Page(ctx context.Context, id string, markdown bool) (Page, error) { + return s.page(ctx, id, 0, markdown) +} + +// PageAtVersion reads saved content while preserving the page's current tree metadata. +func (s *Store) PageAtVersion(ctx context.Context, id string, version int, markdown bool) (Page, error) { + if version < 1 { + return Page{}, invalid("version must be positive") + } + return s.page(ctx, id, version, markdown) +} + +func (s *Store) page(ctx context.Context, id string, version int, markdown bool) (Page, error) { var p Page err := s.workspace.Read(ctx, func(tx *sql.Tx, v int) error { if v < 3 { @@ -97,6 +166,19 @@ func (s *Store) Page(ctx context.Context, id string, markdown bool) (Page, error } var err error p, _, err = pageIn(ctx, tx, id) + if err != nil || version == 0 || version == p.Version { + return err + } + var at int64 + var source sql.NullString + var doc string + err = tx.QueryRowContext(ctx, `SELECT title,doc,version,saved_at,saved_by,source FROM workstream_page_versions WHERE page_id=? AND version=?`, id, version). + Scan(&p.Title, &doc, &p.Version, &at, &p.UpdatedBy, &source) + if err != nil { + return found(err) + } + p.Doc, p.UpdatedAt = json.RawMessage(doc), instant(at) + p.UpdatedFrom, err = readSource(source) return err }) if err == nil && markdown { @@ -118,7 +200,7 @@ func checkDoc(doc json.RawMessage) error { } func saveVersion(ctx context.Context, tx *sql.Tx, id string) error { _, err := tx.ExecContext(ctx, `INSERT INTO workstream_page_versions(page_id,version,saved_at,saved_by,source,title,doc,bytes) - SELECT id,version,updated_at,updated_by,updated_from,title,doc,length(CAST(doc AS BLOB)) FROM workstream_pages WHERE id=?`, id) + SELECT id,version,updated_at,updated_by,updated_from,title,doc,length(CAST(doc AS BLOB)) FROM workstream_pages WHERE id=? AND deleted_at IS NULL`, id) return err } func (s *Store) SavePage(ctx context.Context, id string, base int, doc json.RawMessage, who Attribution) (Page, Change, error) { @@ -152,6 +234,12 @@ func (s *Store) savePage(ctx context.Context, id string, base int, doc json.RawM if err != nil { return err } + if !p.DeletedAt.IsZero() { + if restore > 0 { + return invalid("deleted page restore requires only id") + } + return ErrNotFound + } if p.Version != base { return &VersionConflict{ID: id, Version: p.Version, UpdatedAt: p.UpdatedAt, UpdatedBy: p.UpdatedBy} } @@ -198,7 +286,7 @@ func placePage(ctx context.Context, tx *sql.Tx, p Page, parent, after string, la } seen[id] = true var workstreamID, next string - err := tx.QueryRowContext(ctx, "SELECT workstream_id,coalesce(parent_id,'') FROM workstream_pages WHERE id=?", id).Scan(&workstreamID, &next) + err := tx.QueryRowContext(ctx, "SELECT workstream_id,coalesce(parent_id,'') FROM workstream_pages WHERE id=? AND deleted_at IS NULL", id).Scan(&workstreamID, &next) if err != nil { return false, found(err) } @@ -210,7 +298,7 @@ func placePage(ctx context.Context, tx *sql.Tx, p Page, parent, after string, la } if after != "" { var workstreamID, siblingParent string - err := tx.QueryRowContext(ctx, "SELECT workstream_id,coalesce(parent_id,'') FROM workstream_pages WHERE id=?", after).Scan(&workstreamID, &siblingParent) + err := tx.QueryRowContext(ctx, "SELECT workstream_id,coalesce(parent_id,'') FROM workstream_pages WHERE id=? AND deleted_at IS NULL", after).Scan(&workstreamID, &siblingParent) if err != nil { return false, found(err) } @@ -315,7 +403,7 @@ func (s *Store) CreatePage(ctx context.Context, p PageCreate) (Page, Change, err if _, err := tx.ExecContext(ctx, `INSERT INTO workstream_pages(id,workstream_id,title,owner,read_by_jobs,position,version,updated_at,updated_by,updated_from,doc) VALUES (?,?,?,?,?,2147483647,1,?,?,?,?)`, id, p.WorkstreamID, p.Title, owner, p.ReadByJobs, at, by, source, string(p.Doc)); err != nil { return err } - page, _, err := pageIn(ctx, tx, id) + page, _, err := livePageIn(ctx, tx, id) if err != nil { return err } @@ -359,7 +447,7 @@ func (s *Store) UpdatePage(ctx context.Context, id string, p PagePatch) (PageNod var out PageNode var c Change err = s.workspace.Write(ctx, func(tx *sql.Tx) error { - page, _, err := pageIn(ctx, tx, id) + page, _, err := livePageIn(ctx, tx, id) if err != nil { return err } @@ -398,7 +486,7 @@ func (s *Store) MovePage(ctx context.Context, id, parent, after string) (PageNod var out PageNode var c Change err := s.workspace.Write(ctx, func(tx *sql.Tx) error { - p, overview, err := pageIn(ctx, tx, id) + p, overview, err := livePageIn(ctx, tx, id) if err != nil { return err } @@ -429,7 +517,7 @@ func (s *Store) DeletePage(ctx context.Context, id string) ([]string, Change, er deleted := []string{} var c Change err := s.workspace.Write(ctx, func(tx *sql.Tx) error { - p, overview, err := pageIn(ctx, tx, id) + p, overview, err := livePageIn(ctx, tx, id) if err != nil { return err } @@ -447,10 +535,9 @@ func (s *Store) DeletePage(ctx context.Context, id string) ([]string, Change, er deleted = append(deleted, n.ID) } } - // Delete leaves before parents to avoid SQLite's trigger depth limit on a - // deep subtree; the cascade remains a backstop for ordinary SQL deletions. - for i := len(deleted) - 1; i >= 0; i-- { - if _, err := tx.ExecContext(ctx, "DELETE FROM workstream_pages WHERE id=?", deleted[i]); err != nil { + at := s.now().UnixMilli() + for _, deletedID := range deleted { + if _, err := tx.ExecContext(ctx, "UPDATE workstream_pages SET deleted_at=?,deleted_root=? WHERE id=?", at, id, deletedID); err != nil { return err } } @@ -463,7 +550,7 @@ func (s *Store) DeletePage(ctx context.Context, id string) ([]string, Change, er i++ } } - if err := touch(ctx, tx, p.WorkstreamID, s.now().UnixMilli()); err != nil { + if err := touch(ctx, tx, p.WorkstreamID, at); err != nil { return err } c = changed(p.WorkstreamID, PartPages) @@ -474,6 +561,79 @@ func (s *Store) DeletePage(ctx context.Context, id string) ([]string, Change, er } return deleted, c, nil } + +// UndeletePage restores only descendants removed by the same delete. Like a +// move, it records the person as the actor and does not create a saved version. +func (s *Store) UndeletePage(ctx context.Context, id string) (Page, Change, error) { + var out Page + var c Change + err := s.workspace.Write(ctx, func(tx *sql.Tx) error { + p, _, err := pageIn(ctx, tx, id) + if err != nil { + return err + } + if p.DeletedAt.IsZero() { + return invalid("live page restore requires version and base_version") + } + var root string + if err := tx.QueryRowContext(ctx, "SELECT deleted_root FROM workstream_pages WHERE id=?", id).Scan(&root); err != nil { + return found(err) + } + rows, err := tx.QueryContext(ctx, `SELECT `+pageNodeColumns+` FROM workstream_pages WHERE workstream_id=? AND deleted_root=? AND deleted_at=? ORDER BY position,id`, p.WorkstreamID, root, p.DeletedAt.UnixMilli()) + if err != nil { + return err + } + nodes, err := readPageNodes(rows) + if err != nil { + return err + } + children := map[string][]string{} + for _, n := range nodes { + children[n.ParentID] = append(children[n.ParentID], n.ID) + } + parent := p.ParentID + if parent != "" { + if _, _, err := livePageIn(ctx, tx, parent); errors.Is(err, ErrNotFound) { + parent = "" + } else if err != nil { + return err + } + } + at := s.now().UnixMilli() + stack := []string{id} + seen := map[string]bool{} + for len(stack) > 0 { + last := len(stack) - 1 + restoredID := stack[last] + stack = stack[:last] + if seen[restoredID] { + continue + } + seen[restoredID] = true + if _, err := tx.ExecContext(ctx, `UPDATE workstream_pages SET deleted_at=NULL,deleted_root=NULL,updated_at=?,updated_by='you',updated_from=NULL WHERE id=?`, at, restoredID); err != nil { + return err + } + stack = append(stack, children[restoredID]...) + } + // Force placement even when the old ordinal happens to equal the append + // index: the restored page still needs its parent and attribution written. + p.Position = -1 + if _, err := placePage(ctx, tx, p, parent, "", true, at, "you", nil); err != nil { + return err + } + if err := touch(ctx, tx, p.WorkstreamID, at); err != nil { + return err + } + out, _, err = pageIn(ctx, tx, id) + c = changed(p.WorkstreamID, PartPages) + return err + }) + if err != nil { + return Page{}, Change{}, err + } + return out, c, nil +} + func (s *Store) PageHistory(ctx context.Context, id string, limit int) ([]PageVersion, error) { if limit == 0 { limit = 50 @@ -486,7 +646,7 @@ func (s *Store) PageHistory(ctx context.Context, id string, limit int) ([]PageVe if v < 3 { return ErrNotFound } - if _, _, err := pageIn(ctx, tx, id); err != nil { + if _, _, err := livePageIn(ctx, tx, id); err != nil { return err } rows, err := tx.QueryContext(ctx, "SELECT version,saved_at,saved_by,source,title,bytes FROM workstream_page_versions WHERE page_id=? ORDER BY version DESC LIMIT ?", id, limit) diff --git a/internal/workstream/pages_test.go b/internal/workstream/pages_test.go index 595ac51678..09eb222dc6 100644 --- a/internal/workstream/pages_test.go +++ b/internal/workstream/pages_test.go @@ -2,6 +2,7 @@ package workstream import ( "context" + "database/sql" "encoding/json" "errors" "reflect" @@ -118,8 +119,8 @@ func TestPartAPageTreePlacementCyclesAndDelete(t *testing.T) { if err != nil || !reflect.DeepEqual(deleted, []string{a.ID, child.ID}) { t.Fatalf("delete: %v %v", deleted, err) } - if _, err := s.Page(ctx, child.ID, false); !errors.Is(err, ErrNotFound) { - t.Fatalf("child remains: %v", err) + if page, err := s.Page(ctx, child.ID, false); err != nil || page.DeletedAt.IsZero() { + t.Fatalf("deleted child must remain readable: %+v %v", page, err) } } @@ -215,3 +216,290 @@ func TestPartAOverviewIdentitySurvivesRenaming(t *testing.T) { t.Fatalf("Overview order: %+v %v", pages, err) } } + +// WP-F1 (a): saved content and attribution combine with current page metadata; +// selecting the current version is identical to reading without a version. +func TestWorkstreamPageGetSavedVersion(t *testing.T) { + s, _, now := testStore(t) + ctx := context.Background() + w := createStream(t, s, "Version reads") + firstDoc := json.RawMessage(`{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"First"}]}]}`) + first, _, err := s.CreatePage(ctx, PageCreate{WorkstreamID: w.ID, Title: "First title", Doc: firstDoc, + Attribution: Attribution{By: "agent", From: &Source{Kind: "chat", ID: "first-source"}}}) + if err != nil { + t.Fatal(err) + } + *now = now.Add(time.Hour) + if _, _, err := s.SavePage(ctx, first.ID, 1, json.RawMessage(emptyDoc), Attribution{}); err != nil { + t.Fatal(err) + } + parent, _, err := s.CreatePage(ctx, PageCreate{WorkstreamID: w.ID, Title: "Parent"}) + if err != nil { + t.Fatal(err) + } + if _, _, err := s.MovePage(ctx, first.ID, parent.ID, ""); err != nil { + t.Fatal(err) + } + title, owner, read := "Current title", "agent", true + if _, _, err := s.UpdatePage(ctx, first.ID, PagePatch{Title: &title, Owner: &owner, ReadByJobs: &read}); err != nil { + t.Fatal(err) + } + current, err := s.Page(ctx, first.ID, false) + if err != nil { + t.Fatal(err) + } + selected, err := s.PageAtVersion(ctx, first.ID, current.Version, false) + if err != nil || !reflect.DeepEqual(selected, current) { + t.Fatalf("current version: %+v, want %+v: %v", selected, current, err) + } + check := func(deleted bool) { + t.Helper() + old, err := s.PageAtVersion(ctx, first.ID, 1, true) + if err != nil { + t.Fatal(err) + } + want := current + want.Title, want.Doc, want.Version = first.Title, first.Doc, first.Version + want.UpdatedAt, want.UpdatedBy, want.UpdatedFrom = first.UpdatedAt, first.UpdatedBy, first.UpdatedFrom + md, err := pagedoc.Markdown(firstDoc) + if err != nil { + t.Fatal(err) + } + want.Markdown = &md + if deleted { + want.DeletedAt = *now + } + if !reflect.DeepEqual(old, want) || old.Markdown == nil || !strings.Contains(*old.Markdown, "First") { + t.Fatalf("saved version: %+v, want %+v", old, want) + } + } + check(false) + if _, err := s.PageAtVersion(ctx, first.ID, 99, false); !errors.Is(err, ErrNotFound) { + t.Fatalf("unknown version: %v", err) + } + for _, version := range []int{0, -1} { + if _, err := s.PageAtVersion(ctx, first.ID, version, false); !errors.Is(err, ErrInvalid) { + t.Fatalf("invalid version %d: %v", version, err) + } + } + if _, _, err := s.DeletePage(ctx, first.ID); err != nil { + t.Fatal(err) + } + check(true) +} + +func createWorkstreamPage(t *testing.T, s *Store, workstreamID, parent, title string) Page { + t.Helper() + p, _, err := s.CreatePage(context.Background(), PageCreate{WorkstreamID: workstreamID, ParentID: parent, Title: title}) + if err != nil { + t.Fatal(err) + } + return p +} + +// WP-F1 (b): delete groups preserve separate deletions, history, nesting and order; +// undelete appends under a live parent and changes attribution without a save. +func TestWorkstreamDeletedPageTreeRestore(t *testing.T) { + s, _, now := testStore(t) + ctx := context.Background() + w := createStream(t, s, "Restorable tree") + deleted, err := s.DeletedPages(ctx, w.ID) + if err != nil || deleted == nil || len(deleted) != 0 { + t.Fatalf("initial deleted list: %+v %v", deleted, err) + } + parent := createWorkstreamPage(t, s, w.ID, "", "Parent") + root := createWorkstreamPage(t, s, w.ID, parent.ID, "Root") + earlier := createWorkstreamPage(t, s, w.ID, root.ID, "Earlier deletion") + first := createWorkstreamPage(t, s, w.ID, root.ID, "First") + grandchild := createWorkstreamPage(t, s, w.ID, first.ID, "Grandchild") + second := createWorkstreamPage(t, s, w.ID, root.ID, "Second") + sibling := createWorkstreamPage(t, s, w.ID, parent.ID, "Sibling") + doc := json.RawMessage(`{"type":"doc","content":[{"type":"paragraph","content":[{"type":"text","text":"Keep me"}]}]}`) + if _, _, err := s.SavePage(ctx, root.ID, 1, doc, Attribution{By: "agent", From: &Source{Kind: "chat", ID: "source"}}); err != nil { + t.Fatal(err) + } + lastTitle := "Last title" + if _, _, err := s.UpdatePage(ctx, root.ID, PagePatch{Title: &lastTitle, Attribution: Attribution{By: "agent", From: &Source{Kind: "chat", ID: "source"}}}); err != nil { + t.Fatal(err) + } + if _, _, err := s.DeletePage(ctx, earlier.ID); err != nil { + t.Fatal(err) + } + *now = now.Add(time.Hour) + ids, change, err := s.DeletePage(ctx, root.ID) + if err != nil || !reflect.DeepEqual(ids, []string{root.ID, first.ID, grandchild.ID, second.ID}) || !reflect.DeepEqual(change.Parts, []ChangedPart{PartWorkstream, PartPages}) { + t.Fatalf("delete receipt: %v %+v %v", ids, change, err) + } + deleted, err = s.DeletedPages(ctx, w.ID) + if err != nil || len(deleted) != 2 || deleted[0].ID != root.ID || deleted[1].ID != earlier.ID || !deleted[0].DeletedAt.Equal(*now) || !deleted[0].DeletedAt.After(deleted[1].DeletedAt) { + t.Fatalf("delete roots: %+v %v", deleted, err) + } + nodes, err := s.Pages(ctx, w.ID) + if err != nil || len(nodes) != 3 || nodes[1].ID != parent.ID || nodes[2].ID != sibling.ID || nodes[2].Position != 0 { + t.Fatalf("live tree: %+v %v", nodes, err) + } + for _, id := range ids { + p, err := s.Page(ctx, id, true) + if err != nil || !p.DeletedAt.Equal(*now) { + t.Fatalf("deleted page %s: %+v %v", id, p, err) + } + } + for name, write := range map[string]func() error{ + "save": func() error { + _, _, err := s.SavePage(ctx, root.ID, 2, json.RawMessage(emptyDoc), Attribution{}) + return err + }, + "update": func() error { _, _, err := s.UpdatePage(ctx, root.ID, PagePatch{}); return err }, + "move": func() error { _, _, err := s.MovePage(ctx, root.ID, "", ""); return err }, + "delete": func() error { _, _, err := s.DeletePage(ctx, root.ID); return err }, + "history": func() error { _, err := s.PageHistory(ctx, root.ID, 50); return err }, + "create parent": func() error { + _, _, err := s.CreatePage(ctx, PageCreate{WorkstreamID: w.ID, ParentID: root.ID, Title: "No"}) + return err + }, + "create after": func() error { + _, _, err := s.CreatePage(ctx, PageCreate{WorkstreamID: w.ID, AfterID: root.ID, Title: "No"}) + return err + }, + "move parent": func() error { _, _, err := s.MovePage(ctx, sibling.ID, root.ID, ""); return err }, + "move after": func() error { _, _, err := s.MovePage(ctx, sibling.ID, "", root.ID); return err }, + } { + t.Run(name, func(t *testing.T) { + if err := write(); !errors.Is(err, ErrNotFound) { + t.Fatalf("deleted target: %v", err) + } + }) + } + *now = now.Add(time.Hour) + restored, change, err := s.UndeletePage(ctx, root.ID) + if err != nil || restored.ParentID != parent.ID || restored.Position != 1 || restored.Title != lastTitle || restored.Version != 2 || string(restored.Doc) != string(doc) || !restored.DeletedAt.IsZero() || !restored.UpdatedAt.Equal(*now) || restored.UpdatedBy != "you" || restored.UpdatedFrom != nil || change.ID != w.ID || !reflect.DeepEqual(change.Parts, []ChangedPart{PartWorkstream, PartPages}) { + t.Fatalf("restore: %+v %+v %v", restored, change, err) + } + nodes, err = s.Pages(ctx, w.ID) + var liveIDs []string + for _, p := range nodes { + liveIDs = append(liveIDs, p.ID) + } + if err != nil || len(liveIDs) != 7 || !reflect.DeepEqual(liveIDs[1:], []string{parent.ID, sibling.ID, root.ID, first.ID, grandchild.ID, second.ID}) { + t.Fatalf("restored order: %+v %v", nodes, err) + } + for _, want := range []Page{first, grandchild, second} { + p, err := s.Page(ctx, want.ID, false) + if err != nil || p.ParentID != want.ParentID || p.Version != want.Version || !p.DeletedAt.IsZero() { + t.Fatalf("restored nesting: %+v %v", p, err) + } + } + deleted, err = s.DeletedPages(ctx, w.ID) + if err != nil || len(deleted) != 1 || deleted[0].ID != earlier.ID { + t.Fatalf("earlier deletion revived: %+v %v", deleted, err) + } + history, err := s.PageHistory(ctx, root.ID, 50) + if err != nil || len(history) != 2 || history[0].Title != root.Title || history[0].SavedBy != "agent" || history[0].From == nil || history[0].From.ID != "source" { + t.Fatalf("restore changed saved history: %+v %v", history, err) + } +} + +// WP-F1 (b): restoring a descendant of a deleted parent restores only its own +// group subtree, appends at the root, and remains permitted on a done workstream. +func TestWorkstreamDeletedDescendantRestoresAtRoot(t *testing.T) { + s, _, _ := testStore(t) + ctx := context.Background() + w := createStream(t, s, "Detached restore") + root := createWorkstreamPage(t, s, w.ID, "", "Root") + child := createWorkstreamPage(t, s, w.ID, root.ID, "Child") + grandchild := createWorkstreamPage(t, s, w.ID, child.ID, "Grandchild") + other := createWorkstreamPage(t, s, w.ID, root.ID, "Other") + last := createWorkstreamPage(t, s, w.ID, "", "Last") + if _, _, err := s.MarkDone(ctx, w.ID, ""); err != nil { + t.Fatal(err) + } + if _, _, err := s.DeletePage(ctx, root.ID); err != nil { + t.Fatal(err) + } + restored, _, err := s.UndeletePage(ctx, child.ID) + if err != nil || restored.ParentID != "" || restored.Position != 2 || restored.Version != 1 { + t.Fatalf("descendant restore: %+v %v", restored, err) + } + nodes, err := s.Pages(ctx, w.ID) + if err != nil || len(nodes) != 4 || nodes[1].ID != last.ID || nodes[2].ID != child.ID || nodes[3].ID != grandchild.ID { + t.Fatalf("detached tree: %+v %v", nodes, err) + } + for _, id := range []string{root.ID, other.ID} { + p, err := s.Page(ctx, id, false) + if err != nil || p.DeletedAt.IsZero() { + t.Fatalf("unrelated deleted page: %+v %v", p, err) + } + } + rootRestored, _, err := s.UndeletePage(ctx, root.ID) + if err != nil || rootRestored.ParentID != "" || rootRestored.Position != 3 { + t.Fatalf("root append: %+v %v", rootRestored, err) + } + p, err := s.Page(ctx, child.ID, false) + if err != nil || p.ParentID != "" || p.Position != 2 { + t.Fatalf("later root restore moved detached child: %+v %v", p, err) + } + stream, err := s.Get(ctx, w.ID) + if err != nil || stream.DoneAt.IsZero() { + t.Fatalf("restore changed done state: %+v %v", stream, err) + } +} + +// WP-F1 (b): collection deletion still purges deleted pages and their saved +// versions through the existing cascade, while page.delete retains both. +func TestWorkstreamDeletedPagesCollectionCascade(t *testing.T) { + s, storage, _ := testStore(t) + ctx := context.Background() + w := createStream(t, s, "Page retention") + root := createWorkstreamPage(t, s, w.ID, "", "Root") + child := createWorkstreamPage(t, s, w.ID, root.ID, "Child") + if _, _, err := s.DeletePage(ctx, root.ID); err != nil { + t.Fatal(err) + } + for _, id := range []string{root.ID, child.ID} { + if p, err := s.PageAtVersion(ctx, id, 1, false); err != nil || p.DeletedAt.IsZero() { + t.Fatalf("retained version: %+v %v", p, err) + } + } + if err := storage.Write(ctx, func(tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, "DELETE FROM collections WHERE id=?", w.ID) + return err + }); err != nil { + t.Fatal(err) + } + if err := storage.Read(ctx, func(tx *sql.Tx, _ int) error { + for _, table := range []string{"workstream_pages", "workstream_page_versions"} { + var count int + if err := tx.QueryRowContext(ctx, "SELECT count(*) FROM "+table).Scan(&count); err != nil { + return err + } + if count != 0 { + t.Errorf("%s retained %d rows after collection purge", table, count) + } + } + return nil + }); err != nil { + t.Fatal(err) + } +} + +// WP-F1 (c): the exported creation default trims Unicode space and uses case +// folding, while the store's existing explicit boolean remains authoritative. +func TestWorkstreamReadByJobsDefault(t *testing.T) { + for _, tc := range []struct { + title string + want bool + }{ + {"Spec", true}, {" decisions ", true}, {"DECISIONS", true}, {"\u2003sPeC\u2003", true}, + {"Spec notes", false}, {"Overview", false}, {"Notes", false}, {"", false}, + } { + if got := ReadByJobsDefault(tc.title); got != tc.want { + t.Errorf("%q: %v, want %v", tc.title, got, tc.want) + } + } + s, _, _ := testStore(t) + w := createStream(t, s, "Explicit store default") + p := createWorkstreamPage(t, s, w.ID, "", "Spec") + if p.ReadByJobs { + t.Fatal("store changed an explicit false to the title default") + } +} diff --git a/internal/workstream/workstream.go b/internal/workstream/workstream.go index ce31042bb1..247132bedf 100644 --- a/internal/workstream/workstream.go +++ b/internal/workstream/workstream.go @@ -157,6 +157,7 @@ type PageNode struct { UpdatedAt time.Time UpdatedBy string UpdatedFrom *Source + DeletedAt time.Time } type Page struct { PageNode