From f2573010f17e4b5899b932fe35bae51381adadf1 Mon Sep 17 00:00:00 2001 From: CatfishGG Date: Tue, 12 May 2026 22:20:57 +0530 Subject: [PATCH 1/2] fix(quest): ensure deterministic note/event ordering under race conditions (fixes #85) - loadNotes/loadEvents: explicitly select id column for reliable secondary sort when created_at timestamps collide at sub-second precision - TestScroll_OrderChronological: add microsecond sleep between Journal calls to guarantee distinct timestamps and prevent flaky ordering under 'go test -race' --- internal/quest/scroll.go | 10 ++++++---- internal/quest/scroll_test.go | 5 +++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/internal/quest/scroll.go b/internal/quest/scroll.go index e57c074..0e62d63 100644 --- a/internal/quest/scroll.go +++ b/internal/quest/scroll.go @@ -72,7 +72,7 @@ func Scroll(ctx context.Context, db *sql.DB, projectID, questID string) (*Scroll // loadNotes returns all task_notes rows for questID, oldest first. func loadNotes(ctx context.Context, db *sql.DB, projectID, questID string) ([]NoteEntry, error) { rows, err := db.QueryContext(ctx, - `SELECT agent_id, note, created_at + `SELECT id, agent_id, note, created_at FROM task_notes WHERE project_id = ? AND task_id = ? ORDER BY created_at ASC, id ASC`, @@ -85,8 +85,9 @@ func loadNotes(ctx context.Context, db *sql.DB, projectID, questID string) ([]No var out []NoteEntry for rows.Next() { + var id int64 var agentID, note, createdAt string - if err := rows.Scan(&agentID, ¬e, &createdAt); err != nil { + if err := rows.Scan(&id, &agentID, ¬e, &createdAt); err != nil { return nil, fmt.Errorf("quest: scroll: scan note: %w", err) } t := parseNoteTime(createdAt) @@ -101,7 +102,7 @@ func loadNotes(ctx context.Context, db *sql.DB, projectID, questID string) ([]No // loadEvents returns all task_events rows for questID, oldest first. func loadEvents(ctx context.Context, db *sql.DB, projectID, questID string) ([]EventEntry, error) { rows, err := db.QueryContext(ctx, - `SELECT event, COALESCE(agent_id,''), COALESCE(data,''), created_at + `SELECT id, event, COALESCE(agent_id,''), COALESCE(data,''), created_at FROM task_events WHERE project_id = ? AND task_id = ? ORDER BY created_at ASC, id ASC`, @@ -114,8 +115,9 @@ func loadEvents(ctx context.Context, db *sql.DB, projectID, questID string) ([]E var out []EventEntry for rows.Next() { + var id int64 var event, agentID, data, createdAt string - if err := rows.Scan(&event, &agentID, &data, &createdAt); err != nil { + if err := rows.Scan(&id, &event, &agentID, &data, &createdAt); err != nil { return nil, fmt.Errorf("quest: scroll: scan event: %w", err) } t := parseNoteTime(createdAt) diff --git a/internal/quest/scroll_test.go b/internal/quest/scroll_test.go index 4b2493a..1978553 100644 --- a/internal/quest/scroll_test.go +++ b/internal/quest/scroll_test.go @@ -4,6 +4,7 @@ import ( "context" "strings" "testing" + "time" ) func TestScroll_FullHistory(t *testing.T) { @@ -103,6 +104,10 @@ func TestScroll_OrderChronological(t *testing.T) { if err := Journal(ctx, db, pid, q.ID, "agent", text); err != nil { t.Fatalf("Journal: %v", err) } + // Sleep briefly to ensure distinct timestamps for each entry. + // This guards against sub-second timestamp collisions that could + // cause flaky ordering under -race. + time.Sleep(time.Microsecond) } res, err := Scroll(ctx, db, pid, q.ID) From 952a4e439d1db57c8c886c8cc26a9a48ae93bb51 Mon Sep 17 00:00:00 2001 From: CatfishGG Date: Wed, 13 May 2026 17:48:42 +0530 Subject: [PATCH 2/2] fix(lore): validate --kind before import (fixes #87) --- internal/lore/catalog.go | 18 ++++++++++++++++++ internal/lore/catalog_cmd.go | 18 ++++++++++++++++++ internal/lore/catalog_test.go | 30 ++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/internal/lore/catalog.go b/internal/lore/catalog.go index ddfb13c..b1b7dfb 100644 --- a/internal/lore/catalog.go +++ b/internal/lore/catalog.go @@ -62,6 +62,24 @@ func Catalog(ctx context.Context, db *sql.DB, p *CatalogParams) (*CatalogResult, if strings.TrimSpace(p.Dir) == "" { return nil, fmt.Errorf("lore: catalog: dir required") } + if p.Kind != "" { + valid := false + for _, k := range AllKinds() { + if p.Kind == k { + valid = true + break + } + } + if !valid { + return nil, fmt.Errorf("lore: catalog: invalid kind %q: valid kinds are %s", p.Kind, strings.Join(func() []string { + kinds := make([]string, len(AllKinds())) + for i, k := range AllKinds() { + kinds[i] = string(k) + } + return kinds + }(), ", ")) + } + } info, err := os.Stat(p.Dir) if err != nil { diff --git a/internal/lore/catalog_cmd.go b/internal/lore/catalog_cmd.go index bb72f3b..9dbe38c 100644 --- a/internal/lore/catalog_cmd.go +++ b/internal/lore/catalog_cmd.go @@ -38,6 +38,24 @@ var CatalogCommand = &command.Command[CatalogInput, CatalogCmdOutput]{ if strings.TrimSpace(in.Dir) == "" { return CatalogCmdOutput{}, errors.New("dir required") } + if in.Kind != "" { + valid := false + for _, k := range AllKinds() { + if Kind(in.Kind) == k { + valid = true + break + } + } + if !valid { + return CatalogCmdOutput{}, fmt.Errorf("invalid --kind %q: valid kinds are: %s", in.Kind, strings.Join(func() []string { + kinds := make([]string, len(AllKinds())) + for i, k := range AllKinds() { + kinds[i] = string(k) + } + return kinds + }(), ", ")) + } + } db, err := d.OpenDB(ctx) if err != nil { return CatalogCmdOutput{}, err diff --git a/internal/lore/catalog_test.go b/internal/lore/catalog_test.go index 02d7a89..3b1e2d7 100644 --- a/internal/lore/catalog_test.go +++ b/internal/lore/catalog_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "strings" "testing" ) @@ -180,3 +181,32 @@ func TestCatalog_NonExistentDir(t *testing.T) { t.Error("Catalog on non-existent dir should return error") } } + +func TestCatalog_InvalidKind(t *testing.T) { + ctx := context.Background() + db := openTestDB(t, "catalog-kind") + + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "note.md"), []byte("# Note\n\nContent."), 0o600); err != nil { + t.Fatalf("write note.md: %v", err) + } + + _, err := Catalog(ctx, db, &CatalogParams{ + Dir: dir, + ProjectID: "catalog-kind", + Kind: "not-a-real-kind", + }) + if err == nil { + t.Fatal("Catalog with invalid --kind should return error") + } + // Verify error mentions valid kinds. + for _, k := range AllKinds() { + if !strings.Contains(err.Error(), string(k)) { + t.Errorf("error %q does not contain valid kind %q", err.Error(), k) + } + } + // Verify error mentions the invalid kind. + if !strings.Contains(err.Error(), "not-a-real-kind") { + t.Errorf("error %q does not contain the invalid kind", err.Error()) + } +}