From dc1e5d0c4000dd1a900a14975c44327a87f2c14d Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Wed, 22 Jul 2026 11:16:37 +0300 Subject: [PATCH] feat: Postgres schema, initial migration and the db/ query layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The six tables of the design's Postgres schema — space, document_id, proposal, agent_token, index_stamp, digest_mark — plus a typed query layer over them. project and comment stay deliberately absent. Two invariants are enforced in SQL rather than in Go, because a check-then-write in Go has a window and this does not: - Global document ID uniqueness is the doc_id PRIMARY KEY, and the merge path's multi-row upsert guards its ON CONFLICT branch on the owning space, so a batch claiming another space's ID leaves that row untouched and surfaces as a CollisionError naming where the ID really lives. - Proposal transitions run as UPDATE ... WHERE state = 'open'; the current state is read back only to name it in the error. CHECK constraints make an unknown state, a merged row without an approval kind or merged_rev, and empty provenance unwritable by anything, including SQL that does not go through this package. MergeProposal wraps the transition and the registry re-pointing in one transaction: a merged proposal whose documents are still registered at their pre-merge paths would break link resolution and the next staleness check. Agent tokens store only a SHA-256 hash; the lookup boundary re-verifies with subtle.ConstantTimeCompare and rejects revoked tokens distinctly. Tests skip when SPECSRHT_TEST_PG is unset, so the pure-logic half — batch duplicate detection, token comparison, argument guards, and the agreement between schema.sql and migrations/0001_initial.sql — still runs with no database available. --- db/db_test.go | 154 +++++++++++++++ db/document.go | 327 +++++++++++++++++++++++++++++++ db/document_test.go | 176 +++++++++++++++++ db/proposal.go | 282 ++++++++++++++++++++++++++ db/proposal_test.go | 295 ++++++++++++++++++++++++++++ db/space.go | 117 +++++++++++ db/space_test.go | 77 ++++++++ db/stamps.go | 101 ++++++++++ db/stamps_test.go | 97 +++++++++ db/store.go | 178 +++++++++++++++++ db/token.go | 201 +++++++++++++++++++ db/token_test.go | 118 +++++++++++ db/unit_test.go | 381 ++++++++++++++++++++++++++++++++++++ migrations/0001_initial.sql | 66 +++++++ schema.sql | 97 +++++++++ 15 files changed, 2667 insertions(+) create mode 100644 db/db_test.go create mode 100644 db/document.go create mode 100644 db/document_test.go create mode 100644 db/proposal.go create mode 100644 db/proposal_test.go create mode 100644 db/space.go create mode 100644 db/space_test.go create mode 100644 db/stamps.go create mode 100644 db/stamps_test.go create mode 100644 db/store.go create mode 100644 db/token.go create mode 100644 db/token_test.go create mode 100644 db/unit_test.go create mode 100644 migrations/0001_initial.sql create mode 100644 schema.sql diff --git a/db/db_test.go b/db/db_test.go new file mode 100644 index 0000000000000000000000000000000000000000..6bb1cb35ebad2ffdd404d5a14b4df36ff5042793 --- /dev/null +++ b/db/db_test.go @@ -0,0 +1,154 @@ +package db + +import ( + "context" + "crypto/rand" + "database/sql" + "encoding/hex" + "net/url" + "os" + "strings" + "testing" + + _ "github.com/lib/pq" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// testEnv names the DSN env var that gates the Postgres-backed tests. When +// unset, every integration test in this package skips with a clear message; the +// pure-logic tests (transition guards, token hashing, batch duplicate +// detection, schema/migration agreement) run regardless, so `go test ./db/...` +// is meaningful on a machine with no database at all. +const testEnv = "SPECSRHT_TEST_PG" + +// newTestStore connects to the Postgres pointed at by SPECSRHT_TEST_PG, creates +// an isolated scratch schema (specsrht_test_), applies schema.sql into +// it, and returns a Store bound to a pool whose search_path is that schema. The +// returned cleanup drops the schema and closes the pools. If the env var is +// unset the test is skipped. +// +// Isolation is achieved with a per-test schema rather than a whole database so +// no admin/CREATE DATABASE privilege is required and cleanup is a single +// DROP SCHEMA ... CASCADE. The scratch pool routes every connection to that +// schema via lib/pq's `options=-c search_path=...` startup parameter. +func newTestStore(t *testing.T) (*Store, *sql.DB, func()) { + t.Helper() + base := os.Getenv(testEnv) + if base == "" { + t.Skipf("%s not set; skipping Postgres-backed test (set it to a DSN to run)", testEnv) + } + + admin, err := sql.Open("postgres", base) + if err != nil { + t.Fatalf("open admin pool: %v", err) + } + if err := admin.Ping(); err != nil { + admin.Close() + t.Fatalf("ping %s: %v", testEnv, err) + } + + schema := "specsrht_test_" + randToken() + if _, err := admin.Exec(`CREATE SCHEMA "` + schema + `"`); err != nil { + admin.Close() + t.Fatalf("create schema %s: %v", schema, err) + } + + scopedDSN, err := withSearchPath(base, schema) + if err != nil { + admin.Exec(`DROP SCHEMA "` + schema + `" CASCADE`) + admin.Close() + t.Fatalf("build scoped dsn: %v", err) + } + pool, err := sql.Open("postgres", scopedDSN) + if err != nil { + admin.Exec(`DROP SCHEMA "` + schema + `" CASCADE`) + admin.Close() + t.Fatalf("open scoped pool: %v", err) + } + + ddl, err := os.ReadFile("../schema.sql") + if err != nil { + pool.Close() + admin.Exec(`DROP SCHEMA "` + schema + `" CASCADE`) + admin.Close() + t.Fatalf("read schema.sql: %v", err) + } + if _, err := pool.Exec(string(ddl)); err != nil { + pool.Close() + admin.Exec(`DROP SCHEMA "` + schema + `" CASCADE`) + admin.Close() + t.Fatalf("apply schema.sql: %v", err) + } + + cleanup := func() { + pool.Close() + if _, err := admin.Exec(`DROP SCHEMA "` + schema + `" CASCADE`); err != nil { + t.Errorf("drop schema %s: %v", schema, err) + } + admin.Close() + } + return NewStore(pool), pool, cleanup +} + +// withSearchPath returns base with a connection option that pins search_path to +// schema for every pooled connection, handling both URL and keyword DSN forms. +func withSearchPath(base, schema string) (string, error) { + opt := "-c search_path=" + schema + if strings.Contains(base, "://") { + u, err := url.Parse(base) + if err != nil { + return "", err + } + q := u.Query() + q.Set("options", opt) + u.RawQuery = q.Encode() + return u.String(), nil + } + return base + " options='" + opt + "'", nil +} + +func randToken() string { + b := make([]byte, 8) + if _, err := rand.Read(b); err != nil { + panic(err) + } + return hex.EncodeToString(b) +} + +// mkSpace is a convenience for CreateSpace in tests. +func mkSpace(t *testing.T, s *Store, ctx context.Context, owner, name string) *Space { + t.Helper() + sp, err := s.CreateSpace(ctx, core.SpaceRef{Owner: owner, Name: name}) + if err != nil { + t.Fatalf("create space ~%s/%s: %v", owner, name, err) + } + return sp +} + +// docID parses a document ID or fails the test. +func docID(t *testing.T, s string) core.DocID { + t.Helper() + id, err := core.ParseDocID(s) + if err != nil { + t.Fatalf("parse doc id %q: %v", s, err) + } + return id +} + +// mkProposal opens a proposal with plausible provenance. +func mkProposal(t *testing.T, s *Store, ctx context.Context, spaceID int, title string) *Proposal { + t.Helper() + p, err := s.OpenProposal(ctx, &Proposal{ + SpaceID: spaceID, + Title: title, + Rationale: "because", + BaseRev: "0000000000000000000000000000000000000000", + Agent: "claude-code/spec-writer", + AgentSession: "8fb9c9a4-b078-4af1-89eb-d97c522f9921", + }) + if err != nil { + t.Fatalf("open proposal %q: %v", title, err) + } + return p +} diff --git a/db/document.go b/db/document.go new file mode 100644 index 0000000000000000000000000000000000000000..023dcd7e21f672c66024b9ffa0af706011a59a2b --- /dev/null +++ b/db/document.go @@ -0,0 +1,327 @@ +package db + +import ( + "context" + "database/sql" + "errors" + "fmt" + "sort" + "strings" + + "github.com/lib/pq" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// Document is one row of the global document ID registry: where the document +// carrying this ID currently lives on its space's approved branch. +// +// Paths move; IDs do not. Cross-space links resolve by ID, comment anchors +// reference IDs, and merge staleness is keyed by ID — so this table is the map +// from the stable name to the moving one, and nothing else. +type Document struct { + ID core.DocID + SpaceID int + Path string + UpdatedRev string +} + +// DocRef pairs a document ID with the path it occupies in a tree. It is the +// unit the push validator and the merge path work in: a set of (id, path) taken +// from the frontmatter of the documents a push or a proposal touches. +type DocRef struct { + ID core.DocID + Path string +} + +// Collision is one registered ID that a batch tried to claim for a different +// space. Existing is the row that already holds it, so the rejection message +// can name where the ID actually lives instead of just saying "taken". +type Collision struct { + DocID core.DocID + Existing *Document +} + +func (c Collision) String() string { + return fmt.Sprintf("%s is already registered in space %d at %s", + c.DocID, c.Existing.SpaceID, c.Existing.Path) +} + +// CollisionError reports one or more global ID collisions. It wraps +// ErrDocIDTaken so callers can match the class with errors.Is and still reach +// the per-ID detail for the rejection message the `update` hook prints. +type CollisionError struct { + Collisions []Collision +} + +func (e *CollisionError) Error() string { + parts := make([]string, len(e.Collisions)) + for i, c := range e.Collisions { + parts[i] = c.String() + } + return "db: document id collision: " + strings.Join(parts, "; ") +} + +func (e *CollisionError) Unwrap() error { return ErrDocIDTaken } + +const documentSelect = `SELECT doc_id, space_id, path, updated_rev FROM document_id` + +func scanDocument(sc rowScanner) (*Document, error) { + var ( + d Document + docID string + ) + if err := sc.Scan(&docID, &d.SpaceID, &d.Path, &d.UpdatedRev); err != nil { + return nil, err + } + parsed, err := core.ParseDocID(docID) + if err != nil { + // The registry only ever accepts parsed IDs, so a row that no longer + // parses is corruption, not input. Surface it instead of guessing. + return nil, fmt.Errorf("registry row %q: %w", docID, err) + } + d.ID = parsed + return &d, nil +} + +// RegisterDocID claims a document ID for a space at a path. Global uniqueness is +// the doc_id PRIMARY KEY, so a second claim cannot be inserted at all — this +// method maps that refusal to ErrDocIDTaken rather than being the thing that +// prevents it. +func (s *Store) RegisterDocID(ctx context.Context, spaceID int, ref DocRef, rev string) (*Document, error) { + if err := core.ValidateDocPath(ref.Path); err != nil { + return nil, err + } + const q = ` +INSERT INTO document_id (doc_id, space_id, path, updated_rev) +VALUES ($1, $2, $3, $4)` + _, err := s.q.ExecContext(ctx, q, ref.ID.String(), spaceID, ref.Path, rev) + if err != nil { + var pqErr *pq.Error + if errors.As(err, &pqErr) && pqErr.Code == "23505" { + return nil, fmt.Errorf("%w: %s", ErrDocIDTaken, ref.ID) + } + return nil, fmt.Errorf("register doc id %s: %w", ref.ID, err) + } + return &Document{ID: ref.ID, SpaceID: spaceID, Path: ref.Path, UpdatedRev: rev}, nil +} + +// DocByID resolves a document ID to its current space and path. This is the +// lookup wikilink resolution and merge staleness both go through. Returns +// ErrNotFound if the ID is not registered. +func (s *Store) DocByID(ctx context.Context, id core.DocID) (*Document, error) { + q := documentSelect + ` WHERE doc_id = $1` + d, err := scanDocument(s.q.QueryRowContext(ctx, q, id.String())) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("get doc id %s: %w", id, err) + } + return d, nil +} + +// SetDocPath re-points a registered ID at a new path after a rename, stamping +// the revision that moved it. It is scoped by space: a rename never crosses +// spaces (only a human push can move a file, and a push touches one repo), so a +// spaceID that does not match the registered row yields ErrNotFound rather than +// silently relocating the document into another space. +func (s *Store) SetDocPath(ctx context.Context, spaceID int, ref DocRef, rev string) error { + if err := core.ValidateDocPath(ref.Path); err != nil { + return err + } + const q = ` +UPDATE document_id +SET path = $3, updated_rev = $4 +WHERE doc_id = $1 AND space_id = $2` + res, err := s.q.ExecContext(ctx, q, ref.ID.String(), spaceID, ref.Path, rev) + if err != nil { + return fmt.Errorf("set path for doc id %s: %w", ref.ID, err) + } + return requireOne(res, "set doc path") +} + +// UnregisterDocID drops an ID from the registry, for a human push that deleted +// the document (deletion is human-push-only by design; an agent proposes +// `status: superseded` instead). Scoped by space for the same reason SetDocPath +// is. Returns ErrNotFound if no such row exists in that space. +func (s *Store) UnregisterDocID(ctx context.Context, spaceID int, id core.DocID) error { + res, err := s.q.ExecContext(ctx, + `DELETE FROM document_id WHERE doc_id = $1 AND space_id = $2`, id.String(), spaceID) + if err != nil { + return fmt.Errorf("unregister doc id %s: %w", id, err) + } + return requireOne(res, "unregister doc id") +} + +// ListDocsBySpace returns every registered document of a space, by path. The +// reconciler uses it to diff the registry against what the approved tree +// actually contains. +func (s *Store) ListDocsBySpace(ctx context.Context, spaceID int) ([]*Document, error) { + q := documentSelect + ` WHERE space_id = $1 ORDER BY path` + rows, err := s.q.QueryContext(ctx, q, spaceID) + if err != nil { + return nil, fmt.Errorf("list docs space=%d: %w", spaceID, err) + } + defer rows.Close() + var docs []*Document + for rows.Next() { + d, err := scanDocument(rows) + if err != nil { + return nil, fmt.Errorf("scan document: %w", err) + } + docs = append(docs, d) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate documents: %w", err) + } + return docs, nil +} + +// DuplicateDocIDs reports IDs that appear more than once in a single batch, +// sorted. This is the half of the collision check that needs no database: a +// push carrying the same `id:` on two different documents is malformed on its +// own terms, whatever the registry says. It is also a precondition of +// UpsertDocIDs, since a multi-row upsert cannot touch the same key twice. +func DuplicateDocIDs(refs []DocRef) []core.DocID { + seen := make(map[string]int, len(refs)) + for _, r := range refs { + seen[r.ID.String()]++ + } + var dup []string + for id, n := range seen { + if n > 1 { + dup = append(dup, id) + } + } + sort.Strings(dup) + out := make([]core.DocID, 0, len(dup)) + for _, id := range dup { + parsed, err := core.ParseDocID(id) + if err != nil { + // Impossible: the input carried parsed DocIDs. + panic(fmt.Sprintf("db: unparseable DocID in batch: %v", err)) + } + out = append(out, parsed) + } + return out +} + +// CheckDocIDCollisions reports which of refs are already registered to a +// different space. This is what the push-validation path calls before a push is +// allowed through: a duplicated `id:` corrupts the global registry and silently +// breaks link resolution and search, and is far cheaper to reject at push time +// than to find weeks later. +// +// An ID already registered to *this* space is not a collision — that is the +// ordinary case of editing or renaming a document that already exists. +// Duplicates within refs itself are reported separately by DuplicateDocIDs; +// this method only asks the registry. +func (s *Store) CheckDocIDCollisions(ctx context.Context, spaceID int, refs []DocRef) ([]Collision, error) { + if len(refs) == 0 { + return nil, nil + } + ids := make([]string, len(refs)) + for i, r := range refs { + ids[i] = r.ID.String() + } + q := documentSelect + ` WHERE doc_id = ANY($1) AND space_id <> $2 ORDER BY doc_id` + rows, err := s.q.QueryContext(ctx, q, pq.Array(ids), spaceID) + if err != nil { + return nil, fmt.Errorf("check doc id collisions: %w", err) + } + defer rows.Close() + var out []Collision + for rows.Next() { + d, err := scanDocument(rows) + if err != nil { + return nil, fmt.Errorf("scan collision: %w", err) + } + out = append(out, Collision{DocID: d.ID, Existing: d}) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate collisions: %w", err) + } + return out, nil +} + +// UpsertDocIDs registers or re-points every ID in refs for one space, in a +// single statement. It is the registry half of a merge: after the merge commit +// lands, the documents it touched are at these paths, at this revision. +// +// The cross-space guard lives in the statement, not in Go: the ON CONFLICT +// branch only updates when the existing row belongs to the same space, so a row +// owned by another space is left untouched and simply not returned. Any ID that +// does not come back is therefore a collision, reported as a *CollisionError +// naming where it really lives. A check-then-write in Go would have a window +// between the two; this does not. +func (s *Store) UpsertDocIDs(ctx context.Context, spaceID int, refs []DocRef, rev string) error { + if len(refs) == 0 { + return nil + } + if dup := DuplicateDocIDs(refs); len(dup) > 0 { + names := make([]string, len(dup)) + for i, d := range dup { + names[i] = d.String() + } + return fmt.Errorf("%w: %s", ErrDocIDDuplicate, strings.Join(names, ", ")) + } + ids := make([]string, len(refs)) + paths := make([]string, len(refs)) + for i, r := range refs { + if err := core.ValidateDocPath(r.Path); err != nil { + return err + } + ids[i] = r.ID.String() + paths[i] = r.Path + } + + const q = ` +INSERT INTO document_id (doc_id, space_id, path, updated_rev) +SELECT d.doc_id, $2, d.path, $3 +FROM unnest($1::text[], $4::text[]) AS d(doc_id, path) +ON CONFLICT (doc_id) DO UPDATE + SET path = EXCLUDED.path, updated_rev = EXCLUDED.updated_rev + WHERE document_id.space_id = EXCLUDED.space_id +RETURNING doc_id` + rows, err := s.q.QueryContext(ctx, q, pq.Array(ids), spaceID, rev, pq.Array(paths)) + if err != nil { + return fmt.Errorf("upsert doc ids: %w", err) + } + applied := make(map[string]bool, len(ids)) + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + rows.Close() + return fmt.Errorf("scan upserted doc id: %w", err) + } + applied[id] = true + } + if err := rows.Err(); err != nil { + rows.Close() + return fmt.Errorf("iterate upserted doc ids: %w", err) + } + rows.Close() + if len(applied) == len(ids) { + return nil + } + + var missing []DocRef + for _, r := range refs { + if !applied[r.ID.String()] { + missing = append(missing, r) + } + } + collisions, err := s.CheckDocIDCollisions(ctx, spaceID, missing) + if err != nil { + return err + } + if len(collisions) == 0 { + // The guard skipped rows but the registry says nothing owns them + // elsewhere. That is not a condition this schema can produce; refuse + // rather than report a merge as clean. + return fmt.Errorf("upsert doc ids: %d of %d rows not applied, but no collision found", + len(ids)-len(applied), len(ids)) + } + return &CollisionError{Collisions: collisions} +} diff --git a/db/document_test.go b/db/document_test.go new file mode 100644 index 0000000000000000000000000000000000000000..a4a22f733b1f14468c7f42418f4bd560e237a6aa --- /dev/null +++ b/db/document_test.go @@ -0,0 +1,176 @@ +package db + +import ( + "context" + "errors" + "testing" +) + +func TestDocumentRegistryLifecycle(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + space := mkSpace(t, s, ctx, "bigbes", "rfcs") + id := docID(t, "SPEC-0007") + + doc, err := s.RegisterDocID(ctx, space.ID, DocRef{ID: id, Path: "specs/0007-storage.md"}, "rev1") + if err != nil { + t.Fatalf("register: %v", err) + } + if doc.SpaceID != space.ID || doc.Path != "specs/0007-storage.md" || doc.UpdatedRev != "rev1" { + t.Fatalf("unexpected registered doc: %+v", doc) + } + + // The doc_id PRIMARY KEY makes a second claim unwritable, in this space or + // any other. + if _, err := s.RegisterDocID(ctx, space.ID, DocRef{ID: id, Path: "other.md"}, "rev2"); !errors.Is(err, ErrDocIDTaken) { + t.Fatalf("re-register in the same space = %v, want ErrDocIDTaken", err) + } + other := mkSpace(t, s, ctx, "bigbes", "notes") + if _, err := s.RegisterDocID(ctx, other.ID, DocRef{ID: id, Path: "n.md"}, "rev2"); !errors.Is(err, ErrDocIDTaken) { + t.Fatalf("re-register in another space = %v, want ErrDocIDTaken", err) + } + + got, err := s.DocByID(ctx, id) + if err != nil { + t.Fatalf("lookup: %v", err) + } + if got.ID != id || got.Path != "specs/0007-storage.md" { + t.Fatalf("unexpected lookup: %+v", got) + } + if _, err := s.DocByID(ctx, docID(t, "SPEC-9999")); !errors.Is(err, ErrNotFound) { + t.Fatalf("missing id = %v, want ErrNotFound", err) + } + + // Rename: the ID is stable, the path moves. + if err := s.SetDocPath(ctx, space.ID, DocRef{ID: id, Path: "specs/0007-proposals.md"}, "rev3"); err != nil { + t.Fatalf("rename: %v", err) + } + got, err = s.DocByID(ctx, id) + if err != nil { + t.Fatalf("lookup after rename: %v", err) + } + if got.Path != "specs/0007-proposals.md" || got.UpdatedRev != "rev3" { + t.Fatalf("rename did not stick: %+v", got) + } + + // A rename scoped to the wrong space must not relocate the document. + if err := s.SetDocPath(ctx, other.ID, DocRef{ID: id, Path: "stolen.md"}, "rev4"); !errors.Is(err, ErrNotFound) { + t.Fatalf("cross-space rename = %v, want ErrNotFound", err) + } + if got, _ := s.DocByID(ctx, id); got.Path != "specs/0007-proposals.md" { + t.Fatalf("cross-space rename moved the document: %+v", got) + } + + docs, err := s.ListDocsBySpace(ctx, space.ID) + if err != nil { + t.Fatalf("list docs: %v", err) + } + if len(docs) != 1 || docs[0].ID != id { + t.Fatalf("unexpected doc list: %+v", docs) + } + + // Deletion (human push only) frees the ID again. + if err := s.UnregisterDocID(ctx, other.ID, id); !errors.Is(err, ErrNotFound) { + t.Fatalf("unregister from the wrong space = %v, want ErrNotFound", err) + } + if err := s.UnregisterDocID(ctx, space.ID, id); err != nil { + t.Fatalf("unregister: %v", err) + } + if _, err := s.DocByID(ctx, id); !errors.Is(err, ErrNotFound) { + t.Fatalf("after unregister = %v, want ErrNotFound", err) + } +} + +func TestDocIDCollisionCheck(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + specs := mkSpace(t, s, ctx, "bigbes", "rfcs") + notes := mkSpace(t, s, ctx, "bigbes", "notes") + + mine := docID(t, "SPEC-1") + theirs := docID(t, "NOTE-1") + if _, err := s.RegisterDocID(ctx, specs.ID, DocRef{ID: mine, Path: "a.md"}, "rev1"); err != nil { + t.Fatalf("register: %v", err) + } + if _, err := s.RegisterDocID(ctx, notes.ID, DocRef{ID: theirs, Path: "b.md"}, "rev1"); err != nil { + t.Fatalf("register: %v", err) + } + + // Pushing into specs: SPEC-1 is ours (a rename at worst), NOTE-1 is not. + batch := []DocRef{ + {ID: mine, Path: "a-renamed.md"}, + {ID: theirs, Path: "c.md"}, + {ID: docID(t, "SPEC-2"), Path: "d.md"}, + } + collisions, err := s.CheckDocIDCollisions(ctx, specs.ID, batch) + if err != nil { + t.Fatalf("collision check: %v", err) + } + if len(collisions) != 1 { + t.Fatalf("expected 1 collision, got %+v", collisions) + } + if collisions[0].DocID != theirs || collisions[0].Existing.SpaceID != notes.ID { + t.Fatalf("unexpected collision: %+v", collisions[0]) + } +} + +func TestUpsertDocIDsRejectsCrossSpaceClaim(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + specs := mkSpace(t, s, ctx, "bigbes", "rfcs") + notes := mkSpace(t, s, ctx, "bigbes", "notes") + stolen := docID(t, "NOTE-1") + if _, err := s.RegisterDocID(ctx, notes.ID, DocRef{ID: stolen, Path: "n.md"}, "rev1"); err != nil { + t.Fatalf("register: %v", err) + } + + // A fresh ID plus one owned by another space: the statement's ON CONFLICT + // guard leaves the foreign row alone and the missing row surfaces as a + // CollisionError. + fresh := docID(t, "SPEC-1") + err := s.UpsertDocIDs(ctx, specs.ID, []DocRef{ + {ID: fresh, Path: "a.md"}, + {ID: stolen, Path: "b.md"}, + }, "rev2") + if !errors.Is(err, ErrDocIDTaken) { + t.Fatalf("cross-space upsert = %v, want ErrDocIDTaken", err) + } + var ce *CollisionError + if !errors.As(err, &ce) || len(ce.Collisions) != 1 || ce.Collisions[0].DocID != stolen { + t.Fatalf("expected a CollisionError naming %s, got %v", stolen, err) + } + // The foreign row is untouched. + got, err := s.DocByID(ctx, stolen) + if err != nil { + t.Fatalf("lookup foreign doc: %v", err) + } + if got.SpaceID != notes.ID || got.Path != "n.md" || got.UpdatedRev != "rev1" { + t.Fatalf("foreign row was modified: %+v", got) + } + + // A clean batch inserts and then updates in place. + if err := s.UpsertDocIDs(ctx, specs.ID, []DocRef{ + {ID: fresh, Path: "a.md"}, + {ID: docID(t, "SPEC-2"), Path: "b.md"}, + }, "rev3"); err != nil { + t.Fatalf("clean upsert: %v", err) + } + if err := s.UpsertDocIDs(ctx, specs.ID, []DocRef{ + {ID: fresh, Path: "a-moved.md"}, + }, "rev4"); err != nil { + t.Fatalf("upsert rename: %v", err) + } + got, err = s.DocByID(ctx, fresh) + if err != nil { + t.Fatalf("lookup: %v", err) + } + if got.Path != "a-moved.md" || got.UpdatedRev != "rev4" { + t.Fatalf("upsert rename did not stick: %+v", got) + } +} diff --git a/db/proposal.go b/db/proposal.go new file mode 100644 index 0000000000000000000000000000000000000000..7400d46541f51a70af999f35cae554821676b6d7 --- /dev/null +++ b/db/proposal.go @@ -0,0 +1,282 @@ +package db + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strconv" + "time" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// BranchPrefix is the ref namespace agents may write. The refs rule — an agent +// token can only update refs under this prefix, and only the owner can move the +// approved branch — is the boundary that actually bounds the damage a confused +// agent can do. +const BranchPrefix = "proposals/" + +// ProposalBranch is the branch name for a proposal id: "proposals/42". The row +// stores it verbatim (proposal.branch) because the row, not this function, is +// what the reconciler compares against the refs it finds. +func ProposalBranch(id int) string { return BranchPrefix + strconv.Itoa(id) } + +// Proposal is a bundle of document edits awaiting review: a branch under +// BranchPrefix plus this row. +// +// BaseRev is the If-Match value the agent sent when the proposal was opened — +// the space's approved-head sha at the time it read the document — and it does +// not move as the proposal accumulates edits. Agent and AgentSession are +// mandatory provenance: one shared token still yields a full audit trail, +// because the identity strings, not the credential, are what say who did what. +// +// Approval and MergedRev are empty until the proposal merges; Resolved is nil +// until it leaves the open state. +type Proposal struct { + ID int + SpaceID int + Title string + Rationale string + BaseRev string + Branch string + State core.ProposalState + Approval core.Approval + MergedRev string + Agent string + AgentSession string + Created time.Time + Resolved *time.Time +} + +// Merge is everything one merge writes to Postgres: the proposal's transition +// to merged, and the new location of every document the merge touched. The two +// are one invariant — a merged proposal whose documents are still registered at +// their old paths would break link resolution and the next staleness check — +// so MergeProposal writes them in a single transaction. +type Merge struct { + ProposalID int + SpaceID int + Approval core.Approval + MergedRev string + // Docs is the (id, path) set of the documents the merge landed, at their + // paths in the merge commit. Empty is legal but unusual: it means the + // proposal touched nothing the registry tracks. + Docs []DocRef +} + +const proposalSelect = ` +SELECT id, space_id, title, COALESCE(rationale, ''), base_rev, branch, state, + COALESCE(approval, ''), COALESCE(merged_rev, ''), agent, agent_session, + created, resolved +FROM proposal` + +func scanProposal(sc rowScanner) (*Proposal, error) { + var ( + p Proposal + state string + approval string + resolved sql.NullTime + ) + if err := sc.Scan(&p.ID, &p.SpaceID, &p.Title, &p.Rationale, &p.BaseRev, + &p.Branch, &state, &approval, &p.MergedRev, &p.Agent, &p.AgentSession, + &p.Created, &resolved); err != nil { + return nil, err + } + parsedState, err := core.ParseProposalState(state) + if err != nil { + return nil, fmt.Errorf("proposal %d: %w", p.ID, err) + } + p.State = parsedState + if approval != "" { + parsedApproval, err := core.ParseApproval(approval) + if err != nil { + return nil, fmt.Errorf("proposal %d: %w", p.ID, err) + } + p.Approval = parsedApproval + } + if resolved.Valid { + t := resolved.Time + p.Resolved = &t + } + return &p, nil +} + +// OpenProposal inserts a new proposal in the open state and returns it with its +// id, branch and creation time filled in. p.SpaceID, p.Title, p.BaseRev, +// p.Agent and p.AgentSession must be set; State, Approval, MergedRev and +// Resolved are ignored on input — a proposal is always born open. +// +// The branch name derives from the generated id ("proposals/42"), so id and +// branch are allocated in one statement: taking the id in a first round trip +// and writing the branch in a second would leave a window where a crash yields +// a row whose branch names nothing. +func (s *Store) OpenProposal(ctx context.Context, p *Proposal) (*Proposal, error) { + if p.Agent == "" || p.AgentSession == "" { + return nil, fmt.Errorf("open proposal: agent identity and session are required provenance") + } + if p.BaseRev == "" { + return nil, fmt.Errorf("open proposal: base rev is required") + } + const q = ` +WITH next AS (SELECT nextval(pg_get_serial_sequence('proposal', 'id')) AS id) +INSERT INTO proposal (id, space_id, title, rationale, base_rev, branch, state, + agent, agent_session, created) +SELECT next.id, $1, $2, $3, $4, $5::text || next.id::text, $6, $7, $8, $9 +FROM next +RETURNING id, branch, created` + out := *p + out.State = core.StateOpen + out.Approval = "" + out.MergedRev = "" + out.Resolved = nil + err := s.q.QueryRowContext(ctx, q, + p.SpaceID, p.Title, nullable(p.Rationale), p.BaseRev, BranchPrefix, + string(core.StateOpen), p.Agent, p.AgentSession, time.Now().UTC(), + ).Scan(&out.ID, &out.Branch, &out.Created) + if err != nil { + return nil, fmt.Errorf("open proposal: %w", err) + } + return &out, nil +} + +// GetProposal resolves a proposal by id. Returns ErrNotFound if it does not +// exist. Proposal URLs are stable and shareable — a link still resolves after +// merge or rejection, showing the outcome — so this is the same lookup whatever +// the state. +func (s *Store) GetProposal(ctx context.Context, id int) (*Proposal, error) { + q := proposalSelect + ` WHERE id = $1` + p, err := scanProposal(s.q.QueryRowContext(ctx, q, id)) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("get proposal %d: %w", id, err) + } + return p, nil +} + +// ListProposalsByState lists proposals in one state, newest first — the inbox +// query ("N proposals waiting on you"), served by ix_proposal_state_created. +// limit <= 0 means no limit. +func (s *Store) ListProposalsByState(ctx context.Context, state core.ProposalState, limit int) ([]*Proposal, error) { + if _, err := core.ParseProposalState(string(state)); err != nil { + return nil, err + } + q := proposalSelect + ` WHERE state = $1 ORDER BY created DESC, id DESC` + args := []any{string(state)} + if limit > 0 { + q += ` LIMIT $2` + args = append(args, limit) + } + rows, err := s.q.QueryContext(ctx, q, args...) + if err != nil { + return nil, fmt.Errorf("list proposals state=%s: %w", state, err) + } + defer rows.Close() + var out []*Proposal + for rows.Next() { + p, err := scanProposal(rows) + if err != nil { + return nil, fmt.Errorf("scan proposal: %w", err) + } + out = append(out, p) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate proposals: %w", err) + } + return out, nil +} + +// MarkProposalMerged transitions a proposal to merged, recording how it was +// authorized (human or policy) and the merge commit. It writes only the row; +// use MergeProposal to update the document registry in the same transaction. +// +// The legality of the transition is enforced in SQL by `WHERE state = 'open'`, +// not by reading the row first: a check-then-write would let two concurrent +// resolutions both pass the check. When the guard bites, the current state is +// read back only to name it in the error. +func (s *Store) MarkProposalMerged(ctx context.Context, id int, approval core.Approval, mergedRev string) error { + if _, err := core.ParseApproval(string(approval)); err != nil { + return err + } + if mergedRev == "" { + return fmt.Errorf("merge proposal %d: merged rev is required", id) + } + return s.resolveProposal(ctx, id, core.StateMerged, string(approval), mergedRev) +} + +// RejectProposal transitions a proposal to rejected. There is no +// request-changes cycle: with one reviewer, a proposal you dislike is rejected +// and the agent proposes again. +func (s *Store) RejectProposal(ctx context.Context, id int) error { + return s.resolveProposal(ctx, id, core.StateRejected, "", "") +} + +// resolveProposal is the shared open->terminal update. next must be a legal +// destination from open; approval and mergedRev are stored as SQL NULL when +// empty, which the ck_proposal_merged constraints require for a rejection. +func (s *Store) resolveProposal(ctx context.Context, id int, next core.ProposalState, approval, mergedRev string) error { + if err := core.StateOpen.CanTransitionTo(next); err != nil { + return err + } + const q = ` +UPDATE proposal +SET state = $2, approval = $3, merged_rev = $4, resolved = $5 +WHERE id = $1 AND state = $6` + res, err := s.q.ExecContext(ctx, q, id, string(next), nullable(approval), + nullable(mergedRev), time.Now().UTC(), string(core.StateOpen)) + if err != nil { + return fmt.Errorf("resolve proposal %d as %s: %w", id, next, err) + } + n, err := res.RowsAffected() + if err != nil { + return fmt.Errorf("resolve proposal %d: rows affected: %w", id, err) + } + if n == 1 { + return nil + } + + // Nothing moved: either the proposal is gone, or it is no longer open. + var current string + err = s.q.QueryRowContext(ctx, `SELECT state FROM proposal WHERE id = $1`, id).Scan(¤t) + if errors.Is(err, sql.ErrNoRows) { + return ErrNotFound + } + if err != nil { + return fmt.Errorf("resolve proposal %d: read current state: %w", id, err) + } + from, err := core.ParseProposalState(current) + if err != nil { + return fmt.Errorf("proposal %d: %w", id, err) + } + if err := from.CanTransitionTo(next); err != nil { + return fmt.Errorf("proposal %d: %w", id, err) + } + // The row is open and the transition is legal, yet the guarded UPDATE + // matched nothing. That cannot happen; refuse rather than report success. + return fmt.Errorf("proposal %d: guarded update matched no row while state is %s", id, from) +} + +// MergeProposal records a merge: the proposal's transition to merged and the +// new registry location of every document it landed, atomically. +// +// Git refs remain the source of truth for whether the merge happened — this row +// is what the reconciler repairs from the ref when a crash lands between the +// two. What the transaction buys is that Postgres never holds the half-state +// where the proposal reads merged but its documents are still registered at +// their pre-merge paths. +func (s *Store) MergeProposal(ctx context.Context, m Merge) error { + if _, err := core.ParseApproval(string(m.Approval)); err != nil { + return err + } + if m.MergedRev == "" { + return fmt.Errorf("merge proposal %d: merged rev is required", m.ProposalID) + } + return s.InTx(ctx, func(tx *Store) error { + if err := tx.MarkProposalMerged(ctx, m.ProposalID, m.Approval, m.MergedRev); err != nil { + return err + } + return tx.UpsertDocIDs(ctx, m.SpaceID, m.Docs, m.MergedRev) + }) +} diff --git a/db/proposal_test.go b/db/proposal_test.go new file mode 100644 index 0000000000000000000000000000000000000000..a34e72fe5337adc596f617b33b0a9fcf0f961c2e --- /dev/null +++ b/db/proposal_test.go @@ -0,0 +1,295 @@ +package db + +import ( + "context" + "errors" + "testing" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +func TestProposalOpenAndRead(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + space := mkSpace(t, s, ctx, "bigbes", "rfcs") + p := mkProposal(t, s, ctx, space.ID, "Add storage model section") + + if p.ID == 0 { + t.Fatal("expected a non-zero proposal id") + } + if p.Branch != ProposalBranch(p.ID) { + t.Fatalf("branch = %q, want %q", p.Branch, ProposalBranch(p.ID)) + } + if p.State != core.StateOpen { + t.Fatalf("state = %q, want open", p.State) + } + if p.Approval != "" || p.MergedRev != "" || p.Resolved != nil { + t.Fatalf("a fresh proposal must not be resolved: %+v", p) + } + + got, err := s.GetProposal(ctx, p.ID) + if err != nil { + t.Fatalf("get proposal: %v", err) + } + if got.Title != p.Title || got.Rationale != "because" || got.BaseRev != p.BaseRev { + t.Fatalf("round-trip mismatch: %+v", got) + } + if got.Agent != "claude-code/spec-writer" || got.AgentSession == "" { + t.Fatalf("provenance lost: %+v", got) + } + if got.Branch != p.Branch || got.State != core.StateOpen { + t.Fatalf("unexpected proposal: %+v", got) + } + + if _, err := s.GetProposal(ctx, 99999); !errors.Is(err, ErrNotFound) { + t.Fatalf("missing proposal = %v, want ErrNotFound", err) + } + + // An empty rationale round-trips as "" rather than surfacing a NULL. + bare, err := s.OpenProposal(ctx, &Proposal{ + SpaceID: space.ID, Title: "no rationale", BaseRev: "abc", + Agent: "a", AgentSession: "s", + }) + if err != nil { + t.Fatalf("open bare proposal: %v", err) + } + got, err = s.GetProposal(ctx, bare.ID) + if err != nil { + t.Fatalf("get bare proposal: %v", err) + } + if got.Rationale != "" { + t.Fatalf("rationale = %q, want empty", got.Rationale) + } +} + +func TestProposalListByState(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + space := mkSpace(t, s, ctx, "bigbes", "rfcs") + first := mkProposal(t, s, ctx, space.ID, "first") + second := mkProposal(t, s, ctx, space.ID, "second") + third := mkProposal(t, s, ctx, space.ID, "third") + + open, err := s.ListProposalsByState(ctx, core.StateOpen, 0) + if err != nil { + t.Fatalf("list open: %v", err) + } + if len(open) != 3 { + t.Fatalf("expected 3 open proposals, got %d", len(open)) + } + // Newest first. + if open[0].ID != third.ID || open[2].ID != first.ID { + t.Fatalf("expected newest first, got %d, %d, %d", open[0].ID, open[1].ID, open[2].ID) + } + + limited, err := s.ListProposalsByState(ctx, core.StateOpen, 2) + if err != nil { + t.Fatalf("list limited: %v", err) + } + if len(limited) != 2 || limited[0].ID != third.ID { + t.Fatalf("unexpected limited list: %+v", limited) + } + + if err := s.RejectProposal(ctx, second.ID); err != nil { + t.Fatalf("reject: %v", err) + } + rejected, err := s.ListProposalsByState(ctx, core.StateRejected, 0) + if err != nil { + t.Fatalf("list rejected: %v", err) + } + if len(rejected) != 1 || rejected[0].ID != second.ID { + t.Fatalf("unexpected rejected list: %+v", rejected) + } + if rejected[0].Resolved == nil { + t.Fatal("a rejected proposal must carry a resolved timestamp") + } + if rejected[0].Approval != "" || rejected[0].MergedRev != "" { + t.Fatalf("a rejection must not record approval or a merged rev: %+v", rejected[0]) + } + merged, err := s.ListProposalsByState(ctx, core.StateMerged, 0) + if err != nil { + t.Fatalf("list merged: %v", err) + } + if len(merged) != 0 { + t.Fatalf("expected no merged proposals, got %d", len(merged)) + } +} + +func TestProposalTransitionsAreGuarded(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + space := mkSpace(t, s, ctx, "bigbes", "rfcs") + + merged := mkProposal(t, s, ctx, space.ID, "to merge") + if err := s.MarkProposalMerged(ctx, merged.ID, core.ApprovalHuman, "deadbeef"); err != nil { + t.Fatalf("merge: %v", err) + } + got, err := s.GetProposal(ctx, merged.ID) + if err != nil { + t.Fatalf("get merged: %v", err) + } + if got.State != core.StateMerged || got.Approval != core.ApprovalHuman || got.MergedRev != "deadbeef" { + t.Fatalf("unexpected merged proposal: %+v", got) + } + if got.Resolved == nil { + t.Fatal("a merged proposal must carry a resolved timestamp") + } + + // merged -> merged and merged -> rejected must both fail. + if err := s.MarkProposalMerged(ctx, merged.ID, core.ApprovalHuman, "cafe"); !errors.Is(err, core.ErrInvalidTransition) { + t.Fatalf("re-merge = %v, want ErrInvalidTransition", err) + } + if err := s.RejectProposal(ctx, merged.ID); !errors.Is(err, core.ErrInvalidTransition) { + t.Fatalf("merged->rejected = %v, want ErrInvalidTransition", err) + } + // The row did not move. + got, err = s.GetProposal(ctx, merged.ID) + if err != nil { + t.Fatalf("get merged: %v", err) + } + if got.State != core.StateMerged || got.MergedRev != "deadbeef" { + t.Fatalf("illegal transition modified the row: %+v", got) + } + + rejected := mkProposal(t, s, ctx, space.ID, "to reject") + if err := s.RejectProposal(ctx, rejected.ID); err != nil { + t.Fatalf("reject: %v", err) + } + if err := s.RejectProposal(ctx, rejected.ID); !errors.Is(err, core.ErrInvalidTransition) { + t.Fatalf("re-reject = %v, want ErrInvalidTransition", err) + } + if err := s.MarkProposalMerged(ctx, rejected.ID, core.ApprovalPolicy, "cafe"); !errors.Is(err, core.ErrInvalidTransition) { + t.Fatalf("rejected->merged = %v, want ErrInvalidTransition", err) + } + + if err := s.RejectProposal(ctx, 99999); !errors.Is(err, ErrNotFound) { + t.Fatalf("resolving a missing proposal = %v, want ErrNotFound", err) + } + if err := s.MarkProposalMerged(ctx, 99999, core.ApprovalHuman, "cafe"); !errors.Is(err, ErrNotFound) { + t.Fatalf("merging a missing proposal = %v, want ErrNotFound", err) + } +} + +// TestMergeProposalIsAtomic is the reason the transaction exists: a merge that +// cannot re-point the registry must not leave the proposal reading merged. +func TestMergeProposalIsAtomic(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + specs := mkSpace(t, s, ctx, "bigbes", "rfcs") + notes := mkSpace(t, s, ctx, "bigbes", "notes") + foreign := docID(t, "NOTE-1") + if _, err := s.RegisterDocID(ctx, notes.ID, DocRef{ID: foreign, Path: "n.md"}, "rev1"); err != nil { + t.Fatalf("register: %v", err) + } + + p := mkProposal(t, s, ctx, specs.ID, "claims a foreign id") + err := s.MergeProposal(ctx, Merge{ + ProposalID: p.ID, + SpaceID: specs.ID, + Approval: core.ApprovalHuman, + MergedRev: "deadbeef", + Docs: []DocRef{ + {ID: docID(t, "SPEC-1"), Path: "a.md"}, + {ID: foreign, Path: "b.md"}, + }, + }) + if !errors.Is(err, ErrDocIDTaken) { + t.Fatalf("merge with a colliding id = %v, want ErrDocIDTaken", err) + } + got, err := s.GetProposal(ctx, p.ID) + if err != nil { + t.Fatalf("get proposal: %v", err) + } + if got.State != core.StateOpen { + t.Fatalf("failed merge left the proposal %s, want open", got.State) + } + // The whole batch rolled back, including the ID that would have applied. + if _, err := s.DocByID(ctx, docID(t, "SPEC-1")); !errors.Is(err, ErrNotFound) { + t.Fatalf("failed merge registered SPEC-1 anyway: %v", err) + } + + // The same merge without the collision commits both halves. + if err := s.MergeProposal(ctx, Merge{ + ProposalID: p.ID, + SpaceID: specs.ID, + Approval: core.ApprovalPolicy, + MergedRev: "deadbeef", + Docs: []DocRef{{ID: docID(t, "SPEC-1"), Path: "a.md"}}, + }); err != nil { + t.Fatalf("merge: %v", err) + } + got, err = s.GetProposal(ctx, p.ID) + if err != nil { + t.Fatalf("get proposal: %v", err) + } + if got.State != core.StateMerged || got.Approval != core.ApprovalPolicy { + t.Fatalf("unexpected merged proposal: %+v", got) + } + doc, err := s.DocByID(ctx, docID(t, "SPEC-1")) + if err != nil { + t.Fatalf("lookup after merge: %v", err) + } + if doc.SpaceID != specs.ID || doc.Path != "a.md" || doc.UpdatedRev != "deadbeef" { + t.Fatalf("unexpected registry row after merge: %+v", doc) + } +} + +// TestProposalCheckConstraints proves the invariants hold against SQL that does +// not go through this package — a stray UPDATE cannot produce a merged row with +// no approval, or an unknown state. +func TestProposalCheckConstraints(t *testing.T) { + s, pool, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + space := mkSpace(t, s, ctx, "bigbes", "rfcs") + p := mkProposal(t, s, ctx, space.ID, "constrained") + + for _, tc := range []struct { + name string + q string + args []any + }{ + {"unknown state", `UPDATE proposal SET state = 'abandoned' WHERE id = $1`, []any{p.ID}}, + {"unknown approval", `UPDATE proposal SET approval = 'vibes' WHERE id = $1`, []any{p.ID}}, + { + "merged without approval", + `UPDATE proposal SET state = 'merged', merged_rev = 'abc', resolved = now() WHERE id = $1`, + []any{p.ID}, + }, + { + "merged without a merged rev", + `UPDATE proposal SET state = 'merged', approval = 'human', resolved = now() WHERE id = $1`, + []any{p.ID}, + }, + { + "open but resolved", + `UPDATE proposal SET resolved = now() WHERE id = $1`, + []any{p.ID}, + }, + { + "approval on an open proposal", + `UPDATE proposal SET approval = 'human' WHERE id = $1`, + []any{p.ID}, + }, + { + "empty provenance", + `UPDATE proposal SET agent = '' WHERE id = $1`, + []any{p.ID}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := pool.ExecContext(ctx, tc.q, tc.args...); err == nil { + t.Fatalf("%s: expected a constraint violation, got none", tc.name) + } + }) + } +} diff --git a/db/space.go b/db/space.go new file mode 100644 index 0000000000000000000000000000000000000000..88a2d882c49a6b97e2dc336acf0ab0e07f0ecc33 --- /dev/null +++ b/db/space.go @@ -0,0 +1,117 @@ +package db + +import ( + "context" + "database/sql" + "errors" + "fmt" + "time" + + "github.com/lib/pq" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// Space is one bare git repo: the unit of ownership, ACL and review policy. +// The repo on disk is the real thing; this row exists so spaces can be listed +// and so index bookkeeping has something to hang off. Ref.Owner never carries +// the leading '~'. +type Space struct { + ID int + Ref core.SpaceRef + Created time.Time +} + +const spaceSelect = `SELECT id, owner, name, created FROM space` + +func scanSpace(sc rowScanner) (*Space, error) { + var sp Space + if err := sc.Scan(&sp.ID, &sp.Ref.Owner, &sp.Ref.Name, &sp.Created); err != nil { + return nil, err + } + return &sp, nil +} + +// CreateSpace inserts a space row. The name is validated with core before it +// reaches SQL: an owner or space name that is not a safe path segment would +// become a directory under the repos root, so it is rejected at every door +// rather than at whichever one happens to check. +// +// A uq_space_owner_name violation maps to ErrSpaceExists. +func (s *Store) CreateSpace(ctx context.Context, ref core.SpaceRef) (*Space, error) { + if err := core.ValidateOwner(ref.Owner); err != nil { + return nil, err + } + if err := core.ValidateSpaceName(ref.Name); err != nil { + return nil, err + } + const q = ` +INSERT INTO space (owner, name, created) +VALUES ($1, $2, $3) +RETURNING id, created` + var sp Space + sp.Ref = ref + err := s.q.QueryRowContext(ctx, q, ref.Owner, ref.Name, time.Now().UTC()). + Scan(&sp.ID, &sp.Created) + if err != nil { + var pqErr *pq.Error + if errors.As(err, &pqErr) && pqErr.Code == "23505" { + return nil, fmt.Errorf("%w: %s", ErrSpaceExists, ref) + } + return nil, fmt.Errorf("insert space %s: %w", ref, err) + } + return &sp, nil +} + +// GetSpace resolves a space by owner and name. Returns ErrNotFound if no such +// space exists. +func (s *Store) GetSpace(ctx context.Context, ref core.SpaceRef) (*Space, error) { + q := spaceSelect + ` WHERE owner = $1 AND name = $2` + sp, err := scanSpace(s.q.QueryRowContext(ctx, q, ref.Owner, ref.Name)) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("get space %s: %w", ref, err) + } + return sp, nil +} + +// GetSpaceByID resolves a space by its primary key — the form every other table +// references it by. Returns ErrNotFound if no such space exists. +func (s *Store) GetSpaceByID(ctx context.Context, id int) (*Space, error) { + q := spaceSelect + ` WHERE id = $1` + sp, err := scanSpace(s.q.QueryRowContext(ctx, q, id)) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("get space %d: %w", id, err) + } + return sp, nil +} + +// ListSpaces returns every space, ordered by owner then name. There is one human +// on this instance and no visibility levels, so there is nothing to filter by: +// the list is the whole corpus, which is also exactly what the meta-project (a +// filter that excludes nothing) needs. +func (s *Store) ListSpaces(ctx context.Context) ([]*Space, error) { + q := spaceSelect + ` ORDER BY owner, name` + rows, err := s.q.QueryContext(ctx, q) + if err != nil { + return nil, fmt.Errorf("list spaces: %w", err) + } + defer rows.Close() + var spaces []*Space + for rows.Next() { + sp, err := scanSpace(rows) + if err != nil { + return nil, fmt.Errorf("scan space: %w", err) + } + spaces = append(spaces, sp) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate spaces: %w", err) + } + return spaces, nil +} diff --git a/db/space_test.go b/db/space_test.go new file mode 100644 index 0000000000000000000000000000000000000000..3f9e331ba05acce6f5a411eeefdbc7c90d9c1307 --- /dev/null +++ b/db/space_test.go @@ -0,0 +1,77 @@ +package db + +import ( + "context" + "errors" + "testing" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +func TestSpaceLifecycle(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + ref := core.SpaceRef{Owner: "bigbes", Name: "rfcs"} + sp, err := s.CreateSpace(ctx, ref) + if err != nil { + t.Fatalf("create space: %v", err) + } + if sp.ID == 0 { + t.Fatal("expected a non-zero space id") + } + if sp.Created.IsZero() { + t.Fatal("expected created to be set") + } + if sp.Ref != ref { + t.Fatalf("ref round-trip: got %v, want %v", sp.Ref, ref) + } + + // (owner, name) is unique. + if _, err := s.CreateSpace(ctx, ref); !errors.Is(err, ErrSpaceExists) { + t.Fatalf("duplicate space = %v, want ErrSpaceExists", err) + } + // Same name under a different owner is fine. + if _, err := s.CreateSpace(ctx, core.SpaceRef{Owner: "someone", Name: "rfcs"}); err != nil { + t.Fatalf("same name, different owner: %v", err) + } + + got, err := s.GetSpace(ctx, ref) + if err != nil { + t.Fatalf("get space: %v", err) + } + if got.ID != sp.ID || got.Ref != ref { + t.Fatalf("get space returned %+v, want id=%d ref=%v", got, sp.ID, ref) + } + byID, err := s.GetSpaceByID(ctx, sp.ID) + if err != nil { + t.Fatalf("get space by id: %v", err) + } + if byID.Ref != ref { + t.Fatalf("get by id returned %v, want %v", byID.Ref, ref) + } + + if _, err := s.GetSpace(ctx, core.SpaceRef{Owner: "bigbes", Name: "absent"}); !errors.Is(err, ErrNotFound) { + t.Fatalf("missing space = %v, want ErrNotFound", err) + } + if _, err := s.GetSpaceByID(ctx, 99999); !errors.Is(err, ErrNotFound) { + t.Fatalf("missing space id = %v, want ErrNotFound", err) + } + + mkSpace(t, s, ctx, "bigbes", "notes") + spaces, err := s.ListSpaces(ctx) + if err != nil { + t.Fatalf("list spaces: %v", err) + } + if len(spaces) != 3 { + t.Fatalf("expected 3 spaces, got %d", len(spaces)) + } + // Ordered by owner then name. + want := []string{"~bigbes/notes", "~bigbes/rfcs", "~someone/rfcs"} + for i, sp := range spaces { + if sp.Ref.String() != want[i] { + t.Errorf("spaces[%d] = %s, want %s", i, sp.Ref, want[i]) + } + } +} diff --git a/db/stamps.go b/db/stamps.go new file mode 100644 index 0000000000000000000000000000000000000000..9eb0fba11ac3164794ab9c9aa2fbca364387bffc --- /dev/null +++ b/db/stamps.go @@ -0,0 +1,101 @@ +package db + +import ( + "context" + "database/sql" + "errors" + "fmt" + "time" +) + +// IndexStamp records which revision of a space's approved branch the global +// bleve index currently reflects. +// +// It is the reconciler's staleness comparison, and it is why a crash between +// merge and reindex is a defined repair rather than a bespoke one: stamp rev +// != approved head means reindex. The index is a pure cache, so a missing or +// wrong stamp costs a rebuild, never data. +type IndexStamp struct { + SpaceID int + Rev string + IndexedAt time.Time +} + +// GetIndexStamp returns the space's index stamp. Returns ErrNotFound when the +// space has never been indexed — which the reconciler must treat as "stale", +// not as "up to date"; that is exactly why this is an error rather than a zero +// value. +func (s *Store) GetIndexStamp(ctx context.Context, spaceID int) (*IndexStamp, error) { + const q = `SELECT space_id, rev, indexed_at FROM index_stamp WHERE space_id = $1` + var st IndexStamp + err := s.q.QueryRowContext(ctx, q, spaceID).Scan(&st.SpaceID, &st.Rev, &st.IndexedAt) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("get index stamp space=%d: %w", spaceID, err) + } + return &st, nil +} + +// SetIndexStamp records that the index now reflects rev for this space. Called +// after a rebuild completes, never before: a stamp written ahead of the rebuild +// would make a crash look like a fresh index. +func (s *Store) SetIndexStamp(ctx context.Context, spaceID int, rev string) (*IndexStamp, error) { + if rev == "" { + return nil, fmt.Errorf("set index stamp space=%d: rev is required", spaceID) + } + const q = ` +INSERT INTO index_stamp (space_id, rev, indexed_at) +VALUES ($1, $2, $3) +ON CONFLICT (space_id) DO UPDATE + SET rev = EXCLUDED.rev, indexed_at = EXCLUDED.indexed_at +RETURNING space_id, rev, indexed_at` + var st IndexStamp + err := s.q.QueryRowContext(ctx, q, spaceID, rev, time.Now().UTC()). + Scan(&st.SpaceID, &st.Rev, &st.IndexedAt) + if err != nil { + return nil, fmt.Errorf("set index stamp space=%d: %w", spaceID, err) + } + return &st, nil +} + +// GetDigestMark returns when the owner last looked at the digest of +// policy-merged content. Returns ErrNotFound if they never have. +// +// Auto-merged content that never appears in any view is write-only and rots +// invisibly — the exact failure this service exists to prevent, just relocated. +// This one timestamp is the whole of "what landed since you last looked". +func (s *Store) GetDigestMark(ctx context.Context, owner string) (time.Time, error) { + const q = `SELECT seen_at FROM digest_mark WHERE owner = $1` + var seen time.Time + err := s.q.QueryRowContext(ctx, q, owner).Scan(&seen) + if errors.Is(err, sql.ErrNoRows) { + return time.Time{}, ErrNotFound + } + if err != nil { + return time.Time{}, fmt.Errorf("get digest mark %q: %w", owner, err) + } + return seen, nil +} + +// SetDigestMark moves the owner's "last looked at" timestamp. seenAt is passed +// in rather than defaulted to now() so the caller can mark the digest as of the +// moment it rendered the page, not the moment the write happened — anything +// that lands in between must still show up next time. +func (s *Store) SetDigestMark(ctx context.Context, owner string, seenAt time.Time) error { + if owner == "" { + return fmt.Errorf("set digest mark: owner is required") + } + if seenAt.IsZero() { + return fmt.Errorf("set digest mark %q: seen_at is required", owner) + } + const q = ` +INSERT INTO digest_mark (owner, seen_at) +VALUES ($1, $2) +ON CONFLICT (owner) DO UPDATE SET seen_at = EXCLUDED.seen_at` + if _, err := s.q.ExecContext(ctx, q, owner, seenAt.UTC()); err != nil { + return fmt.Errorf("set digest mark %q: %w", owner, err) + } + return nil +} diff --git a/db/stamps_test.go b/db/stamps_test.go new file mode 100644 index 0000000000000000000000000000000000000000..db0fc212954d17ef819f59190550bf8d151af5d2 --- /dev/null +++ b/db/stamps_test.go @@ -0,0 +1,97 @@ +package db + +import ( + "context" + "errors" + "testing" + "time" +) + +func TestIndexStamp(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + space := mkSpace(t, s, ctx, "bigbes", "rfcs") + + // Never indexed is ErrNotFound, not a zero rev: the reconciler must read it + // as stale. + if _, err := s.GetIndexStamp(ctx, space.ID); !errors.Is(err, ErrNotFound) { + t.Fatalf("unindexed space = %v, want ErrNotFound", err) + } + + st, err := s.SetIndexStamp(ctx, space.ID, "rev1") + if err != nil { + t.Fatalf("set stamp: %v", err) + } + if st.SpaceID != space.ID || st.Rev != "rev1" || st.IndexedAt.IsZero() { + t.Fatalf("unexpected stamp: %+v", st) + } + + got, err := s.GetIndexStamp(ctx, space.ID) + if err != nil { + t.Fatalf("get stamp: %v", err) + } + if got.Rev != "rev1" { + t.Fatalf("rev = %q, want rev1", got.Rev) + } + + // Re-stamping the same space updates in place. + if _, err := s.SetIndexStamp(ctx, space.ID, "rev2"); err != nil { + t.Fatalf("re-stamp: %v", err) + } + got, err = s.GetIndexStamp(ctx, space.ID) + if err != nil { + t.Fatalf("get stamp: %v", err) + } + if got.Rev != "rev2" { + t.Fatalf("rev = %q, want rev2", got.Rev) + } + if !got.IndexedAt.After(st.IndexedAt) && !got.IndexedAt.Equal(st.IndexedAt) { + t.Fatalf("indexed_at went backwards: %v -> %v", st.IndexedAt, got.IndexedAt) + } + + // The stamp is per space and cascades with it. + other := mkSpace(t, s, ctx, "bigbes", "notes") + if _, err := s.GetIndexStamp(ctx, other.ID); !errors.Is(err, ErrNotFound) { + t.Fatalf("other space stamp = %v, want ErrNotFound", err) + } +} + +func TestDigestMark(t *testing.T) { + s, _, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + if _, err := s.GetDigestMark(ctx, "bigbes"); !errors.Is(err, ErrNotFound) { + t.Fatalf("never looked = %v, want ErrNotFound", err) + } + + first := time.Now().UTC().Add(-time.Hour).Truncate(time.Microsecond) + if err := s.SetDigestMark(ctx, "bigbes", first); err != nil { + t.Fatalf("set mark: %v", err) + } + got, err := s.GetDigestMark(ctx, "bigbes") + if err != nil { + t.Fatalf("get mark: %v", err) + } + if !got.Equal(first) { + t.Fatalf("mark = %v, want %v", got, first) + } + + second := first.Add(30 * time.Minute) + if err := s.SetDigestMark(ctx, "bigbes", second); err != nil { + t.Fatalf("move mark: %v", err) + } + got, err = s.GetDigestMark(ctx, "bigbes") + if err != nil { + t.Fatalf("get mark: %v", err) + } + if !got.Equal(second) { + t.Fatalf("mark = %v, want %v", got, second) + } + + if _, err := s.GetDigestMark(ctx, "someone"); !errors.Is(err, ErrNotFound) { + t.Fatalf("other owner = %v, want ErrNotFound", err) + } +} diff --git a/db/store.go b/db/store.go new file mode 100644 index 0000000000000000000000000000000000000000..207b641fd9590682869b73128d27f9313d0fce8a --- /dev/null +++ b/db/store.go @@ -0,0 +1,178 @@ +// Package db is the PostgreSQL persistence layer for spec.sr.ht. It maps the +// six tables of schema.sql — space, document_id, proposal, agent_token, +// index_stamp and digest_mark — to core value types with plain database/sql and +// $n placeholders (no ORM). +// +// The layering rule from the design is what shapes this package: **git refs are +// the source of truth for whether a proposal exists and whether it merged; +// Postgres holds metadata that is reconstructable from git; the index and the +// render cache are pure caches.** So nothing here stores a document body, and +// every row is something the reconciler could rebuild from refs. The queries +// are correspondingly small: registry lookups, one state machine, and two +// key/value stamps. +// +// Design: a Store wraps a Querier — an interface satisfied by *sql.DB, *sql.Tx +// and *sql.Conn alike. This gives us two things at once: +// +// - Context-first, middleware-compatible signatures. Production callers build +// a Store from the connection that core-go's database middleware injects +// into the request context (see FromContext, which reads the same *sql.DB +// that database.Middleware installed). Every method takes ctx first and +// threads it into the query for cancellation. +// +// - Trivial test injection. Tests call NewStore(db) with a plain *sql.DB, no +// HTTP context required. +// +// Because the wrapped value is an interface, a Store can be re-bound to an open +// transaction with WithTx(tx) or InTx(ctx, fn). That is what MergeProposal +// needs: recording the merge and re-pointing the affected document_id rows are +// one invariant, not two writes that may half-happen. +package db + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "sourcecraft.dev/bigbes/sr-ht-core/database" +) + +// Querier is the common subset of *sql.DB, *sql.Tx and *sql.Conn used by this +// package. Binding a Store to any of them keeps every method identical whether +// it runs standalone (autocommit) or inside a caller-managed transaction. +type Querier interface { + ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error) + QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error) + QueryRowContext(ctx context.Context, query string, args ...any) *sql.Row +} + +// beginner is the subset of *sql.DB that can open a transaction. A Store bound +// to a *sql.Tx does not satisfy it, which is how InTx refuses to nest instead of +// silently running the body outside a transaction. +type beginner interface { + BeginTx(ctx context.Context, opts *sql.TxOptions) (*sql.Tx, error) +} + +// Store is the entry point for all queries in this package. +type Store struct { + q Querier +} + +// NewStore builds a Store over a database handle (or any Querier). Tests pass a +// plain *sql.DB; production wiring passes the shared pool. +func NewStore(q Querier) *Store { + return &Store{q: q} +} + +// FromContext builds a Store over the *sql.DB that core-go's database.Middleware +// installed in ctx. It panics (via database.DBForContext) if no database is +// present in the context — a programming error, never a runtime condition to +// recover from. We wrap the pooled *sql.DB rather than checking out a *sql.Conn +// (database.ForContext) so the Store has no connection to leak; the pool manages +// connection lifetime and ctx still bounds each query. +func FromContext(ctx context.Context) *Store { + return &Store{q: database.DBForContext(ctx)} +} + +// WithTx returns a Store that runs every query on tx instead of the pool. Used +// where the caller already owns a transaction and wants these queries inside it. +func (s *Store) WithTx(tx *sql.Tx) *Store { + return &Store{q: tx} +} + +// InTx runs fn inside a transaction, on a Store bound to it. fn returning an +// error rolls back and the error is returned unwrapped, so callers can still +// match sentinels with errors.Is. A panic in fn rolls back and re-panics rather +// than leaving the transaction open. +// +// It requires the Store to wrap something that can begin a transaction (the +// pool). A Store already bound to a *sql.Tx returns ErrNoTransaction: nesting is +// a caller bug, and quietly running the body without transactional isolation +// would defeat the only reason this method exists. +func (s *Store) InTx(ctx context.Context, fn func(*Store) error) error { + b, ok := s.q.(beginner) + if !ok { + return ErrNoTransaction + } + tx, err := b.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("begin transaction: %w", err) + } + committed := false + defer func() { + if !committed { + tx.Rollback() + } + }() + if err := fn(&Store{q: tx}); err != nil { + return err + } + if err := tx.Commit(); err != nil { + return fmt.Errorf("commit transaction: %w", err) + } + committed = true + return nil +} + +// Typed errors returned by this package. Callers match them with errors.Is. +var ( + // ErrNotFound is returned when a lookup, update or delete matched no row. + ErrNotFound = errors.New("db: not found") + + // ErrSpaceExists is returned by CreateSpace when the owner already has a + // space with that name (uq_space_owner_name violation). + ErrSpaceExists = errors.New("db: space already exists") + + // ErrDocIDTaken is returned when a document ID is already registered. + // Document IDs are globally unique, so this is the registry refusing a + // collision — the invariant that lets [[SPEC-0007]] resolve the same way + // everywhere and lets a later import not clash with what is already here. + ErrDocIDTaken = errors.New("db: document id already registered") + + // ErrDocIDDuplicate is returned when one batch of documents carries the + // same ID twice. Distinct from ErrDocIDTaken: the collision is inside the + // push itself, not against the registry. + ErrDocIDDuplicate = errors.New("db: document id appears twice in one batch") + + // ErrTokenExists is returned by CreateAgentToken when that exact token is + // already registered (agent_token.token_hash UNIQUE). + ErrTokenExists = errors.New("db: agent token already registered") + + // ErrTokenRevoked is returned by AuthenticateAgentToken for a token that + // exists but has been revoked. Kept distinct from ErrNotFound so the audit + // log can say which happened; both map to 401 at the API boundary. + ErrTokenRevoked = errors.New("db: agent token revoked") + + // ErrNoTransaction is returned by InTx when the Store is not bound to + // something that can begin one (i.e. it is already inside a transaction). + ErrNoTransaction = errors.New("db: store cannot begin a transaction") +) + +// rowScanner is satisfied by both *sql.Row and *sql.Rows. +type rowScanner interface { + Scan(dest ...any) error +} + +// requireOne turns a zero-rows-affected result into ErrNotFound so that missing +// targets surface loudly instead of passing silently. +func requireOne(res sql.Result, what string) error { + n, err := res.RowsAffected() + if err != nil { + return fmt.Errorf("%s: rows affected: %w", what, err) + } + if n == 0 { + return ErrNotFound + } + return nil +} + +// nullable maps the empty string to a SQL NULL, for the columns the schema +// leaves nullable (proposal.rationale). Storing "" and NULL interchangeably +// would make round-tripping lossy. +func nullable(s string) any { + if s == "" { + return nil + } + return s +} diff --git a/db/token.go b/db/token.go new file mode 100644 index 0000000000000000000000000000000000000000..805a3af70a173c1ef4b73401826dee365185cefb --- /dev/null +++ b/db/token.go @@ -0,0 +1,201 @@ +package db + +import ( + "context" + "crypto/rand" + "crypto/sha256" + "crypto/subtle" + "database/sql" + "encoding/base64" + "errors" + "fmt" + "time" + + "github.com/lib/pq" +) + +// TokenBytes is the entropy of a minted agent token before encoding. 32 bytes +// is well past any brute-force concern and keeps the encoded form short enough +// to paste into an agent's environment. +const TokenBytes = 32 + +// AgentToken is a credential an agent presents on the write plane. The token +// value itself is never stored — only Hash — so a database dump cannot be +// replayed against the service. +// +// v1 ships one token plus mandatory provenance rather than per-agent scopes: +// the refs rule (an agent credential can only move refs under BranchPrefix) is +// the boundary that actually bounds the damage, and it holds with a single +// shared token. Per-space scoping is a later column on this row plus a filter +// clause, not an architectural change. +type AgentToken struct { + ID int + Name string + Hash []byte + Created time.Time + Revoked *time.Time +} + +// Active reports whether the token may still authenticate. +func (t *AgentToken) Active() bool { return t.Revoked == nil } + +// GenerateToken mints a fresh token value: TokenBytes of crypto/rand, URL-safe +// base64 without padding. This is the only moment the plaintext exists; the +// caller shows it to the operator once and stores only HashToken(it). +func GenerateToken() (string, error) { + b := make([]byte, TokenBytes) + if _, err := rand.Read(b); err != nil { + return "", fmt.Errorf("generate agent token: %w", err) + } + return base64.RawURLEncoding.EncodeToString(b), nil +} + +// HashToken is the one-way function between a presented token and the stored +// agent_token.token_hash. SHA-256 is the right tool here rather than a password +// KDF: the input is 256 bits of uniform randomness we generated, not a +// human-chosen secret, so there is no dictionary to stretch against. +func HashToken(token string) []byte { + sum := sha256.Sum256([]byte(token)) + return sum[:] +} + +// TokenMatches compares a stored hash with the hash of a presented token in +// constant time. Byte-wise early exit on a hash comparison leaks how many +// leading bytes an attacker guessed right, which is enough to walk a forged +// value into place one byte at a time; subtle.ConstantTimeCompare does not. +func TokenMatches(stored []byte, token string) bool { + return subtle.ConstantTimeCompare(stored, HashToken(token)) == 1 +} + +// CreateAgentToken stores a token by hash and returns the row. The plaintext is +// never passed to this function and never reaches SQL — callers hash with +// HashToken and keep the value only long enough to show it once. +// +// A token_hash UNIQUE violation means the same token was registered twice — +// for 32 random bytes, that is a caller re-registering a value it already had, +// not a collision — and is mapped to ErrTokenExists. +func (s *Store) CreateAgentToken(ctx context.Context, name string, hash []byte) (*AgentToken, error) { + if name == "" { + return nil, fmt.Errorf("create agent token: name is required") + } + if len(hash) != sha256.Size { + return nil, fmt.Errorf("create agent token: hash must be %d bytes, got %d", + sha256.Size, len(hash)) + } + const q = ` +INSERT INTO agent_token (name, token_hash, created) +VALUES ($1, $2, $3) +RETURNING id, created` + t := AgentToken{Name: name, Hash: hash} + err := s.q.QueryRowContext(ctx, q, name, hash, time.Now().UTC()).Scan(&t.ID, &t.Created) + if err != nil { + var pqErr *pq.Error + if errors.As(err, &pqErr) && pqErr.Code == "23505" { + return nil, ErrTokenExists + } + return nil, fmt.Errorf("create agent token %q: %w", name, err) + } + return &t, nil +} + +// AgentTokenByHash looks a token up by its stored hash. It does not consider +// revocation — use AuthenticateAgentToken for the authorization decision. +// Returns ErrNotFound if no such token is registered. +func (s *Store) AgentTokenByHash(ctx context.Context, hash []byte) (*AgentToken, error) { + const q = ` +SELECT id, name, token_hash, created, revoked +FROM agent_token +WHERE token_hash = $1` + var ( + t AgentToken + revoked sql.NullTime + ) + err := s.q.QueryRowContext(ctx, q, hash).Scan(&t.ID, &t.Name, &t.Hash, &t.Created, &revoked) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("agent token by hash: %w", err) + } + if revoked.Valid { + r := revoked.Time + t.Revoked = &r + } + return &t, nil +} + +// AuthenticateAgentToken is the authorization boundary: it hashes the presented +// token, looks the row up by that hash, re-verifies the stored hash against the +// presentation in constant time, and rejects a revoked token. +// +// The re-verification is not redundant with the SQL equality. The index lookup +// is what finds the row; TokenMatches is what decides, and it is the one +// comparison an attacker can time. Returns ErrNotFound for an unknown token and +// ErrTokenRevoked for a known but revoked one; both are 401 at the API edge. +func (s *Store) AuthenticateAgentToken(ctx context.Context, token string) (*AgentToken, error) { + if token == "" { + return nil, ErrNotFound + } + t, err := s.AgentTokenByHash(ctx, HashToken(token)) + if err != nil { + return nil, err + } + if !TokenMatches(t.Hash, token) { + // The row was found by hash equality, so a mismatch here means the + // stored hash is not what the index matched on — corruption, not a bad + // credential. + return nil, fmt.Errorf("agent token %d: stored hash does not verify", t.ID) + } + if !t.Active() { + return nil, fmt.Errorf("%w: token %q revoked at %s", ErrTokenRevoked, t.Name, t.Revoked) + } + return t, nil +} + +// RevokeAgentToken stamps a token revoked. Revocation is a stamp rather than a +// delete so the audit trail keeps naming the token that made past proposals. +// Revoking an already-revoked token is a no-op that returns nil; re-revoking is +// not an error worth failing an operator over. Returns ErrNotFound if id does +// not exist. +func (s *Store) RevokeAgentToken(ctx context.Context, id int) error { + const q = `UPDATE agent_token SET revoked = COALESCE(revoked, $2) WHERE id = $1` + res, err := s.q.ExecContext(ctx, q, id, time.Now().UTC()) + if err != nil { + return fmt.Errorf("revoke agent token %d: %w", id, err) + } + return requireOne(res, "revoke agent token") +} + +// ListAgentTokens returns every token, newest first, so the operator can see +// what exists and pick one to revoke. Hashes are included; there is nothing +// secret about them and the reconciler-style tooling compares by them. +func (s *Store) ListAgentTokens(ctx context.Context) ([]*AgentToken, error) { + const q = ` +SELECT id, name, token_hash, created, revoked +FROM agent_token +ORDER BY created DESC, id DESC` + rows, err := s.q.QueryContext(ctx, q) + if err != nil { + return nil, fmt.Errorf("list agent tokens: %w", err) + } + defer rows.Close() + var out []*AgentToken + for rows.Next() { + var ( + t AgentToken + revoked sql.NullTime + ) + if err := rows.Scan(&t.ID, &t.Name, &t.Hash, &t.Created, &revoked); err != nil { + return nil, fmt.Errorf("scan agent token: %w", err) + } + if revoked.Valid { + r := revoked.Time + t.Revoked = &r + } + out = append(out, &t) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("iterate agent tokens: %w", err) + } + return out, nil +} diff --git a/db/token_test.go b/db/token_test.go new file mode 100644 index 0000000000000000000000000000000000000000..3c3570fd0df1e0c0f25818e23fb73a90cea41a25 --- /dev/null +++ b/db/token_test.go @@ -0,0 +1,118 @@ +package db + +import ( + "bytes" + "context" + "errors" + "testing" +) + +func TestAgentTokenLifecycle(t *testing.T) { + s, pool, cleanup := newTestStore(t) + defer cleanup() + ctx := context.Background() + + token, err := GenerateToken() + if err != nil { + t.Fatalf("generate token: %v", err) + } + hash := HashToken(token) + + created, err := s.CreateAgentToken(ctx, "spec-writer", hash) + if err != nil { + t.Fatalf("create token: %v", err) + } + if created.ID == 0 || !created.Active() { + t.Fatalf("unexpected created token: %+v", created) + } + + // The plaintext must never have reached the database. + var found int + if err := pool.QueryRowContext(ctx, + `SELECT count(*) FROM agent_token WHERE encode(token_hash, 'escape') LIKE '%' || $1 || '%'`, + token).Scan(&found); err != nil { + t.Fatalf("scan for plaintext: %v", err) + } + if found != 0 { + t.Fatal("the token plaintext is recoverable from the row") + } + + if _, err := s.CreateAgentToken(ctx, "duplicate", hash); !errors.Is(err, ErrTokenExists) { + t.Fatalf("duplicate token = %v, want ErrTokenExists", err) + } + + byHash, err := s.AgentTokenByHash(ctx, hash) + if err != nil { + t.Fatalf("lookup by hash: %v", err) + } + if byHash.ID != created.ID || byHash.Name != "spec-writer" || !bytes.Equal(byHash.Hash, hash) { + t.Fatalf("unexpected lookup: %+v", byHash) + } + if _, err := s.AgentTokenByHash(ctx, HashToken("nope")); !errors.Is(err, ErrNotFound) { + t.Fatalf("unknown hash = %v, want ErrNotFound", err) + } + + auth, err := s.AuthenticateAgentToken(ctx, token) + if err != nil { + t.Fatalf("authenticate: %v", err) + } + if auth.ID != created.ID { + t.Fatalf("authenticated the wrong token: %+v", auth) + } + if _, err := s.AuthenticateAgentToken(ctx, token+"x"); !errors.Is(err, ErrNotFound) { + t.Fatalf("wrong token = %v, want ErrNotFound", err) + } + if _, err := s.AuthenticateAgentToken(ctx, ""); !errors.Is(err, ErrNotFound) { + t.Fatalf("empty token = %v, want ErrNotFound", err) + } + + // Revocation is a stamp, so the row stays for the audit trail. + if err := s.RevokeAgentToken(ctx, created.ID); err != nil { + t.Fatalf("revoke: %v", err) + } + if _, err := s.AuthenticateAgentToken(ctx, token); !errors.Is(err, ErrTokenRevoked) { + t.Fatalf("revoked token = %v, want ErrTokenRevoked", err) + } + revoked, err := s.AgentTokenByHash(ctx, hash) + if err != nil { + t.Fatalf("lookup revoked: %v", err) + } + if revoked.Active() || revoked.Revoked == nil { + t.Fatalf("revocation did not stick: %+v", revoked) + } + first := *revoked.Revoked + + // Re-revoking keeps the original timestamp rather than moving it. + if err := s.RevokeAgentToken(ctx, created.ID); err != nil { + t.Fatalf("re-revoke: %v", err) + } + again, err := s.AgentTokenByHash(ctx, hash) + if err != nil { + t.Fatalf("lookup revoked: %v", err) + } + if !again.Revoked.Equal(first) { + t.Fatalf("re-revoking moved the timestamp: %v -> %v", first, *again.Revoked) + } + + if err := s.RevokeAgentToken(ctx, 99999); !errors.Is(err, ErrNotFound) { + t.Fatalf("revoking a missing token = %v, want ErrNotFound", err) + } + + second, err := GenerateToken() + if err != nil { + t.Fatalf("generate token: %v", err) + } + if _, err := s.CreateAgentToken(ctx, "notes-writer", HashToken(second)); err != nil { + t.Fatalf("create second token: %v", err) + } + list, err := s.ListAgentTokens(ctx) + if err != nil { + t.Fatalf("list tokens: %v", err) + } + if len(list) != 2 || list[0].Name != "notes-writer" { + t.Fatalf("unexpected token list (newest first): %+v", list) + } + if _, err := s.AuthenticateAgentToken(ctx, second); err != nil { + t.Fatalf("authenticate second token: %v", err) + } +} diff --git a/db/unit_test.go b/db/unit_test.go new file mode 100644 index 0000000000000000000000000000000000000000..586576b9c1f46a7e856b3d4161a918ad914293e0 --- /dev/null +++ b/db/unit_test.go @@ -0,0 +1,381 @@ +package db + +import ( + "context" + "crypto/sha256" + "database/sql" + "errors" + "os" + "regexp" + "strings" + "testing" + "time" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// These tests need no database. They cover the logic that decides things — +// transition legality, token comparison, batch duplicate detection — plus the +// agreement between schema.sql and the migration, which is otherwise only +// discovered on a fresh install. + +func TestProposalBranch(t *testing.T) { + for _, tc := range []struct { + id int + want string + }{ + {1, "proposals/1"}, + {42, "proposals/42"}, + {1000000, "proposals/1000000"}, + } { + if got := ProposalBranch(tc.id); got != tc.want { + t.Errorf("ProposalBranch(%d) = %q, want %q", tc.id, got, tc.want) + } + } + if !strings.HasPrefix(ProposalBranch(7), BranchPrefix) { + t.Errorf("ProposalBranch must stay under the refs-rule prefix %q", BranchPrefix) + } +} + +func TestHashTokenAndMatches(t *testing.T) { + tok, err := GenerateToken() + if err != nil { + t.Fatalf("generate token: %v", err) + } + if len(tok) < 40 { + t.Fatalf("token %q is implausibly short for %d bytes of entropy", tok, TokenBytes) + } + hash := HashToken(tok) + if len(hash) != sha256.Size { + t.Fatalf("HashToken returned %d bytes, want %d", len(hash), sha256.Size) + } + if strings.Contains(string(hash), tok) { + t.Fatal("hash must not contain the token") + } + if !TokenMatches(hash, tok) { + t.Fatal("TokenMatches rejected the token it hashed") + } + if TokenMatches(hash, tok+"x") { + t.Fatal("TokenMatches accepted a different token") + } + if TokenMatches(hash, "") { + t.Fatal("TokenMatches accepted the empty token") + } + if TokenMatches(nil, tok) { + t.Fatal("TokenMatches accepted a nil stored hash") + } + if TokenMatches(hash[:16], tok) { + t.Fatal("TokenMatches accepted a truncated stored hash") + } + + // Two mints must differ; a repeated value would mean the generator is not + // actually random and every token would be the same credential. + other, err := GenerateToken() + if err != nil { + t.Fatalf("generate token: %v", err) + } + if other == tok { + t.Fatal("GenerateToken returned the same value twice") + } +} + +func TestDuplicateDocIDs(t *testing.T) { + ref := func(id, path string) DocRef { + return DocRef{ID: docID(t, id), Path: path} + } + for _, tc := range []struct { + name string + in []DocRef + want []string + }{ + {"empty", nil, nil}, + {"unique", []DocRef{ref("SPEC-1", "a.md"), ref("SPEC-2", "b.md")}, nil}, + { + "same id twice at different paths", + []DocRef{ref("SPEC-1", "a.md"), ref("SPEC-1", "b.md")}, + []string{"SPEC-1"}, + }, + { + "leading zeros are a different id", + []DocRef{ref("SPEC-7", "a.md"), ref("SPEC-0007", "b.md")}, + nil, + }, + { + "several duplicates, sorted", + []DocRef{ + ref("SPEC-2", "a.md"), ref("SPEC-1", "b.md"), + ref("SPEC-2", "c.md"), ref("SPEC-1", "d.md"), + }, + []string{"SPEC-1", "SPEC-2"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + got := DuplicateDocIDs(tc.in) + if len(got) != len(tc.want) { + t.Fatalf("DuplicateDocIDs = %v, want %v", got, tc.want) + } + for i, id := range got { + if id.String() != tc.want[i] { + t.Fatalf("DuplicateDocIDs[%d] = %s, want %s", i, id, tc.want[i]) + } + } + }) + } +} + +func TestCollisionErrorWrapsErrDocIDTaken(t *testing.T) { + err := &CollisionError{Collisions: []Collision{{ + DocID: docID(t, "SPEC-7"), + Existing: &Document{ID: docID(t, "SPEC-7"), SpaceID: 3, Path: "specs/0007.md"}, + }}} + if !errors.Is(err, ErrDocIDTaken) { + t.Fatal("CollisionError must match ErrDocIDTaken") + } + msg := err.Error() + for _, want := range []string{"SPEC-7", "space 3", "specs/0007.md"} { + if !strings.Contains(msg, want) { + t.Errorf("collision message %q does not name %q", msg, want) + } + } +} + +// fakeQuerier satisfies Querier but not beginner, standing in for a Store that +// is already bound to a transaction. +type fakeQuerier struct{} + +func (fakeQuerier) ExecContext(context.Context, string, ...any) (sql.Result, error) { + return nil, errors.New("unexpected Exec") +} +func (fakeQuerier) QueryContext(context.Context, string, ...any) (*sql.Rows, error) { + return nil, errors.New("unexpected Query") +} +func (fakeQuerier) QueryRowContext(context.Context, string, ...any) *sql.Row { return nil } + +func TestInTxRefusesToNest(t *testing.T) { + s := NewStore(fakeQuerier{}) + called := false + err := s.InTx(context.Background(), func(*Store) error { + called = true + return nil + }) + if !errors.Is(err, ErrNoTransaction) { + t.Fatalf("InTx on a non-beginner = %v, want ErrNoTransaction", err) + } + if called { + t.Fatal("InTx ran the body without a transaction") + } +} + +// TestResolveArgumentsValidated covers the guards that reject before any SQL is +// issued; the fake Querier would error if a query were attempted. +func TestResolveArgumentsValidated(t *testing.T) { + s := NewStore(fakeQuerier{}) + ctx := context.Background() + + if err := s.MarkProposalMerged(ctx, 1, core.Approval("bogus"), "abc"); !errors.Is(err, core.ErrInvalidApproval) { + t.Fatalf("merge with a bogus approval = %v, want ErrInvalidApproval", err) + } + if err := s.MarkProposalMerged(ctx, 1, core.ApprovalHuman, ""); err == nil { + t.Fatal("merge without a merged rev must fail") + } + if err := s.MergeProposal(ctx, Merge{ProposalID: 1, Approval: core.ApprovalPolicy}); err == nil { + t.Fatal("MergeProposal without a merged rev must fail") + } + if _, err := s.ListProposalsByState(ctx, core.ProposalState("closed"), 0); !errors.Is(err, core.ErrInvalidState) { + t.Fatalf("list with a bogus state = %v, want ErrInvalidState", err) + } + if _, err := s.OpenProposal(ctx, &Proposal{SpaceID: 1, Title: "t", BaseRev: "abc"}); err == nil { + t.Fatal("OpenProposal without provenance must fail") + } + if _, err := s.OpenProposal(ctx, &Proposal{ + SpaceID: 1, Title: "t", Agent: "a", AgentSession: "s", + }); err == nil { + t.Fatal("OpenProposal without a base rev must fail") + } + if _, err := s.CreateSpace(ctx, core.SpaceRef{Owner: "bigbes", Name: "../etc"}); !errors.Is(err, core.ErrInvalidName) { + t.Fatalf("CreateSpace with a traversing name = %v, want ErrInvalidName", err) + } + if _, err := s.CreateAgentToken(ctx, "", HashToken("x")); err == nil { + t.Fatal("CreateAgentToken without a name must fail") + } + if _, err := s.CreateAgentToken(ctx, "ci", []byte("short")); err == nil { + t.Fatal("CreateAgentToken with a non-sha256 hash must fail") + } + if _, err := s.AuthenticateAgentToken(ctx, ""); !errors.Is(err, ErrNotFound) { + t.Fatalf("authenticating an empty token = %v, want ErrNotFound", err) + } + if err := s.SetDigestMark(ctx, "", time.Now()); err == nil { + t.Fatal("SetDigestMark without an owner must fail") + } + if err := s.SetDigestMark(ctx, "bigbes", time.Time{}); err == nil { + t.Fatal("SetDigestMark with a zero timestamp must fail") + } + if _, err := s.SetIndexStamp(ctx, 1, ""); err == nil { + t.Fatal("SetIndexStamp without a rev must fail") + } + if err := s.UpsertDocIDs(ctx, 1, []DocRef{ + {ID: docID(t, "SPEC-1"), Path: "a.md"}, + {ID: docID(t, "SPEC-1"), Path: "b.md"}, + }, "abc"); !errors.Is(err, ErrDocIDDuplicate) { + t.Fatalf("upsert with an in-batch duplicate = %v, want ErrDocIDDuplicate", err) + } + if err := s.UpsertDocIDs(ctx, 1, []DocRef{ + {ID: docID(t, "SPEC-1"), Path: "../escape.md"}, + }, "abc"); !errors.Is(err, core.ErrInvalidPath) { + t.Fatalf("upsert with a traversing path = %v, want ErrInvalidPath", err) + } + // An empty batch is a legal no-op and must not reach SQL. + if err := s.UpsertDocIDs(ctx, 1, nil, "abc"); err != nil { + t.Fatalf("empty upsert = %v, want nil", err) + } + if c, err := s.CheckDocIDCollisions(ctx, 1, nil); err != nil || c != nil { + t.Fatalf("empty collision check = %v, %v; want nil, nil", c, err) + } +} + +// TestTransitionGuardMatchesCore pins the guard this package relies on: the +// UPDATE ... WHERE state = 'open' clause is only correct because open is the +// sole state anything may leave. +func TestTransitionGuardMatchesCore(t *testing.T) { + legal := map[[2]core.ProposalState]bool{ + {core.StateOpen, core.StateMerged}: true, + {core.StateOpen, core.StateRejected}: true, + } + for _, from := range core.ProposalStates() { + for _, to := range core.ProposalStates() { + want := legal[[2]core.ProposalState{from, to}] + if got := core.ValidTransition(from, to); got != want { + t.Errorf("ValidTransition(%s, %s) = %v, want %v", from, to, got, want) + } + } + } + if err := core.StateOpen.CanTransitionTo(core.StateOpen); !errors.Is(err, core.ErrInvalidTransition) { + t.Fatalf("open->open = %v, want ErrInvalidTransition", err) + } + if err := core.StateMerged.CanTransitionTo(core.StateRejected); !errors.Is(err, core.ErrInvalidTransition) { + t.Fatalf("merged->rejected = %v, want ErrInvalidTransition", err) + } +} + +func TestNullable(t *testing.T) { + if nullable("") != nil { + t.Error("empty string must become SQL NULL") + } + if nullable("x") != any("x") { + t.Error("non-empty string must pass through") + } +} + +// TestSchemaMatchesMigration is the check a fresh install would otherwise fail: +// schema.sql is the authoritative DDL, migrations/0001_initial.sql builds the +// same objects, and the two drifting apart means a migrated database and a +// freshly created one are different databases. +func TestSchemaMatchesMigration(t *testing.T) { + schema, err := os.ReadFile("../schema.sql") + if err != nil { + t.Fatalf("read schema.sql: %v", err) + } + migration, err := os.ReadFile("../migrations/0001_initial.sql") + if err != nil { + t.Fatalf("read migration: %v", err) + } + + up, down, ok := splitBrant(string(migration)) + if !ok { + t.Fatal("migration is missing a `-- +brant Up` / `-- +brant Down` pair") + } + + fromSchema := normalizeStatements(string(schema)) + fromMigration := normalizeStatements(up) + if len(fromSchema) != len(fromMigration) { + t.Fatalf("schema.sql has %d statements, migration Up has %d:\n%v\n%v", + len(fromSchema), len(fromMigration), fromSchema, fromMigration) + } + for i := range fromSchema { + if fromSchema[i] != fromMigration[i] { + t.Errorf("statement %d differs:\n schema.sql: %s\n migration : %s", + i, fromSchema[i], fromMigration[i]) + } + } + + // Every table created must be dropped, so a Down actually undoes the Up. + created := tableNames(fromSchema, "CREATE TABLE ") + dropped := tableNames(normalizeStatements(down), "DROP TABLE ") + if len(created) == 0 { + t.Fatal("no CREATE TABLE statements found in schema.sql") + } + for _, name := range created { + if !contains(dropped, name) { + t.Errorf("table %q is created by Up but not dropped by Down", name) + } + } + + // The design deliberately omits these two; adding either needs a design + // change, not a quiet migration. + for _, absent := range []string{"project", "comment"} { + if contains(created, absent) { + t.Errorf("table %q is deliberately absent from v1", absent) + } + } + for _, want := range []string{"space", "document_id", "proposal", "agent_token", + "index_stamp", "digest_mark"} { + if !contains(created, want) { + t.Errorf("table %q is missing from schema.sql", want) + } + } +} + +func splitBrant(src string) (up, down string, ok bool) { + i := strings.Index(src, "-- +brant Up") + j := strings.Index(src, "-- +brant Down") + if i < 0 || j < 0 || j < i { + return "", "", false + } + return src[i+len("-- +brant Up") : j], src[j+len("-- +brant Down"):], true +} + +var ( + lineComment = regexp.MustCompile(`(?m)--.*$`) + whitespace = regexp.MustCompile(`\s+`) +) + +// normalizeStatements strips line comments, collapses whitespace and splits on +// ';', so two DDL files that differ only in commentary and indentation compare +// equal. +func normalizeStatements(src string) []string { + src = lineComment.ReplaceAllString(src, " ") + var out []string + for _, stmt := range strings.Split(src, ";") { + stmt = strings.TrimSpace(whitespace.ReplaceAllString(stmt, " ")) + stmt = strings.ReplaceAll(stmt, "( ", "(") + stmt = strings.ReplaceAll(stmt, " )", ")") + if stmt != "" { + out = append(out, stmt) + } + } + return out +} + +func tableNames(stmts []string, prefix string) []string { + var out []string + for _, s := range stmts { + if !strings.HasPrefix(s, prefix) { + continue + } + rest := strings.TrimPrefix(s, prefix) + if i := strings.IndexAny(rest, " ("); i >= 0 { + rest = rest[:i] + } + out = append(out, strings.Trim(rest, `"`)) + } + return out +} + +func contains(hay []string, needle string) bool { + for _, h := range hay { + if h == needle { + return true + } + } + return false +} diff --git a/migrations/0001_initial.sql b/migrations/0001_initial.sql new file mode 100644 index 0000000000000000000000000000000000000000..2c99342168e9b6e3900a5a534c9b1ddc59d6a0b8 --- /dev/null +++ b/migrations/0001_initial.sql @@ -0,0 +1,66 @@ +-- +brant Up +CREATE TABLE space ( + id SERIAL PRIMARY KEY, + owner TEXT NOT NULL, + name TEXT NOT NULL, + created TIMESTAMPTZ NOT NULL DEFAULT now(), + CONSTRAINT uq_space_owner_name UNIQUE (owner, name) +); + +CREATE TABLE document_id ( + doc_id TEXT PRIMARY KEY, + space_id INTEGER NOT NULL REFERENCES space(id) ON DELETE CASCADE, + path TEXT NOT NULL, + updated_rev TEXT NOT NULL +); + +CREATE TABLE proposal ( + id SERIAL PRIMARY KEY, + space_id INTEGER NOT NULL REFERENCES space(id) ON DELETE CASCADE, + title TEXT NOT NULL, + rationale TEXT, + base_rev TEXT NOT NULL, + branch TEXT NOT NULL, + state TEXT NOT NULL, + approval TEXT, + merged_rev TEXT, + agent TEXT NOT NULL, + agent_session TEXT NOT NULL, + created TIMESTAMPTZ NOT NULL DEFAULT now(), + resolved TIMESTAMPTZ, + + CONSTRAINT ck_proposal_state CHECK (state IN ('open', 'merged', 'rejected')), + CONSTRAINT ck_proposal_approval CHECK (approval IS NULL OR approval IN ('human', 'policy')), + CONSTRAINT ck_proposal_merged CHECK ((state = 'merged') = (approval IS NOT NULL)), + CONSTRAINT ck_proposal_merged_rev CHECK ((state = 'merged') = (merged_rev IS NOT NULL)), + CONSTRAINT ck_proposal_resolved CHECK ((state = 'open') = (resolved IS NULL)), + CONSTRAINT ck_proposal_provenance CHECK (length(agent) > 0 AND length(agent_session) > 0) +); +CREATE INDEX ix_proposal_state_created ON proposal (state, created DESC); + +CREATE TABLE agent_token ( + id SERIAL PRIMARY KEY, + name TEXT NOT NULL, + token_hash BYTEA NOT NULL UNIQUE, + created TIMESTAMPTZ NOT NULL DEFAULT now(), + revoked TIMESTAMPTZ +); + +CREATE TABLE index_stamp ( + space_id INTEGER PRIMARY KEY REFERENCES space(id) ON DELETE CASCADE, + rev TEXT NOT NULL, + indexed_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE TABLE digest_mark ( + owner TEXT PRIMARY KEY, + seen_at TIMESTAMPTZ NOT NULL +); + +-- +brant Down +DROP TABLE digest_mark; +DROP TABLE index_stamp; +DROP TABLE agent_token; +DROP TABLE proposal; +DROP TABLE document_id; +DROP TABLE space; diff --git a/schema.sql b/schema.sql new file mode 100644 index 0000000000000000000000000000000000000000..5df79e11933f2a938a0cef2cec31147b8766ad35 --- /dev/null +++ b/schema.sql @@ -0,0 +1,97 @@ +-- spec.sr.ht full initial schema. +-- +-- This is the authoritative DDL for a fresh install. The brant migration in +-- migrations/0001_initial.sql applies the same objects incrementally; keep the +-- two in sync. +-- +-- Git is authoritative for document bodies; Postgres never stores a body. Every +-- table here holds only what git cannot answer cheaply, and every table here is +-- reconstructable from refs by the reconciler — which is what makes the +-- non-transactional merge path (git refs, then Postgres, then the index) +-- tolerable. +-- +-- Deliberately absent: `project` (a project is a saved filter — a name plus a +-- space-id list — and committing it before there are several spaces to filter +-- would be speculative; it arrives with the meta-project in Phase 2) and +-- `comment` (inline comments are post-v1, and the anchoring model should be +-- settled by building the review UI before it is committed to a schema). + +-- Spaces exist as repos; this table is for listing and index bookkeeping. +CREATE TABLE space ( + id SERIAL PRIMARY KEY, + owner TEXT NOT NULL, -- "bigbes", no ~ prefix + name TEXT NOT NULL, + created TIMESTAMPTZ NOT NULL DEFAULT now(), + CONSTRAINT uq_space_owner_name UNIQUE (owner, name) +); + +-- Global, not per-project: a later import cannot collide. doc_id is the PRIMARY +-- KEY rather than a (space_id, doc_id) pair on purpose — global uniqueness is +-- the invariant the whole link/comment/staleness model rests on, so a colliding +-- registration must be impossible to insert, not merely detected in Go. +CREATE TABLE document_id ( + doc_id TEXT PRIMARY KEY, -- "SPEC-0007" + space_id INTEGER NOT NULL REFERENCES space(id) ON DELETE CASCADE, + path TEXT NOT NULL, -- current path on the approved branch + updated_rev TEXT NOT NULL +); + +CREATE TABLE proposal ( + id SERIAL PRIMARY KEY, + space_id INTEGER NOT NULL REFERENCES space(id) ON DELETE CASCADE, + title TEXT NOT NULL, + rationale TEXT, + base_rev TEXT NOT NULL, -- the If-Match value; does not move + branch TEXT NOT NULL, -- "proposals/42" + state TEXT NOT NULL, -- open | merged | rejected + approval TEXT, -- human | policy, set on merge + merged_rev TEXT, + agent TEXT NOT NULL, -- "claude-code/spec-writer" + agent_session TEXT NOT NULL, + created TIMESTAMPTZ NOT NULL DEFAULT now(), + resolved TIMESTAMPTZ, + + -- The state machine is `open -> merged` and `open -> rejected`, and nothing + -- else. These constraints make every row that would contradict it + -- unwritable; the UPDATE ... WHERE state = 'open' guard in db/proposal.go + -- is what makes an illegal *transition* unwritable. + CONSTRAINT ck_proposal_state CHECK (state IN ('open', 'merged', 'rejected')), + CONSTRAINT ck_proposal_approval CHECK (approval IS NULL OR approval IN ('human', 'policy')), + -- Auto-merged is not human-approved and readers must be able to tell, so a + -- merged row without an approval kind (or an unmerged row carrying one) + -- would launder unreviewed agent output as blessed. + CONSTRAINT ck_proposal_merged CHECK ((state = 'merged') = (approval IS NOT NULL)), + CONSTRAINT ck_proposal_merged_rev CHECK ((state = 'merged') = (merged_rev IS NOT NULL)), + CONSTRAINT ck_proposal_resolved CHECK ((state = 'open') = (resolved IS NULL)), + -- Provenance is the one thing that is not optional: one shared token still + -- yields a full audit trail because the identity strings, not the + -- credential, are what identify who did what. NOT NULL alone would accept + -- the empty string and lose that. + CONSTRAINT ck_proposal_provenance CHECK (length(agent) > 0 AND length(agent_session) > 0) +); +-- The inbox ("N proposals waiting on you") and the digest are both +-- state-filtered, newest-first scans. +CREATE INDEX ix_proposal_state_created ON proposal (state, created DESC); + +-- Only a hash is ever stored; the token itself exists exactly once, at mint +-- time, in the response to the operator. +CREATE TABLE agent_token ( + id SERIAL PRIMARY KEY, + name TEXT NOT NULL, + token_hash BYTEA NOT NULL UNIQUE, + created TIMESTAMPTZ NOT NULL DEFAULT now(), + revoked TIMESTAMPTZ +); + +-- Index staleness: compared against the space's approved head. +CREATE TABLE index_stamp ( + space_id INTEGER PRIMARY KEY REFERENCES space(id) ON DELETE CASCADE, + rev TEXT NOT NULL, + indexed_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +-- "What landed since you last looked", for the policy-merged digest. +CREATE TABLE digest_mark ( + owner TEXT PRIMARY KEY, + seen_at TIMESTAMPTZ NOT NULL +);