From 29f6e881684794d8f3aa0d3bf7a31ab4650ed26a Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Wed, 22 Jul 2026 11:22:20 +0300 Subject: [PATCH] =?UTF-8?q?feat:=20gitx=20=E2=80=94=20bare=20space=20repos?= =?UTF-8?q?,=20the=20git-object=20read=20path,=20and=20the=20id-keyed=20me?= =?UTF-8?q?rge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One storage tier: there is no checkout on disk, so every read resolves a tree and reads blobs. The approved head, a pinned ?rev= and a proposal branch are the same code path with a different revision. Lifecycle: Create makes a bare repo at /~/ with HEAD on the approved branch and one empty initial commit, so nothing downstream has to special-case an unborn branch. Open reads the approved branch back from HEAD. Merge: tree-splice, exactly as designed. Staleness is keyed by document ID, not path — each changed document is resolved to its path on the approved head through its frontmatter id before its blob is compared, so a rename between the base and the head neither invents a conflict nor resurrects the moved path. On staleness a *StaleError carries the current head for the caller's 409. Otherwise the approved tree is rewritten with each document's blob at its path in the head and committed with two explicit parents [approvedHead, proposalHead]. Repository.Merge is never called; go-git v5 supports only FastForwardMerge and this is pure object.Tree plumbing. Refs rule: CheckRefUpdate is a pure predicate over (principal, ref, ancestry) — agents may only touch proposals/*, the approved branch takes fast-forwards from the human only and is never force-updated or deleted. hooks/ calls it without a repository, which is why ancestry is an input rather than a lookup. Concurrency: a process-wide per-space mutex keyed by repository directory guards every write, and every ref move is a compare-and-swap that rebuilds and retries when it loses to a concurrent native receive-pack push. Bounded throughout: context deadlines on every operation, a per-blob cap and per-walk byte and entry budgets. Nothing is truncated — a partially read document would be indexed and served as though whole, so it fails instead. Tests build every fixture programmatically through this package's own plumbing, never the git binary, and cover the rename between base and head, each staleness case, the two-parent merge shape, the refs rule for both principals including force-update rejection, oversized blobs, and the CAS retry. --- gitx/errors.go | 126 ++++++++++ gitx/fixture_test.go | 168 +++++++++++++ gitx/gitx.go | 371 +++++++++++++++++++++++++++++ gitx/gitx_test.go | 392 ++++++++++++++++++++++++++++++ gitx/lock.go | 69 ++++++ gitx/merge.go | 444 ++++++++++++++++++++++++++++++++++ gitx/merge_test.go | 540 ++++++++++++++++++++++++++++++++++++++++++ gitx/read.go | 392 ++++++++++++++++++++++++++++++ gitx/read_test.go | 232 ++++++++++++++++++ gitx/refsrule.go | 252 ++++++++++++++++++++ gitx/refsrule_test.go | 266 +++++++++++++++++++++ gitx/write.go | 436 ++++++++++++++++++++++++++++++++++ gitx/write_test.go | 300 +++++++++++++++++++++++ 13 files changed, 3988 insertions(+) create mode 100644 gitx/errors.go create mode 100644 gitx/fixture_test.go create mode 100644 gitx/gitx.go create mode 100644 gitx/gitx_test.go create mode 100644 gitx/lock.go create mode 100644 gitx/merge.go create mode 100644 gitx/merge_test.go create mode 100644 gitx/read.go create mode 100644 gitx/read_test.go create mode 100644 gitx/refsrule.go create mode 100644 gitx/refsrule_test.go create mode 100644 gitx/write.go create mode 100644 gitx/write_test.go diff --git a/gitx/errors.go b/gitx/errors.go new file mode 100644 index 0000000000000000000000000000000000000000..25a0dd7e55757128f18194e1b5181ef9d83debec --- /dev/null +++ b/gitx/errors.go @@ -0,0 +1,126 @@ +package gitx + +import ( + "errors" + "fmt" + + "github.com/go-git/go-git/v5/plumbing" +) + +// Sentinel errors, one per failure class. Callers compare with errors.Is and +// map the class to a status code: ErrNotFound to 404, ErrStale to 409, +// ErrRefRejected to a hook rejection message, ErrTooLarge to 413. +// +// core's sentinels (ErrInvalidName, ErrInvalidPath, ErrInvalidDocID, +// ErrMalformedFrontmatter, ...) are reused verbatim wherever the failure is a +// domain-rule violation rather than a git one; this package adds only the +// classes core cannot know about. +var ( + // ErrNotFound marks a missing space, revision, ref, path or object. + // Invalid names resolve here too, so a crafted name cannot distinguish + // "malformed" from "absent" by probing. + ErrNotFound = errors.New("not found") + + // ErrExists marks a create that would clobber something: a space whose + // directory is already present, or a proposal branch already in use. + ErrExists = errors.New("already exists") + + // ErrBadRev marks a revision string that is not a usable ref name or hex + // object id. It is distinct from ErrNotFound: this is "cannot be a + // revision", not "is not in this repository". + ErrBadRev = errors.New("invalid revision") + + // ErrTooLarge marks a blob or a tree walk that exceeded its byte or entry + // budget. Nothing is truncated: a half-read markdown document would be + // indexed and served as if it were whole, so the read fails instead. + ErrTooLarge = errors.New("too large") + + // ErrStale marks a merge whose base moved under it. Match with errors.Is + // and type-assert to *StaleError for the current approved head, which is + // what the caller returns in the 409. + ErrStale = errors.New("stale base") + + // ErrRefRejected marks a ref update the refs rule forbids. This is what the + // update hook reports back to the pushing client. + ErrRefRejected = errors.New("ref update rejected") + + // ErrRefRace marks a compare-and-swap ref update that lost to a concurrent + // writer (in practice a native receive-pack push) more times than the retry + // budget allows. The operation had no effect. + ErrRefRace = errors.New("ref changed concurrently") + + // ErrUnsupportedEntry marks a tree entry the document model has no meaning + // for: a submodule, or a symlink occupying a document path. Skipping it + // silently would drop a document out of the index with no trace, which is + // the exact failure the service exists to prevent. + ErrUnsupportedEntry = errors.New("unsupported tree entry") + + // ErrDuplicateDocID marks two documents carrying the same id in one tree. + // Global uniqueness is the registry's job, but a tree that already violates + // it makes the id-keyed merge ambiguous, so it is refused here too. + ErrDuplicateDocID = errors.New("duplicate document id") + + // ErrUnsupportedChange marks a proposal carrying a change the merge model + // cannot express: a deletion, a rename, or an edit to a non-document path. + // Deletion and rename are human-push-only by design. + ErrUnsupportedChange = errors.New("unsupported proposal change") +) + +// StaleReason names which of the staleness cases fired. It is carried on +// StaleError so the caller can explain the 409 rather than just returning it. +type StaleReason string + +const ( + // StaleBaseDetached means the proposal's base is no longer an ancestor of + // the approved head — the approved branch was rewritten under it. + StaleBaseDetached StaleReason = "base is not an ancestor of the approved head" + + // StaleDocChanged means the document changed on the approved branch since + // the proposal's base. + StaleDocChanged StaleReason = "document changed on the approved branch since the base" + + // StaleDocRemoved means the document existed at the base and no longer + // exists on the approved head. + StaleDocRemoved StaleReason = "document was removed from the approved branch since the base" + + // StaleDocAppeared means a document with this id appeared on the approved + // branch after the base, so the proposal would silently overwrite it. + StaleDocAppeared StaleReason = "a document with this id appeared on the approved branch after the base" + + // StalePathTaken means the proposal's new document targets a path already + // occupied on the approved head by a different document. + StalePathTaken StaleReason = "path is occupied on the approved branch by a different document" +) + +// StaleError reports that a merge cannot proceed because the approved branch +// moved under the proposal. Head is the current approved head, which the caller +// hands back in the 409 so the agent can refetch and re-propose against it. +type StaleError struct { + Reason StaleReason + // DocID is the document that went stale. Empty for StaleBaseDetached, + // which is about the proposal as a whole. + DocID string + // Path is where the document sits on the approved head, or the contested + // path for StalePathTaken. Empty when the document is not on the head. + Path string + // Base is the proposal's base revision, Head the current approved head. + Base plumbing.Hash + Head plumbing.Hash +} + +func (e *StaleError) Error() string { + if e.DocID == "" { + return fmt.Sprintf("gitx: stale base %s (approved head is %s): %s", + e.Base, e.Head, e.Reason) + } + if e.Path == "" { + return fmt.Sprintf("gitx: stale base %s for %s (approved head is %s): %s", + e.Base, e.DocID, e.Head, e.Reason) + } + return fmt.Sprintf("gitx: stale base %s for %s at %q (approved head is %s): %s", + e.Base, e.DocID, e.Path, e.Head, e.Reason) +} + +// Is makes errors.Is(err, ErrStale) true for every StaleError, so callers can +// branch on the class and only type-assert when they need the head. +func (e *StaleError) Is(target error) bool { return target == ErrStale } diff --git a/gitx/fixture_test.go b/gitx/fixture_test.go new file mode 100644 index 0000000000000000000000000000000000000000..eddbd5940acfe1ac48792110ea39cc5ce64d12dd --- /dev/null +++ b/gitx/fixture_test.go @@ -0,0 +1,168 @@ +package gitx + +import ( + "context" + "fmt" + "path/filepath" + "testing" + "time" + + "github.com/go-git/go-git/v5/plumbing" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// Fixtures are built entirely in-process: Create makes the bare repo, and +// commits are written through this package's own plumbing. Nothing shells out +// to the git binary, so the tests exercise the code that actually runs in the +// daemon rather than proving that git works. + +var fxSpace = core.SpaceRef{Owner: "bigbes", Name: "rfcs"} + +// fxTime yields deterministic, monotonically increasing commit timestamps, so +// commit shas are stable within a run and the ordering in git log is defined. +func fxTime(n int) time.Time { + return time.Date(2026, 7, 22, 12, 0, 0, 0, time.UTC).Add(time.Duration(n) * time.Minute) +} + +func fxSig(name, email string, n int) Signature { + return Signature{Name: name, Email: email, When: fxTime(n)} +} + +func owner(n int) Signature { return fxSig("bigbes", "bigbes@gmail.com", n) } + +func agent(n int) Signature { + return fxSig("claude-code/spec-writer (for bigbes)", "agent@srht.bigb.es", n) +} + +// meta builds a commit description with the provenance trailers the design +// requires on an agent write. The trailer *policy* is authn/'s; here they are +// just what a caller supplies. +func meta(subject string, n int, trailers ...Trailer) CommitMeta { + return CommitMeta{ + Message: subject, + Trailers: trailers, + Author: agent(n), + Committer: owner(n), + } +} + +// ownerMeta is a commit made in the owner's own name, as a human push would be. +func ownerMeta(subject string, n int) CommitMeta { + return CommitMeta{Message: subject, Author: owner(n), Committer: owner(n)} +} + +// doc renders a minimal valid document: frontmatter with the three required +// keys, then a body. +func doc(id, title, body string) []byte { + return []byte(fmt.Sprintf("---\nid: %s\ntitle: %s\nstatus: draft\n---\n\n%s\n", id, title, body)) +} + +// newSpace creates a fresh space under a temporary repos root and returns the +// handle plus the root. +func newSpace(t *testing.T) (*Repo, string) { + t.Helper() + root := t.TempDir() + repo, err := Create(context.Background(), root, fxSpace, CreateOptions{Owner: owner(0)}) + if err != nil { + t.Fatalf("Create: %v", err) + } + return repo, root +} + +// pushApproved commits directly onto the approved branch, standing in for a +// human push through receive-pack. It deliberately bypasses CommitProposal, +// which refuses the approved branch — that restriction is the point of the +// write path, and a test that wants a human push has to simulate one. +// +// writes with a nil Content delete the path, which is how the fixture expresses +// the human-only rename that the merge must survive. +func pushApproved(t *testing.T, r *Repo, meta CommitMeta, writes ...Write) plumbing.Hash { + t.Helper() + return pushBranch(t, r, r.ApprovedBranch(), meta, writes...) +} + +func pushBranch(t *testing.T, r *Repo, branch string, meta CommitMeta, writes ...Write) plumbing.Hash { + t.Helper() + + name := plumbing.NewBranchReferenceName(branch) + old, err := r.repo.Reference(name, false) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + tree, err := r.treeOf(old.Hash()) + if err != nil { + t.Fatalf("tree of %s: %v", old.Hash(), err) + } + node, err := r.loadTree(tree, 0) + if err != nil { + t.Fatalf("loadTree: %v", err) + } + for _, w := range writes { + if w.Content == nil { + if !node.remove(w.Path) { + t.Fatalf("remove %q: not present", w.Path) + } + continue + } + h, err := r.writeBlob(w.Path, w.Content) + if err != nil { + t.Fatalf("writeBlob %q: %v", w.Path, err) + } + if err := node.set(w.Path, h); err != nil { + t.Fatalf("set %q: %v", w.Path, err) + } + } + treeHash, err := node.write(r.repo.Storer) + if err != nil { + t.Fatalf("write tree: %v", err) + } + commit, err := r.writeCommit(meta, treeHash, []plumbing.Hash{old.Hash()}) + if err != nil { + t.Fatalf("writeCommit: %v", err) + } + if err := r.repo.Storer.SetReference(plumbing.NewHashReference(name, commit)); err != nil { + t.Fatalf("set %s: %v", name, err) + } + return commit +} + +// openProposal cuts a proposal branch at base and commits writes onto it. +func openProposal(t *testing.T, r *Repo, branch, base string, m CommitMeta, writes ...Write) CommitResult { + t.Helper() + ctx := context.Background() + if _, err := r.CreateProposalBranch(ctx, branch, base); err != nil { + t.Fatalf("CreateProposalBranch(%q, %q): %v", branch, base, err) + } + res, err := r.CommitProposal(ctx, branch, writes, m) + if err != nil { + t.Fatalf("CommitProposal(%q): %v", branch, err) + } + return res +} + +// docPaths lists the document paths present at a revision. +func docPaths(t *testing.T, r *Repo, rev string) []string { + t.Helper() + docs, err := r.ListDocuments(context.Background(), rev) + if err != nil { + t.Fatalf("ListDocuments(%q): %v", rev, err) + } + out := make([]string, 0, len(docs)) + for _, d := range docs { + out = append(out, d.Path) + } + return out +} + +// mustRead reads a document body at a revision. +func mustRead(t *testing.T, r *Repo, rev, path string) string { + t.Helper() + d, err := r.ReadDocument(context.Background(), rev, path) + if err != nil { + t.Fatalf("ReadDocument(%q, %q): %v", rev, path, err) + } + return string(d.Data) +} + +func spaceDir(root string) string { return filepath.Join(root, "~"+fxSpace.Owner, fxSpace.Name) } diff --git a/gitx/gitx.go b/gitx/gitx.go new file mode 100644 index 0000000000000000000000000000000000000000..b34dce21ab3d7ca67b4e3e43312a237446850d2d --- /dev/null +++ b/gitx/gitx.go @@ -0,0 +1,371 @@ +// Package gitx is spec.sr.ht's git layer: the bare-repo lifecycle of a space, +// the only read path there is, the proposal write path, the tree-splice merge, +// and the refs rule the receive hooks enforce. +// +// # One storage tier +// +// There is no checkout on disk. Every read resolves a tree and reads blobs, so +// the approved head, a pinned ?rev= and a proposal branch are the same +// code path with a different revision. Nothing downstream of this package ever +// touches the filesystem to find a document. +// +// # No text merge, ever +// +// go-git v5 implements only FastForwardMerge, and the whole-document write +// grain makes a three-way merge unnecessary anyway. Merge is pure plumbing: +// object.Tree manipulation plus a commit with two explicit parents. A conflict +// is always "your base moved, re-propose" (*StaleError), never a conflict +// marker. Merge never calls Repository.Merge. +// +// # Staleness is keyed by document id, not path +// +// Paths move; ids do not. Every changed document is resolved to its path on the +// approved head through its frontmatter id before its blob is compared, so a +// rename between the base and the head neither invents a conflict nor +// resurrects a document that was moved. +// +// # Two writers, one repo +// +// Human pushes arrive through native receive-pack (spawned by sshd) and agent +// proposals through this package in-process: two independent ref-locking +// implementations over the same loose refs. go-git's locking is not verified to +// interoperate with native git's, so every write here takes a per-space mutex +// (process-wide, keyed by the repository's directory) and every ref move is a +// compare-and-swap that retries when it loses. +// +// # Bounded by construction +// +// Every operation derives a timeout from the caller's context, every blob read +// is capped, and every tree walk is capped in both bytes and entries. Nothing +// is truncated to fit: a partially read document would be indexed and served as +// though it were whole, so an over-budget read fails with ErrTooLarge instead. +package gitx + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/object" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +const ( + // DefaultApprovedBranch is the branch a space is created with when the + // caller does not name one. It is the branch the human pushes to and the + // branch every default read resolves against; "approved" is a property of + // being reachable from it, and of nothing else. + DefaultApprovedBranch = "main" + + // defaultTimeout bounds a single gitx operation when the caller's context + // carries no earlier deadline. + defaultTimeout = 30 * time.Second + + // maxDocumentSize caps one document blob. Documents are markdown written + // for a human to review; anything past this is either an attachment in the + // wrong place or a runaway agent. + maxDocumentSize = 4 << 20 // 4 MiB + + // maxWalkBytes caps the total blob bytes one tree walk may materialize, so + // a single read cannot pull an entire space into memory. + maxWalkBytes = 128 << 20 // 128 MiB + + // maxTreeEntries caps how many entries one walk may visit, bounding the + // cost of a pathological tree independently of its byte size. + maxTreeEntries = 100000 + + // maxTreeDepth caps directory nesting. Deeper than this is not a document + // layout, it is a way to blow the stack. + maxTreeDepth = 64 + + // casAttempts is how many times a ref compare-and-swap is retried after + // losing to a concurrent writer before giving up with ErrRefRace. + casAttempts = 5 + + // maxRevLen caps a revision string before it reaches go-git's parser. + maxRevLen = 255 +) + +// Repo is a handle to one space: a bare git repository at +// "/~/". +// +// Reads are safe for concurrent use. Writes serialize on a process-wide +// per-space lock keyed by the repository directory, so two Repo values opened +// over the same space still exclude each other. +type Repo struct { + dir string + ref core.SpaceRef + repo *git.Repository + approved string // short branch name, read from HEAD + + // Test-only overrides; zero means "use the package constant". They exist so + // truncation and retry paths can be exercised with small fixtures instead + // of pathological ones. + timeout time.Duration + docLimit int64 + walkLimit int64 + entryLimit int + attemptLimit int + + // beforeCAS, when set, runs immediately before each ref compare-and-swap. + // It is the only way to open the window a concurrent native receive-pack + // push would land in, which is the one thing about the retry path that + // cannot be tested from the outside. + beforeCAS func() +} + +// raceHook fires the test-only pre-compare-and-swap hook. +func (r *Repo) raceHook() { + if r.beforeCAS != nil { + r.beforeCAS() + } +} + +// DiskPath returns the bare repository directory for a space, +// "/~/". The space reference must already be validated; +// Create and Open do that before they build a path. +func DiskPath(root string, sr core.SpaceRef) string { + return filepath.Join(root, "~"+sr.Owner, sr.Name) +} + +// CreateOptions configures Create. +type CreateOptions struct { + // ApprovedBranch names the branch the human pushes to. Empty means + // DefaultApprovedBranch. + ApprovedBranch string + + // Owner is the identity on the initial commit, in practice the instance's + // [sr.ht] owner-name/owner-email. It is required: inventing a committer + // would put a fabricated identity in a history whose whole purpose is + // provenance. + Owner Signature + + // Message is the initial commit's subject. Empty means a generated + // "Initialize space ~owner/name". + Message string +} + +// Create initialises a new space: a bare repository at DiskPath(root, sr) whose +// HEAD points at the approved branch, carrying one empty initial commit. +// +// The initial commit is deliberately not skipped. It makes the approved head +// resolvable from the moment the space exists, so no reader, reconciler or +// merge has to special-case an unborn branch, and it costs the human nothing: +// they clone and push on top of it rather than pushing an unrelated history. +// +// root must be absolute — the per-space write lock is keyed by directory, and +// two spellings of the same directory would be two locks. Create refuses to +// touch an existing directory (ErrExists), and removes what it made if it fails +// partway, so a failed create never leaves a half-built space behind. +func Create(ctx context.Context, root string, sr core.SpaceRef, opts CreateOptions) (_ *Repo, err error) { + if !filepath.IsAbs(root) { + return nil, fmt.Errorf("gitx: Create requires an absolute repos root, got %q", root) + } + if err := validateSpaceRef(sr); err != nil { + return nil, err + } + branch := opts.ApprovedBranch + if branch == "" { + branch = DefaultApprovedBranch + } + if err := ValidateBranch(branch); err != nil { + return nil, err + } + if err := opts.Owner.validate("owner"); err != nil { + return nil, err + } + message := opts.Message + if message == "" { + message = "Initialize space " + sr.String() + } + + dir := filepath.Clean(DiskPath(root, sr)) + if _, statErr := os.Stat(dir); statErr == nil { + return nil, fmt.Errorf("%w: space %s at %q", ErrExists, sr, dir) + } else if !os.IsNotExist(statErr) { + return nil, fmt.Errorf("gitx: stat %q: %w", dir, statErr) + } + + if err := os.MkdirAll(dir, 0o755); err != nil { + return nil, fmt.Errorf("gitx: create space dir %q: %w", dir, err) + } + // Nothing past this point may leave a partially built repository behind. + defer func() { + if err != nil { + os.RemoveAll(dir) + } + }() + + head := plumbing.NewBranchReferenceName(branch) + repo, err := git.PlainInitWithOptions(dir, &git.PlainInitOptions{ + Bare: true, + InitOptions: git.InitOptions{DefaultBranch: head}, + }) + if err != nil { + return nil, fmt.Errorf("gitx: init bare repo at %q: %w", dir, err) + } + + r := &Repo{dir: dir, ref: sr, repo: repo, approved: branch} + + unlock, err := r.lock(ctx) + if err != nil { + return nil, err + } + defer unlock() + + emptyTree, err := (&mutableTree{}).write(repo.Storer) + if err != nil { + return nil, fmt.Errorf("gitx: write empty tree for %s: %w", sr, err) + } + commit, err := r.writeCommit(CommitMeta{ + Message: message, + Author: opts.Owner, + Committer: opts.Owner, + }, emptyTree, nil) + if err != nil { + return nil, err + } + if err := repo.Storer.SetReference(plumbing.NewHashReference(head, commit)); err != nil { + return nil, fmt.Errorf("gitx: set %s for %s: %w", head, sr, err) + } + return r, nil +} + +// Open opens an existing space. Invalid names, a missing directory and a +// directory that is not a bare repository all yield ErrNotFound, so existence +// is never leaked and no crafted name escapes root: core's validators reject +// '/', '..' and a leading '-' before any path is built. +// +// The approved branch is read from HEAD rather than configured separately — +// there is exactly one place a space can record it, so there is nothing to keep +// in sync. A detached HEAD is corruption in a space repository and is refused. +func Open(root string, sr core.SpaceRef) (*Repo, error) { + if !filepath.IsAbs(root) { + return nil, fmt.Errorf("gitx: Open requires an absolute repos root, got %q", root) + } + if err := validateSpaceRef(sr); err != nil { + return nil, fmt.Errorf("%w: %v", ErrNotFound, err) + } + + dir := filepath.Clean(DiskPath(root, sr)) + // Cheap bare-repo sanity check before the path reaches go-git. + if fi, err := os.Stat(filepath.Join(dir, "HEAD")); err != nil || fi.IsDir() { + return nil, fmt.Errorf("%w: space %s", ErrNotFound, sr) + } + repo, err := git.PlainOpen(dir) + if err != nil { + return nil, fmt.Errorf("%w: space %s: %v", ErrNotFound, sr, err) + } + + headRef, err := repo.Reference(plumbing.HEAD, false) + if err != nil { + return nil, fmt.Errorf("%w: space %s has no HEAD: %v", ErrNotFound, sr, err) + } + if headRef.Type() != plumbing.SymbolicReference { + return nil, fmt.Errorf("gitx: space %s has a detached HEAD; the approved branch is unknowable", sr) + } + branch := headRef.Target().Short() + if err := ValidateBranch(branch); err != nil { + return nil, fmt.Errorf("gitx: space %s HEAD points at an unusable branch: %w", sr, err) + } + return &Repo{dir: dir, ref: sr, repo: repo, approved: branch}, nil +} + +// Dir returns the bare repository's directory. +func (r *Repo) Dir() string { return r.dir } + +// SpaceRef returns the space this handle addresses. +func (r *Repo) SpaceRef() core.SpaceRef { return r.ref } + +// ApprovedBranch returns the short name of the branch HEAD points at. A +// document is approved exactly when it is reachable from this branch. +func (r *Repo) ApprovedBranch() string { return r.approved } + +// withTimeout derives a per-operation deadline. A caller-supplied deadline that +// is already earlier wins, since context.WithTimeout never extends. +func (r *Repo) withTimeout(ctx context.Context) (context.Context, context.CancelFunc) { + d := r.timeout + if d <= 0 { + d = defaultTimeout + } + return context.WithTimeout(ctx, d) +} + +func (r *Repo) blobLimit() int64 { + if r.docLimit > 0 { + return r.docLimit + } + return maxDocumentSize +} + +func (r *Repo) totalLimit() int64 { + if r.walkLimit > 0 { + return r.walkLimit + } + return maxWalkBytes +} + +func (r *Repo) entryCap() int { + if r.entryLimit > 0 { + return r.entryLimit + } + return maxTreeEntries +} + +func (r *Repo) casBudget() int { + if r.attemptLimit > 0 { + return r.attemptLimit + } + return casAttempts +} + +// validateSpaceRef checks both halves of a space reference with core's rules. +func validateSpaceRef(sr core.SpaceRef) error { + if err := core.ValidateOwner(sr.Owner); err != nil { + return err + } + return core.ValidateSpaceName(sr.Name) +} + +// Signature is a git identity plus the moment it acted. +// +// When is required rather than defaulted to time.Now: a commit whose timestamp +// this package invented would be a fact about the service pretending to be a +// fact about the author, and deterministic timestamps are what make provenance +// testable. +type Signature struct { + Name string + Email string + When time.Time +} + +// validate rejects identities git cannot round-trip. Angle brackets and +// newlines would terminate or forge the ident line in the commit object, which +// is how a provenance record comes to say something nobody wrote. +func (s Signature) validate(role string) error { + if strings.TrimSpace(s.Name) == "" { + return fmt.Errorf("gitx: %s name is required", role) + } + if strings.TrimSpace(s.Email) == "" { + return fmt.Errorf("gitx: %s email is required", role) + } + if s.When.IsZero() { + return fmt.Errorf("gitx: %s timestamp is required", role) + } + for _, field := range []struct{ what, val string }{{"name", s.Name}, {"email", s.Email}} { + if strings.ContainsAny(field.val, "<>\n\r\x00") { + return fmt.Errorf("gitx: %s %s %q contains a disallowed character", role, field.what, field.val) + } + } + return nil +} + +func (s Signature) toGit() object.Signature { + return object.Signature{Name: s.Name, Email: s.Email, When: s.When} +} diff --git a/gitx/gitx_test.go b/gitx/gitx_test.go new file mode 100644 index 0000000000000000000000000000000000000000..0befec5ef471be8c3e7a1d6febf075efda0d104d --- /dev/null +++ b/gitx/gitx_test.go @@ -0,0 +1,392 @@ +package gitx + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/go-git/go-git/v5/plumbing" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +func TestCreateMakesBareSpaceWithInitialCommit(t *testing.T) { + repo, root := newSpace(t) + ctx := context.Background() + + if got, want := repo.Dir(), spaceDir(root); got != want { + t.Fatalf("Dir = %q, want %q", got, want) + } + if got := repo.ApprovedBranch(); got != DefaultApprovedBranch { + t.Fatalf("ApprovedBranch = %q, want %q", got, DefaultApprovedBranch) + } + // Bare: objects and HEAD at the top level, no worktree. + for _, name := range []string{"HEAD", "objects", "refs"} { + if _, err := os.Stat(filepath.Join(repo.Dir(), name)); err != nil { + t.Fatalf("bare repo missing %s: %v", name, err) + } + } + if _, err := os.Stat(filepath.Join(repo.Dir(), ".git")); !os.IsNotExist(err) { + t.Fatalf("repo at %q is not bare", repo.Dir()) + } + + head, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatalf("ApprovedHead: %v", err) + } + c, err := repo.repo.CommitObject(head) + if err != nil { + t.Fatalf("CommitObject: %v", err) + } + if c.NumParents() != 0 { + t.Fatalf("initial commit has %d parents, want 0", c.NumParents()) + } + if !strings.Contains(c.Message, fxSpace.String()) { + t.Fatalf("initial commit message %q does not name the space", c.Message) + } + if c.Author.Email != "bigbes@gmail.com" { + t.Fatalf("initial commit author = %q, want the supplied owner", c.Author.Email) + } + tree, err := c.Tree() + if err != nil { + t.Fatalf("Tree: %v", err) + } + if len(tree.Entries) != 0 { + t.Fatalf("initial commit tree has %d entries, want an empty tree", len(tree.Entries)) + } + if docs := docPaths(t, repo, DefaultApprovedBranch); len(docs) != 0 { + t.Fatalf("fresh space lists documents: %v", docs) + } +} + +func TestCreateRefusesToClobberAndCleansUpOwner(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + + if _, err := Create(ctx, root, fxSpace, CreateOptions{Owner: owner(0)}); err != nil { + t.Fatalf("Create: %v", err) + } + _, err := Create(ctx, root, fxSpace, CreateOptions{Owner: owner(0)}) + if !errors.Is(err, ErrExists) { + t.Fatalf("second Create error = %v, want ErrExists", err) + } + + // A missing owner identity is refused rather than invented, and the + // half-made directory is removed. + other := core.SpaceRef{Owner: "bigbes", Name: "notes"} + if _, err := Create(ctx, root, other, CreateOptions{}); err == nil { + t.Fatal("Create with no owner identity succeeded") + } + if _, err := os.Stat(DiskPath(root, other)); !os.IsNotExist(err) { + t.Fatalf("failed Create left %q behind", DiskPath(root, other)) + } +} + +func TestCreateAndOpenRejectBadInput(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + + if _, err := Create(ctx, "relative/root", fxSpace, CreateOptions{Owner: owner(0)}); err == nil { + t.Fatal("Create accepted a relative repos root") + } + if _, err := Open("relative/root", fxSpace); err == nil { + t.Fatal("Open accepted a relative repos root") + } + + bad := []core.SpaceRef{ + {Owner: "..", Name: "rfcs"}, + {Owner: "bigbes", Name: ".."}, + {Owner: "bigbes", Name: "a/b"}, + {Owner: "", Name: "rfcs"}, + {Owner: "bigbes", Name: "-rf"}, + } + for _, sr := range bad { + if _, err := Create(ctx, root, sr, CreateOptions{Owner: owner(0)}); err == nil { + t.Fatalf("Create accepted %+v", sr) + } + // Open reports every rejection as ErrNotFound so probing cannot tell a + // malformed name from an absent space. + if _, err := Open(root, sr); !errors.Is(err, ErrNotFound) { + t.Fatalf("Open(%+v) error = %v, want ErrNotFound", sr, err) + } + } +} + +func TestOpenReadsApprovedBranchFromHead(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + sr := core.SpaceRef{Owner: "bigbes", Name: "ops"} + + if _, err := Create(ctx, root, sr, CreateOptions{Owner: owner(0), ApprovedBranch: "approved"}); err != nil { + t.Fatalf("Create: %v", err) + } + repo, err := Open(root, sr) + if err != nil { + t.Fatalf("Open: %v", err) + } + if got := repo.ApprovedBranch(); got != "approved" { + t.Fatalf("ApprovedBranch = %q, want %q", got, "approved") + } + if _, err := repo.ApprovedHead(ctx); err != nil { + t.Fatalf("ApprovedHead: %v", err) + } +} + +func TestOpenMissingSpaceIsNotFound(t *testing.T) { + root := t.TempDir() + if _, err := Open(root, fxSpace); !errors.Is(err, ErrNotFound) { + t.Fatalf("Open of an absent space = %v, want ErrNotFound", err) + } + // A directory that exists but is not a repository is equally not found. + if err := os.MkdirAll(DiskPath(root, fxSpace), 0o755); err != nil { + t.Fatal(err) + } + if _, err := Open(root, fxSpace); !errors.Is(err, ErrNotFound) { + t.Fatalf("Open of a non-repository = %v, want ErrNotFound", err) + } +} + +func TestSignatureValidation(t *testing.T) { + now := time.Now() + cases := []struct { + name string + sig Signature + ok bool + }{ + {"complete", Signature{Name: "bigbes", Email: "b@example.com", When: now}, true}, + {"no name", Signature{Email: "b@example.com", When: now}, false}, + {"blank name", Signature{Name: " ", Email: "b@example.com", When: now}, false}, + {"no email", Signature{Name: "bigbes", When: now}, false}, + {"zero time", Signature{Name: "bigbes", Email: "b@example.com"}, false}, + {"angle bracket", Signature{Name: "a ", Email: "b@example.com", When: now}, false}, + {"newline", Signature{Name: "a\nb", Email: "b@example.com", When: now}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := tc.sig.validate("author") + if tc.ok && err != nil { + t.Fatalf("validate = %v, want nil", err) + } + if !tc.ok && err == nil { + t.Fatal("validate = nil, want an error") + } + }) + } +} + +func TestCommitMetaRendersTrailerBlock(t *testing.T) { + m := CommitMeta{ + Message: "Add storage model section\n\nRationale goes here.\n\n\n", + Trailers: []Trailer{ + {Key: "X-Agent-Session", Value: "8fb9c9a4-b078-4af1-89eb-d97c522f9921"}, + {Key: "X-Agent-Base", Value: "deadbeef"}, + }, + Author: agent(1), + Committer: owner(1), + } + want := "Add storage model section\n\nRationale goes here.\n\n" + + "X-Agent-Session: 8fb9c9a4-b078-4af1-89eb-d97c522f9921\n" + + "X-Agent-Base: deadbeef\n" + if got := m.text(); got != want { + t.Fatalf("text() =\n%q\nwant\n%q", got, want) + } + if err := m.validate(); err != nil { + t.Fatalf("validate: %v", err) + } +} + +func TestCommitMetaRejectsForgedTrailers(t *testing.T) { + base := CommitMeta{Message: "subject", Author: agent(1), Committer: owner(1)} + + bad := base + bad.Trailers = []Trailer{{Key: "X-Agent-Session", Value: "a\nX-Agent-Base: forged"}} + if err := bad.validate(); err == nil { + t.Fatal("a trailer value carrying a newline was accepted") + } + + bad = base + bad.Trailers = []Trailer{{Key: "X Agent: Session", Value: "v"}} + if err := bad.validate(); err == nil { + t.Fatal("a malformed trailer key was accepted") + } + + bad = base + bad.Message = "\n\nbody only" + if err := bad.validate(); err == nil { + t.Fatal("a message with no subject line was accepted") + } + + bad = base + bad.Message = " " + if err := bad.validate(); err == nil { + t.Fatal("a blank message was accepted") + } +} + +func TestWithLockSerializesAndHonoursContext(t *testing.T) { + repo, _ := newSpace(t) + + held := make(chan struct{}) + release := make(chan struct{}) + done := make(chan error, 1) + go func() { + done <- repo.WithLock(context.Background(), func(context.Context) error { + close(held) + <-release + return nil + }) + }() + <-held + + // A second holder cannot get in while the first is running, and gives up + // when its context expires rather than blocking forever. + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + err := repo.WithLock(ctx, func(context.Context) error { + t.Error("second WithLock ran while the lock was held") + return nil + }) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("contended WithLock error = %v, want context.DeadlineExceeded", err) + } + + close(release) + if err := <-done; err != nil { + t.Fatalf("first WithLock: %v", err) + } + + // A second handle over the same directory shares the lock: it is keyed by + // the repository directory, not by the Repo value. + other, err := Open(filepath.Dir(filepath.Dir(repo.Dir())), fxSpace) + if err != nil { + t.Fatalf("Open: %v", err) + } + if spaceLock(other.Dir()) != spaceLock(repo.Dir()) { + t.Fatal("two handles over one space have different locks") + } +} + +func TestProposalBranchNaming(t *testing.T) { + b, err := ProposalBranch(42) + if err != nil { + t.Fatalf("ProposalBranch: %v", err) + } + if b != "proposals/42" { + t.Fatalf("ProposalBranch(42) = %q", b) + } + if _, err := ProposalBranch(0); err == nil { + t.Fatal("ProposalBranch(0) succeeded") + } + if id, ok := ParseProposalBranch("proposals/42"); !ok || id != 42 { + t.Fatalf("ParseProposalBranch = %d, %v", id, ok) + } + for _, name := range []string{"proposals", "proposals/", "main", "proposals/draft", "proposals/-1"} { + if _, ok := ParseProposalBranch(name); ok { + t.Fatalf("ParseProposalBranch(%q) reported an id", name) + } + } + if IsProposalBranch("proposals") { + t.Fatal("the bare namespace is not a proposal branch") + } + if !IsProposalBranch("proposals/draft") { + t.Fatal("a non-numeric proposal branch is still a proposal branch") + } +} + +func TestListProposalBranches(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + head, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + for _, b := range []string{"proposals/2", "proposals/1"} { + if _, err := repo.CreateProposalBranch(ctx, b, head.String()); err != nil { + t.Fatalf("CreateProposalBranch(%q): %v", b, err) + } + } + got, err := repo.ListProposalBranches(ctx) + if err != nil { + t.Fatalf("ListProposalBranches: %v", err) + } + if len(got) != 2 || got[0].Name != "proposals/1" || got[1].Name != "proposals/2" { + t.Fatalf("ListProposalBranches = %+v", got) + } + if got[0].Head != head { + t.Fatalf("proposals/1 head = %s, want %s", got[0].Head, head) + } +} + +func TestResolveRevAndValidation(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + head, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + for _, rev := range []string{DefaultApprovedBranch, head.String(), head.String()[:10]} { + got, err := repo.ResolveRev(ctx, rev) + if err != nil { + t.Fatalf("ResolveRev(%q): %v", rev, err) + } + if got != head { + t.Fatalf("ResolveRev(%q) = %s, want %s", rev, got, head) + } + } + + // Revision arithmetic is not part of the read contract. + for _, rev := range []string{"main^", "main~1", "main@{0}", "", "-main", "main..other", "ma in"} { + if _, err := repo.ResolveRev(ctx, rev); !errors.Is(err, ErrBadRev) { + t.Fatalf("ResolveRev(%q) error = %v, want ErrBadRev", rev, err) + } + } + if _, err := repo.ResolveRev(ctx, "nosuchbranch"); !errors.Is(err, ErrNotFound) { + t.Fatalf("ResolveRev of an unknown branch = %v, want ErrNotFound", err) + } + // A tree sha is a valid object but not a commit. + tree, err := repo.treeOf(head) + if err != nil { + t.Fatal(err) + } + if _, err := repo.ResolveRev(ctx, tree.Hash.String()); !errors.Is(err, ErrNotFound) { + t.Fatalf("ResolveRev of a tree sha = %v, want ErrNotFound", err) + } +} + +func TestIsAncestor(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + next := pushApproved(t, repo, ownerMeta("add a doc", 1), + Write{Path: "specs/0001.md", Content: doc("SPEC-0001", "One", "body")}) + + for _, tc := range []struct { + a, b plumbing.Hash + want bool + }{ + {base, next, true}, + {next, base, false}, + {base, base, true}, + } { + got, err := repo.IsAncestor(ctx, tc.a, tc.b) + if err != nil { + t.Fatalf("IsAncestor(%s, %s): %v", tc.a, tc.b, err) + } + if got != tc.want { + t.Fatalf("IsAncestor(%s, %s) = %v, want %v", tc.a, tc.b, got, tc.want) + } + } + if _, err := repo.IsAncestor(ctx, plumbing.NewHash(strings.Repeat("0", 39)+"1"), next); !errors.Is(err, ErrNotFound) { + t.Fatalf("IsAncestor of an unknown commit = %v, want ErrNotFound", err) + } +} diff --git a/gitx/lock.go b/gitx/lock.go new file mode 100644 index 0000000000000000000000000000000000000000..5e454d33011f0a6bc0ea3b55933ed3fe2f8741b8 --- /dev/null +++ b/gitx/lock.go @@ -0,0 +1,69 @@ +package gitx + +import ( + "context" + "fmt" + "sync" +) + +// spaceLocks holds one buffered channel per space directory, used as a +// context-aware mutex. It is process-wide on purpose: a space may be opened +// many times (once per request is normal), and per-handle locking would not +// exclude anything. The key is the cleaned absolute repository directory, which +// Create and Open both guarantee. +// +// This guards only the daemon's own writes. Human pushes go through native +// receive-pack in another process and take git's own ref locks, whose +// interoperation with go-git's is not verified — which is why every ref move +// here is additionally a compare-and-swap that retries. +var spaceLocks = struct { + mu sync.Mutex + m map[string]chan struct{} +}{m: make(map[string]chan struct{})} + +func spaceLock(dir string) chan struct{} { + spaceLocks.mu.Lock() + defer spaceLocks.mu.Unlock() + ch, ok := spaceLocks.m[dir] + if !ok { + ch = make(chan struct{}, 1) + spaceLocks.m[dir] = ch + } + return ch +} + +// lock acquires the space's write lock, honouring ctx's deadline. The returned +// function releases it and must be called exactly once. +// +// Entries are never removed from the registry. A space is a long-lived object +// and the entry is one channel; reclaiming them would need a refcount whose +// only purpose is to free a few dozen bytes. +func (r *Repo) lock(ctx context.Context) (func(), error) { + ch := spaceLock(r.dir) + select { + case ch <- struct{}{}: + var once sync.Once + return func() { once.Do(func() { <-ch }) }, nil + case <-ctx.Done(): + return nil, fmt.Errorf("gitx: acquiring the write lock for %s: %w", r.ref, ctx.Err()) + } +} + +// WithLock runs fn holding the space's write lock, so a caller that has to make +// several git writes look like one operation (a merge plus its bookkeeping, the +// reconciler repairing a branch) can do so without reaching into this package's +// internals. +// +// The lock is not reentrant: fn must not call an exported write method on the +// same space, which would deadlock. +func (r *Repo) WithLock(ctx context.Context, fn func(context.Context) error) error { + ctx, cancel := r.withTimeout(ctx) + defer cancel() + + unlock, err := r.lock(ctx) + if err != nil { + return err + } + defer unlock() + return fn(ctx) +} diff --git a/gitx/merge.go b/gitx/merge.go new file mode 100644 index 0000000000000000000000000000000000000000..3ac5456e13404170d70364332005c9b32af14d21 --- /dev/null +++ b/gitx/merge.go @@ -0,0 +1,444 @@ +package gitx + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/filemode" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/go-git/go-git/v5/storage" + "github.com/go-git/go-git/v5/utils/merkletrie" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// MergeRequest asks for a proposal branch to be spliced onto the approved head. +type MergeRequest struct { + // Branch is the proposal branch, "proposals/42". + Branch string + + // Base is the proposal's base revision B: the approved-head sha the agent + // held when it opened the proposal. It does not move as the proposal + // accumulates edits, and it is what staleness is measured against. + // + // It is required rather than derived from a merge base. The caller has it + // (it is the recorded base_rev, the same value the agent keeps sending as + // If-Match), and inferring it would let a rewritten approved branch quietly + // change which revision the merge believes it was proposed against. + Base string + + // Meta is the merge commit's message, trailers and identities. + Meta CommitMeta +} + +// MergedDoc records what the merge did with one document. +type MergedDoc struct { + // DocID is the document's frontmatter id — the key the whole merge turns + // on, since paths move and ids do not. + DocID string + + // Path is where the blob landed in the new approved tree. + Path string + + // ProposalPath is where the proposal held it. It differs from Path exactly + // when the approved branch moved the document between the base and the + // head, which is the case that must not become a conflict. + ProposalPath string + + // Blob is the document's blob sha, taken unchanged from the proposal. + Blob plumbing.Hash + + // New is set when the document did not exist on the approved head. + New bool +} + +// Renamed reports whether the approved branch had moved this document under the +// proposal, so the proposal's blob followed the document to its new path. +func (d MergedDoc) Renamed() bool { return !d.New && d.Path != d.ProposalPath } + +// MergeResult describes a completed merge. +type MergeResult struct { + // Commit is the merge commit and the new approved head. + Commit plumbing.Hash + Tree plumbing.Hash + + // ApprovedHead is the head this merged onto — the first parent. + ApprovedHead plumbing.Hash + // ProposalHead is the proposal branch tip — the second parent. + ProposalHead plumbing.Hash + + // Docs is every document the merge carried over, sorted by path. + Docs []MergedDoc +} + +// Merge splices a proposal onto the approved head and moves the approved branch +// to the resulting two-parent merge commit. +// +// The model, exactly as designed: +// +// for d in F: # F is document ids, not paths +// if blob(path(d)@H) != blob(path(d)@B): # changed under us since B +// return 409 stale +// newTree = tree(H) with each d's blob replaced (at its path in H) +// commit newTree with parents [H, P.head] +// +// Two things about it are load-bearing and easy to get wrong: +// +// - Each changed document is resolved to its path *on the approved head* +// through its frontmatter id before its blob is compared. A rename between +// B and H therefore neither raises a conflict nor resurrects the document +// at the path it was moved away from: the proposal's blob is written at the +// head's path, and the old path is never touched. +// - There is no text merge. go-git v5 supports only FastForwardMerge, and the +// whole-document write grain makes three-way merging unnecessary anyway. +// A conflict is always "your base moved, re-propose" (*StaleError), which +// is trivial for an agent and comprehensible for a human. +// +// The build runs under the space write lock and ends in a compare-and-swap on +// the approved ref. Losing that swap means a human push landed underneath, so +// the merge is rebuilt against the new head rather than failing — up to a +// bounded number of attempts, after which it is ErrRefRace and nothing moved. +func (r *Repo) Merge(ctx context.Context, req MergeRequest) (MergeResult, error) { + ctx, cancel := r.withTimeout(ctx) + defer cancel() + + if !IsProposalBranch(req.Branch) { + return MergeResult{}, fmt.Errorf("%w: %q is not a %s* branch", ErrBadRev, req.Branch, ProposalPrefix) + } + if req.Base == "" { + return MergeResult{}, fmt.Errorf("gitx: merge of %q needs the proposal's base revision", req.Branch) + } + if err := req.Meta.validate(); err != nil { + return MergeResult{}, err + } + + unlock, err := r.lock(ctx) + if err != nil { + return MergeResult{}, err + } + defer unlock() + + approvedRef := plumbing.NewBranchReferenceName(r.approved) + var lastErr error + for attempt := 0; attempt < r.casBudget(); attempt++ { + if err := ctx.Err(); err != nil { + return MergeResult{}, err + } + old, err := r.repo.Reference(approvedRef, false) + if err != nil { + return MergeResult{}, fmt.Errorf("%w: approved branch %q in %s: %v", + ErrNotFound, r.approved, r.ref, err) + } + res, err := r.buildMerge(ctx, req, old.Hash()) + if err != nil { + return MergeResult{}, err + } + r.raceHook() + err = r.repo.Storer.CheckAndSetReference(plumbing.NewHashReference(approvedRef, res.Commit), old) + if err == nil { + return res, nil + } + if !errors.Is(err, storage.ErrReferenceHasChanged) { + return MergeResult{}, fmt.Errorf("gitx: update %s in %s: %w", approvedRef, r.ref, err) + } + lastErr = err + } + return MergeResult{}, fmt.Errorf("%w: %s in %s after %d attempts: %v", + ErrRefRace, approvedRef, r.ref, r.casBudget(), lastErr) +} + +// buildMerge does everything except moving the ref: it is called afresh on each +// compare-and-swap attempt, against the head it was handed. +func (r *Repo) buildMerge(ctx context.Context, req MergeRequest, head plumbing.Hash) (MergeResult, error) { + proposalHead, err := r.BranchHead(ctx, req.Branch) + if err != nil { + return MergeResult{}, err + } + base, err := r.ResolveRev(ctx, req.Base) + if err != nil { + return MergeResult{}, err + } + + // The base must still be on the approved branch. If it is not, the approved + // branch was rewritten under the proposal and every comparison below would + // be against a revision that is no longer part of the history. + onBranch, err := r.IsAncestor(ctx, base, head) + if err != nil { + return MergeResult{}, err + } + if !onBranch { + return MergeResult{}, &StaleError{Reason: StaleBaseDetached, Base: base, Head: head} + } + + baseTree, err := r.treeOf(base) + if err != nil { + return MergeResult{}, err + } + headTree, err := r.treeOf(head) + if err != nil { + return MergeResult{}, err + } + proposalTree, err := r.treeOf(proposalHead) + if err != nil { + return MergeResult{}, err + } + + changed, err := r.changedDocs(ctx, req.Branch, baseTree, proposalTree) + if err != nil { + return MergeResult{}, err + } + if len(changed) == 0 { + return MergeResult{}, fmt.Errorf("%w: %q changes no document against its base %s", + ErrUnsupportedChange, req.Branch, base) + } + + baseIdx, err := r.buildDocIndex(ctx, baseTree) + if err != nil { + return MergeResult{}, fmt.Errorf("gitx: index documents at base %s: %w", base, err) + } + headIdx, err := r.buildDocIndex(ctx, headTree) + if err != nil { + return MergeResult{}, fmt.Errorf("gitx: index documents at approved head %s: %w", head, err) + } + + node, err := r.loadTree(headTree, 0) + if err != nil { + return MergeResult{}, err + } + + docs := make([]MergedDoc, 0, len(changed)) + for _, c := range changed { + if baseIdx.duplicated[c.docID] || headIdx.duplicated[c.docID] { + return MergeResult{}, fmt.Errorf("%w: %s appears more than once on the approved branch", + ErrDuplicateDocID, c.docID) + } + basePath, inBase := baseIdx.byID[c.docID] + headPath, inHead := headIdx.byID[c.docID] + + switch { + case inBase && inHead: + // The design's comparison, resolved through the id rather than the + // path: a rename between B and H is not a change to the document. + baseBlob, err := blobAt(baseTree, basePath) + if err != nil { + return MergeResult{}, err + } + headBlob, err := blobAt(headTree, headPath) + if err != nil { + return MergeResult{}, err + } + if baseBlob != headBlob { + return MergeResult{}, &StaleError{ + Reason: StaleDocChanged, DocID: c.docID, Path: headPath, Base: base, Head: head, + } + } + case inBase && !inHead: + return MergeResult{}, &StaleError{ + Reason: StaleDocRemoved, DocID: c.docID, Path: basePath, Base: base, Head: head, + } + case !inBase && inHead: + return MergeResult{}, &StaleError{ + Reason: StaleDocAppeared, DocID: c.docID, Path: headPath, Base: base, Head: head, + } + default: + // A genuinely new document. Its path on the head must be free, or + // the splice would overwrite a document the proposal never read. + if _, taken := headIdx.byPath[c.path]; taken { + return MergeResult{}, &StaleError{ + Reason: StalePathTaken, DocID: c.docID, Path: c.path, Base: base, Head: head, + } + } + } + + target := c.path + if inHead { + target = headPath + } + if err := node.set(target, c.blob); err != nil { + return MergeResult{}, err + } + docs = append(docs, MergedDoc{ + DocID: c.docID, + Path: target, + ProposalPath: c.path, + Blob: c.blob, + New: !inHead, + }) + } + sort.Slice(docs, func(i, j int) bool { return docs[i].Path < docs[j].Path }) + + treeHash, err := node.write(r.repo.Storer) + if err != nil { + return MergeResult{}, err + } + // Two parents, approved head first. This is the whole of the "merge": a + // real merge commit, so the proposal stays visible in git log, built with + // explicit ParentHashes rather than any merge strategy. + parents := []plumbing.Hash{head, proposalHead} + commit, err := r.writeCommit(req.Meta, treeHash, parents) + if err != nil { + return MergeResult{}, err + } + return MergeResult{ + Commit: commit, + Tree: treeHash, + ApprovedHead: head, + ProposalHead: proposalHead, + Docs: docs, + }, nil +} + +// changedDoc is one document the proposal touched, as it exists on the proposal +// branch. +type changedDoc struct { + path string + blob plumbing.Hash + docID string +} + +// changedDocs is F: the documents a proposal changed against its base. +// +// Only additions and modifications of markdown documents are expressible. A +// deletion, a rename, or an edit to a non-document path (an attachment, or +// .spec.yml) is refused rather than guessed at — the write plane is a +// whole-document PUT and gives an agent no way to say "delete this" or "move +// this", and those operations are human-push-only by design. +func (r *Repo) changedDocs(ctx context.Context, branch string, baseTree, proposalTree *object.Tree) ([]changedDoc, error) { + changes, err := object.DiffTreeWithOptions(ctx, baseTree, proposalTree, object.DefaultDiffTreeOptions) + if err != nil { + return nil, fmt.Errorf("gitx: diff %q against its base in %s: %w", branch, r.ref, err) + } + out := make([]changedDoc, 0, len(changes)) + seenID := make(map[string]string, len(changes)) + for _, c := range changes { + action, err := c.Action() + if err != nil { + return nil, fmt.Errorf("gitx: classify change in %q: %w", branch, err) + } + switch action { + case merkletrie.Delete: + return nil, fmt.Errorf("%w: %q deletes %q; deletion is human-push-only", + ErrUnsupportedChange, branch, c.From.Name) + case merkletrie.Modify: + if c.From.Name != c.To.Name { + return nil, fmt.Errorf("%w: %q renames %q to %q; rename is human-push-only", + ErrUnsupportedChange, branch, c.From.Name, c.To.Name) + } + } + + path := c.To.Name + if !strings.HasSuffix(path, core.DocExt) { + return nil, fmt.Errorf("%w: %q changes %q, which is not a document; the merge is keyed by document id", + ErrUnsupportedChange, branch, path) + } + if err := core.ValidateDocPath(path); err != nil { + return nil, err + } + switch c.To.TreeEntry.Mode { + case filemode.Regular, filemode.Executable: + default: + return nil, fmt.Errorf("%w: %q at %q in %q is not a document blob", + ErrUnsupportedEntry, c.To.TreeEntry.Mode, path, branch) + } + + data, err := r.readBlob(c.To.TreeEntry.Hash, path) + if err != nil { + return nil, err + } + fm, _, err := core.ParseDocument(data) + if err != nil { + return nil, fmt.Errorf("gitx: %q in %q: %w", path, branch, err) + } + if err := core.ValidateDocID(fm.ID); err != nil { + return nil, fmt.Errorf("gitx: %q in %q: %w", path, branch, err) + } + if prev, dup := seenID[fm.ID]; dup { + return nil, fmt.Errorf("%w: %q changes %s at both %q and %q", + ErrDuplicateDocID, branch, fm.ID, prev, path) + } + seenID[fm.ID] = path + out = append(out, changedDoc{path: path, blob: c.To.TreeEntry.Hash, docID: fm.ID}) + } + sort.Slice(out, func(i, j int) bool { return out[i].path < out[j].path }) + return out, nil +} + +// docIndex maps a tree's documents both ways. +type docIndex struct { + // byID maps a document id to its path. Documents whose frontmatter is + // missing, unparseable or carries no valid id are absent from it. + byID map[string]string + + // byPath maps every document path to its id, "" when it has none. It is + // what stops a new document from being spliced over a path that is already + // occupied by something this merge cannot see. + byPath map[string]string + + // duplicated names ids that appear more than once in the tree. + duplicated map[string]bool +} + +// buildDocIndex walks a tree and resolves every document's id. +// +// A document whose frontmatter will not parse, or that carries no valid id, is +// recorded by path and left out of the id map rather than failing the walk. The +// registry and the update hook are what keep those out of the approved branch; +// making one malformed document — which --push-option=skip-validation can +// always produce — block every future merge in the space would turn a typo into +// an outage. It is still not silently overwritten: byPath keeps its path +// occupied, so a proposal targeting it is refused as stale. +// +// The same reasoning applies to duplicated ids: they are recorded, and only +// refused when the merge actually needs to resolve one of them. +func (r *Repo) buildDocIndex(ctx context.Context, t *object.Tree) (*docIndex, error) { + budget := r.newBudget() + var entries []docEntry + if err := r.collectDocs(ctx, t, "", 0, budget, &entries); err != nil { + return nil, err + } + idx := &docIndex{ + byID: make(map[string]string, len(entries)), + byPath: make(map[string]string, len(entries)), + duplicated: map[string]bool{}, + } + for _, e := range entries { + if err := ctx.Err(); err != nil { + return nil, err + } + data, err := r.readBlob(e.hash, e.path) + if err != nil { + return nil, err + } + if err := budget.read(e.path, int64(len(data))); err != nil { + return nil, err + } + idx.byPath[e.path] = "" + fm, _, err := core.ParseDocument(data) + if err != nil { + continue + } + if err := core.ValidateDocID(fm.ID); err != nil { + continue + } + idx.byPath[e.path] = fm.ID + if _, dup := idx.byID[fm.ID]; dup { + idx.duplicated[fm.ID] = true + continue + } + idx.byID[fm.ID] = e.path + } + return idx, nil +} + +// blobAt returns the blob sha of a document already known to be in the tree. +func blobAt(t *object.Tree, path string) (plumbing.Hash, error) { + entry, err := t.FindEntry(path) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("gitx: %q vanished from tree %s: %w", path, t.Hash, err) + } + return entry.Hash, nil +} diff --git a/gitx/merge_test.go b/gitx/merge_test.go new file mode 100644 index 0000000000000000000000000000000000000000..25b73a33c2fb70c5b8cc65ec808a5189065bea0e --- /dev/null +++ b/gitx/merge_test.go @@ -0,0 +1,540 @@ +package gitx + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/go-git/go-git/v5/plumbing" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// mergeMeta is the merge commit description a service layer would build. +func mergeMeta(n int) CommitMeta { + return CommitMeta{ + Message: "Merge proposals/1", + Trailers: []Trailer{{Key: "X-Agent-Session", Value: "8fb9c9a4"}}, + Author: agent(n), + Committer: owner(n), + } +} + +// TestMergeIsATwoParentTreeSplice pins the shape of the merge commit: a real +// merge commit whose first parent is the approved head and whose second is the +// proposal tip, so the proposal stays visible in git log. +func TestMergeIsATwoParentTreeSplice(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "approved text")}, + Write{Path: "specs/0008.md", Content: doc("SPEC-0008", "Other", "untouched")}, + ) + prop := openProposal(t, repo, "proposals/1", base.String(), meta("revise 0007", 2), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "proposed text")}) + + res, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(3)}) + if err != nil { + t.Fatalf("Merge: %v", err) + } + + if res.ApprovedHead != base { + t.Fatalf("ApprovedHead = %s, want %s", res.ApprovedHead, base) + } + if res.ProposalHead != prop.Commit { + t.Fatalf("ProposalHead = %s, want %s", res.ProposalHead, prop.Commit) + } + + c, err := repo.repo.CommitObject(res.Commit) + if err != nil { + t.Fatalf("CommitObject: %v", err) + } + if c.NumParents() != 2 { + t.Fatalf("merge commit has %d parents, want 2", c.NumParents()) + } + if c.ParentHashes[0] != base || c.ParentHashes[1] != prop.Commit { + t.Fatalf("parents = %v, want [%s %s]", c.ParentHashes, base, prop.Commit) + } + if !strings.Contains(c.Message, "X-Agent-Session: 8fb9c9a4") { + t.Fatalf("merge commit message lost its provenance trailers:\n%s", c.Message) + } + + // The approved branch moved to the merge commit. + head, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + if head != res.Commit { + t.Fatalf("approved head = %s, want the merge commit %s", head, res.Commit) + } + + // The tree is the approved tree with the one document replaced. + if got := mustRead(t, repo, DefaultApprovedBranch, "specs/0007.md"); !strings.Contains(got, "proposed text") { + t.Fatalf("merged 0007 = %q", got) + } + if got := mustRead(t, repo, DefaultApprovedBranch, "specs/0008.md"); !strings.Contains(got, "untouched") { + t.Fatalf("untouched 0008 = %q", got) + } + if len(res.Docs) != 1 || res.Docs[0].DocID != "SPEC-0007" || res.Docs[0].Path != "specs/0007.md" { + t.Fatalf("Docs = %+v", res.Docs) + } + if res.Docs[0].New || res.Docs[0].Renamed() { + t.Fatalf("an in-place edit reported New=%v Renamed=%v", res.Docs[0].New, res.Docs[0].Renamed()) + } +} + +// TestMergeFollowsARenameBetweenBaseAndHead is the case the whole id-keyed +// design exists for. The human moves a document on the approved branch while an +// agent is editing it at its old path. That must be neither a conflict nor a +// resurrection of the old path. +func TestMergeFollowsARenameBetweenBaseAndHead(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + body := doc("SPEC-0007", "Storage", "approved text") + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007-storage.md", Content: body}) + + // The agent proposes against the old path, from the old base. + openProposal(t, repo, "proposals/1", base.String(), meta("revise 0007", 2), + Write{Path: "specs/0007-storage.md", Content: doc("SPEC-0007", "Storage", "proposed text")}) + + // Meanwhile the human renames it — byte-identical content at a new path, + // which is what a rename is. Deletion and rename are human-push-only. + head := pushApproved(t, repo, ownerMeta("rename 0007", 3), + Write{Path: "archive/0007-storage.md", Content: body}, + Write{Path: "specs/0007-storage.md"}, // nil content deletes + ) + if got := docPaths(t, repo, head.String()); len(got) != 1 || got[0] != "archive/0007-storage.md" { + t.Fatalf("after the rename the approved branch holds %v", got) + } + + res, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(4)}) + if err != nil { + t.Fatalf("Merge across a rename = %v, want it to follow the document", err) + } + + // The proposal's blob landed at the document's path on the head... + if got := mustRead(t, repo, DefaultApprovedBranch, "archive/0007-storage.md"); !strings.Contains(got, "proposed text") { + t.Fatalf("merged document at its new path = %q", got) + } + // ...and the path it was moved away from was NOT resurrected. + paths := docPaths(t, repo, DefaultApprovedBranch) + if len(paths) != 1 || paths[0] != "archive/0007-storage.md" { + t.Fatalf("merged tree holds %v; the old path must not come back", paths) + } + + d := res.Docs[0] + if d.DocID != "SPEC-0007" || d.Path != "archive/0007-storage.md" || d.ProposalPath != "specs/0007-storage.md" { + t.Fatalf("MergedDoc = %+v", d) + } + if !d.Renamed() { + t.Fatal("MergedDoc.Renamed() = false, want true") + } +} + +// TestMergeRefusesAStaleBase covers the 409: the document changed on the +// approved branch under the proposal. +func TestMergeRefusesAStaleBase(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}) + openProposal(t, repo, "proposals/1", base.String(), meta("revise", 2), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "agent v2")}) + head := pushApproved(t, repo, ownerMeta("human edit", 3), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "human v2")}) + + _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(4)}) + if !errors.Is(err, ErrStale) { + t.Fatalf("Merge over a changed document = %v, want ErrStale", err) + } + var stale *StaleError + if !errors.As(err, &stale) { + t.Fatalf("error %v is not a *StaleError", err) + } + // The caller needs the current head to put in the 409 so the agent can + // refetch and re-propose against it. + if stale.Head != head { + t.Fatalf("StaleError.Head = %s, want the current approved head %s", stale.Head, head) + } + if stale.Base != base { + t.Fatalf("StaleError.Base = %s, want %s", stale.Base, base) + } + if stale.DocID != "SPEC-0007" || stale.Path != "specs/0007.md" { + t.Fatalf("StaleError does not name the document: %+v", stale) + } + if stale.Reason != StaleDocChanged { + t.Fatalf("StaleError.Reason = %q", stale.Reason) + } + + // Nothing moved. + if got, _ := repo.ApprovedHead(ctx); got != head { + t.Fatalf("a stale merge moved the approved branch to %s", got) + } +} + +func TestMergeStalenessCases(t *testing.T) { + cases := []struct { + name string + // setup runs after the base is pushed and the proposal is open; it + // makes the approved branch move underneath. root is the space's + // initial commit. + setup func(t *testing.T, r *Repo, root plumbing.Hash) + reason StaleReason + }{ + { + name: "document removed from the approved branch", + setup: func(t *testing.T, r *Repo, _ plumbing.Hash) { + pushApproved(t, r, ownerMeta("delete 0007", 5), Write{Path: "specs/0007.md"}) + }, + reason: StaleDocRemoved, + }, + { + name: "the approved branch was rewritten under the proposal", + setup: func(t *testing.T, r *Repo, root plumbing.Hash) { + // Rewind the approved branch past the base, as a force-push + // would: the recorded base is no longer an ancestor of the + // head, so every comparison would be against a revision that + // is not part of the history any more. + ref := plumbing.NewBranchReferenceName(r.ApprovedBranch()) + if err := r.repo.Storer.SetReference(plumbing.NewHashReference(ref, root)); err != nil { + t.Fatal(err) + } + }, + reason: StaleBaseDetached, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + root, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + body := doc("SPEC-0007", "Storage", "v1") + base := pushApproved(t, repo, ownerMeta("seed", 1), Write{Path: "specs/0007.md", Content: body}) + openProposal(t, repo, "proposals/1", base.String(), meta("revise", 2), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "agent v2")}) + tc.setup(t, repo, root) + + _, err = repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(6)}) + var stale *StaleError + if !errors.As(err, &stale) { + t.Fatalf("Merge = %v, want a *StaleError", err) + } + if stale.Reason != tc.reason { + t.Fatalf("StaleError.Reason = %q, want %q", stale.Reason, tc.reason) + } + if stale.Head.IsZero() { + t.Fatal("StaleError carries no head; the caller cannot answer the 409") + } + }) + } +} + +// TestMergeRefusesADocumentThatAppearedUnderIt guards the id collision: the +// approved branch gained a document with the same id after the base, so the +// proposal would silently overwrite work it never saw. +func TestMergeRefusesADocumentThatAppearedUnderIt(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + openProposal(t, repo, "proposals/1", base.String(), meta("add 0007", 2), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "agent draft")}) + pushApproved(t, repo, ownerMeta("human adds the same id elsewhere", 3), + Write{Path: "archive/0007.md", Content: doc("SPEC-0007", "Storage", "human draft")}) + + _, err = repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(4)}) + var stale *StaleError + if !errors.As(err, &stale) || stale.Reason != StaleDocAppeared { + t.Fatalf("Merge = %v, want StaleDocAppeared", err) + } +} + +// TestMergeRefusesToOverwriteAnOccupiedPath covers a new document whose path is +// already taken on the head by a different document. +func TestMergeRefusesToOverwriteAnOccupiedPath(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + openProposal(t, repo, "proposals/1", base.String(), meta("add 0007", 2), + Write{Path: "specs/next.md", Content: doc("SPEC-0007", "Storage", "agent draft")}) + pushApproved(t, repo, ownerMeta("human takes the path", 3), + Write{Path: "specs/next.md", Content: doc("SPEC-0009", "Something else", "human text")}) + + _, err = repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(4)}) + var stale *StaleError + if !errors.As(err, &stale) || stale.Reason != StalePathTaken { + t.Fatalf("Merge = %v, want StalePathTaken", err) + } + if got := mustRead(t, repo, DefaultApprovedBranch, "specs/next.md"); !strings.Contains(got, "human text") { + t.Fatalf("the occupied path was overwritten: %q", got) + } +} + +func TestMergeAddsANewDocument(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}) + openProposal(t, repo, "proposals/1", base.String(), meta("add 0010", 2), + Write{Path: "notes/2026-07-22.md", Content: doc("NOTE-0010", "Daily", "new note")}) + + res, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(3)}) + if err != nil { + t.Fatalf("Merge: %v", err) + } + if len(res.Docs) != 1 || !res.Docs[0].New { + t.Fatalf("Docs = %+v, want one new document", res.Docs) + } + paths := docPaths(t, repo, DefaultApprovedBranch) + if strings.Join(paths, ",") != "notes/2026-07-22.md,specs/0007.md" { + t.Fatalf("merged tree holds %v", paths) + } +} + +func TestMergeRefusesChangesTheModelCannotExpress(t *testing.T) { + ctx := context.Background() + + t.Run("deletion", func(t *testing.T) { + repo, _ := newSpace(t) + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}, + Write{Path: "specs/0008.md", Content: doc("SPEC-0008", "Other", "v1")}) + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); err != nil { + t.Fatal(err) + } + pushBranch(t, repo, "proposals/1", meta("delete 0008", 2), Write{Path: "specs/0008.md"}) + + _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(3)}) + if !errors.Is(err, ErrUnsupportedChange) { + t.Fatalf("Merge of a deletion = %v, want ErrUnsupportedChange", err) + } + if !strings.Contains(err.Error(), "human-push-only") { + t.Fatalf("error %q does not say where deletion belongs", err) + } + }) + + t.Run("rename", func(t *testing.T) { + repo, _ := newSpace(t) + body := doc("SPEC-0007", "Storage", "v1") + base := pushApproved(t, repo, ownerMeta("seed", 1), Write{Path: "specs/0007.md", Content: body}) + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); err != nil { + t.Fatal(err) + } + pushBranch(t, repo, "proposals/1", meta("move 0007", 2), + Write{Path: "archive/0007.md", Content: body}, + Write{Path: "specs/0007.md"}) + + _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(3)}) + if !errors.Is(err, ErrUnsupportedChange) { + t.Fatalf("Merge of a rename = %v, want ErrUnsupportedChange", err) + } + }) + + t.Run("non-document path", func(t *testing.T) { + repo, _ := newSpace(t) + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}) + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); err != nil { + t.Fatal(err) + } + pushBranch(t, repo, "proposals/1", meta("policy", 2), + Write{Path: core.PolicyFile, Content: []byte("review:\n auto_merge: [notes/**]\n")}) + + _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(3)}) + if !errors.Is(err, ErrUnsupportedChange) { + t.Fatalf("Merge of a %s change = %v, want ErrUnsupportedChange", core.PolicyFile, err) + } + }) + + t.Run("empty proposal", func(t *testing.T) { + repo, _ := newSpace(t) + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}) + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); err != nil { + t.Fatal(err) + } + _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(3)}) + if !errors.Is(err, ErrUnsupportedChange) { + t.Fatalf("Merge of an empty proposal = %v, want ErrUnsupportedChange", err) + } + }) + + t.Run("document with no id", func(t *testing.T) { + repo, _ := newSpace(t) + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + openProposal(t, repo, "proposals/1", base.String(), meta("no id", 2), + Write{Path: "notes/x.md", Content: []byte("---\ntitle: No id\nstatus: draft\n---\n\nbody\n")}) + _, err = repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(3)}) + if !errors.Is(err, core.ErrInvalidDocID) { + t.Fatalf("Merge of an id-less document = %v, want core.ErrInvalidDocID", err) + } + }) + + t.Run("two documents sharing an id", func(t *testing.T) { + repo, _ := newSpace(t) + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + openProposal(t, repo, "proposals/1", base.String(), meta("dupe", 2), + Write{Path: "notes/a.md", Content: doc("NOTE-0001", "A", "a")}, + Write{Path: "notes/b.md", Content: doc("NOTE-0001", "B", "b")}) + _, err = repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(3)}) + if !errors.Is(err, ErrDuplicateDocID) { + t.Fatalf("Merge of two documents sharing an id = %v, want ErrDuplicateDocID", err) + } + }) +} + +func TestMergeRejectsBadRequests(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + openProposal(t, repo, "proposals/1", base.String(), meta("add", 2), + Write{Path: "notes/a.md", Content: doc("NOTE-0001", "A", "a")}) + + if _, err := repo.Merge(ctx, MergeRequest{Branch: "main", Base: base.String(), Meta: mergeMeta(3)}); !errors.Is(err, ErrBadRev) { + t.Fatalf("Merge of the approved branch = %v, want ErrBadRev", err) + } + if _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Meta: mergeMeta(3)}); err == nil { + t.Fatal("Merge with no base succeeded") + } + if _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/99", Base: base.String(), Meta: mergeMeta(3)}); !errors.Is(err, ErrNotFound) { + t.Fatalf("Merge of an absent branch = %v, want ErrNotFound", err) + } + if _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String()}); err == nil { + t.Fatal("Merge with no commit identity succeeded") + } +} + +// TestMergeToleratesAMalformedDocumentElsewhere: one unparseable document on +// the approved branch — which --push-option=skip-validation can always produce — +// must not make every future merge in the space impossible. +func TestMergeToleratesAMalformedDocumentElsewhere(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}, + Write{Path: "notes/broken.md", Content: []byte("no frontmatter at all\n")}, + ) + openProposal(t, repo, "proposals/1", base.String(), meta("revise", 2), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v2")}) + + if _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(3)}); err != nil { + t.Fatalf("Merge alongside a malformed document = %v, want success", err) + } + if got := mustRead(t, repo, DefaultApprovedBranch, "specs/0007.md"); !strings.Contains(got, "v2") { + t.Fatalf("merged document = %q", got) + } + // But its path is still occupied: a proposal targeting it is refused, not + // silently allowed to overwrite it. + head, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + openProposal(t, repo, "proposals/2", head.String(), meta("claim the path", 4), + Write{Path: "notes/broken.md", Content: doc("NOTE-0002", "Mine now", "text")}) + _, err = repo.Merge(ctx, MergeRequest{Branch: "proposals/2", Base: head.String(), Meta: mergeMeta(5)}) + var stale *StaleError + if !errors.As(err, &stale) || stale.Reason != StalePathTaken { + t.Fatalf("Merge over a malformed document = %v, want StalePathTaken", err) + } +} + +// TestMergeRetriesALostRefCAS proves the compare-and-swap retry: the approved +// branch moves between the build and the swap, exactly as a concurrent native +// receive-pack push would move it, and the merge rebuilds against the new head +// rather than failing. +func TestMergeRetriesALostRefCAS(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}) + openProposal(t, repo, "proposals/1", base.String(), meta("add a note", 2), + Write{Path: "notes/a.md", Content: doc("NOTE-0001", "A", "agent note")}) + + // Move the approved branch out from under the merge, once, between the + // build and the swap. + var raced bool + repo.beforeCAS = func() { + if raced { + return + } + raced = true + pushApproved(t, repo, ownerMeta("concurrent human push", 3), + Write{Path: "specs/0009.md", Content: doc("SPEC-0009", "Late", "human text")}) + } + + res, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(4)}) + if err != nil { + t.Fatalf("Merge that lost a CAS = %v, want a retry", err) + } + if !raced { + t.Fatal("the race hook never fired") + } + + // The retry rebuilt onto the human's commit, so both changes survive. + paths := docPaths(t, repo, DefaultApprovedBranch) + if strings.Join(paths, ",") != "notes/a.md,specs/0007.md,specs/0009.md" { + t.Fatalf("merged tree holds %v; the concurrent push was lost", paths) + } + c, err := repo.repo.CommitObject(res.Commit) + if err != nil { + t.Fatal(err) + } + if c.ParentHashes[0] == base { + t.Fatal("the merge kept the stale first parent instead of rebuilding") + } +} + +func TestMergeGivesUpAfterTooManyLostCAS(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}) + openProposal(t, repo, "proposals/1", base.String(), meta("add a note", 2), + Write{Path: "notes/a.md", Content: doc("NOTE-0001", "A", "agent note")}) + + repo.attemptLimit = 2 + n := 0 + repo.beforeCAS = func() { + n++ + pushApproved(t, repo, ownerMeta("relentless human", 2+n), + Write{Path: "notes/human.md", Content: doc("NOTE-9999", "H", strings.Repeat("z", n))}) + } + _, err := repo.Merge(ctx, MergeRequest{Branch: "proposals/1", Base: base.String(), Meta: mergeMeta(9)}) + if !errors.Is(err, ErrRefRace) { + t.Fatalf("Merge that never wins the CAS = %v, want ErrRefRace", err) + } + if n != 2 { + t.Fatalf("merge made %d attempts, want 2", n) + } +} diff --git a/gitx/read.go b/gitx/read.go new file mode 100644 index 0000000000000000000000000000000000000000..024d3e06bc4c5bd575f406c98bc126c7a7f43af0 --- /dev/null +++ b/gitx/read.go @@ -0,0 +1,392 @@ +package gitx + +import ( + "context" + "errors" + "fmt" + "io" + "sort" + "strings" + + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/filemode" + "github.com/go-git/go-git/v5/plumbing/object" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// Document is one markdown document as it exists at a revision: its path in the +// tree, the sha of its blob, and the blob's bytes. +// +// Blob is the render cache key. It is content-addressed, so a cache entry keyed +// by it can never go stale and the cache can be dropped at any moment. +type Document struct { + Path string + Blob plumbing.Hash + Data []byte +} + +// docEntry is a document located in a tree, before its blob is read. +type docEntry struct { + path string + hash plumbing.Hash +} + +// ValidateRev checks a revision string before it reaches go-git's parser. +// +// What is accepted is a ref name or a full-or-abbreviated hex object id. What +// is rejected is revision arithmetic — "main^2", "HEAD~3", "main@{yesterday}" — +// because the read contract is a pinned immutable ?rev= or a branch, and every +// extra accepted spelling is one more thing the review UI, the index stamp and +// the agent's If-Match have to agree about. +// +// It deliberately does not try to tell an object id from a ref name. The two +// grammars overlap ("cafe" is both a plausible abbreviation and a perfectly +// legal branch name), so imposing a minimum length on "things that look hex" +// would reject real branches. Resolution decides which one it is; this function +// only decides whether it could be either. +func ValidateRev(rev string) error { + if rev == "" { + return fmt.Errorf("%w: empty revision", ErrBadRev) + } + if len(rev) > maxRevLen { + return fmt.Errorf("%w: revision is too long (%d > %d)", ErrBadRev, len(rev), maxRevLen) + } + if rev == "HEAD" { + return nil + } + return validateRefComponent("revision", rev) +} + +// ResolveRev resolves a revision to the commit it names. Anything that is not a +// commit in this repository — a tree sha, an unknown branch, a truncated id — +// is ErrNotFound; anything that is not a usable revision string at all is +// ErrBadRev. +func (r *Repo) ResolveRev(ctx context.Context, rev string) (plumbing.Hash, error) { + _, cancel := r.withTimeout(ctx) + defer cancel() + + if err := ValidateRev(rev); err != nil { + return plumbing.ZeroHash, err + } + h, err := r.repo.ResolveRevision(plumbing.Revision(rev)) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("%w: revision %q in %s: %v", ErrNotFound, rev, r.ref, err) + } + if _, err := r.repo.CommitObject(*h); err != nil { + return plumbing.ZeroHash, fmt.Errorf("%w: revision %q in %s does not name a commit: %v", + ErrNotFound, rev, r.ref, err) + } + return *h, nil +} + +// ApprovedHead returns the current tip of the approved branch. This is the +// value an agent's If-Match carries and the value a stale merge reports back. +func (r *Repo) ApprovedHead(ctx context.Context) (plumbing.Hash, error) { + return r.BranchHead(ctx, r.approved) +} + +// BranchHead returns the tip of a branch. A branch that does not exist is +// ErrNotFound. +func (r *Repo) BranchHead(ctx context.Context, branch string) (plumbing.Hash, error) { + _, cancel := r.withTimeout(ctx) + defer cancel() + + if err := ValidateBranch(branch); err != nil { + return plumbing.ZeroHash, err + } + ref, err := r.repo.Reference(plumbing.NewBranchReferenceName(branch), true) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("%w: branch %q in %s: %v", ErrNotFound, branch, r.ref, err) + } + return ref.Hash(), nil +} + +// IsAncestor reports whether a is reachable from b. It is what the service uses +// to decide an If-Match is still valid, and what the update hook uses to tell a +// fast-forward from a force-update before calling CheckRefUpdate. +func (r *Repo) IsAncestor(ctx context.Context, a, b plumbing.Hash) (bool, error) { + _, cancel := r.withTimeout(ctx) + defer cancel() + + if a == b { + return true, nil + } + ca, err := r.repo.CommitObject(a) + if err != nil { + return false, fmt.Errorf("%w: commit %s in %s: %v", ErrNotFound, a, r.ref, err) + } + cb, err := r.repo.CommitObject(b) + if err != nil { + return false, fmt.Errorf("%w: commit %s in %s: %v", ErrNotFound, b, r.ref, err) + } + return ca.IsAncestor(cb) +} + +// ListProposalBranches returns every proposals/* branch with its tip, sorted by +// name. Refs are the source of truth for whether a proposal exists, so this is +// what the reconciler scans to rebuild rows it lost. +func (r *Repo) ListProposalBranches(ctx context.Context) ([]Branch, error) { + _, cancel := r.withTimeout(ctx) + defer cancel() + + iter, err := r.repo.Branches() + if err != nil { + return nil, fmt.Errorf("gitx: list branches of %s: %w", r.ref, err) + } + var out []Branch + err = iter.ForEach(func(ref *plumbing.Reference) error { + name := ref.Name().Short() + if !IsProposalBranch(name) { + return nil + } + out = append(out, Branch{Name: name, Head: ref.Hash()}) + return nil + }) + if err != nil { + return nil, fmt.Errorf("gitx: list branches of %s: %w", r.ref, err) + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out, nil +} + +// Branch is a branch name paired with its tip. +type Branch struct { + Name string + Head plumbing.Hash +} + +// treeAt resolves a revision to its commit's tree. +func (r *Repo) treeAt(ctx context.Context, rev string) (*object.Tree, error) { + h, err := r.ResolveRev(ctx, rev) + if err != nil { + return nil, err + } + return r.treeOf(h) +} + +// treeOf returns the tree of a commit already resolved to a hash. +func (r *Repo) treeOf(commit plumbing.Hash) (*object.Tree, error) { + c, err := r.repo.CommitObject(commit) + if err != nil { + return nil, fmt.Errorf("%w: commit %s in %s: %v", ErrNotFound, commit, r.ref, err) + } + t, err := c.Tree() + if err != nil { + return nil, fmt.Errorf("gitx: tree of %s in %s: %w", commit, r.ref, err) + } + return t, nil +} + +// walkBudget tracks the per-walk entry and byte caps. +type walkBudget struct { + entries int + maxEntries int + bytes int64 + maxBytes int64 +} + +func (b *walkBudget) entry(path string) error { + b.entries++ + if b.entries > b.maxEntries { + return fmt.Errorf("%w: tree has more than %d entries (at %q)", ErrTooLarge, b.maxEntries, path) + } + return nil +} + +func (b *walkBudget) read(path string, n int64) error { + b.bytes += n + if b.bytes > b.maxBytes { + return fmt.Errorf("%w: walk exceeded %d bytes (at %q)", ErrTooLarge, b.maxBytes, path) + } + return nil +} + +func (r *Repo) newBudget() *walkBudget { + return &walkBudget{maxEntries: r.entryCap(), maxBytes: r.totalLimit()} +} + +// collectDocs lists every markdown document in a tree, depth-first and in tree +// order, without reading any blob. +// +// Entries that are not documents — attachments, .spec.yml, anything without the +// .md extension — are skipped, because they are legitimately not documents. An +// entry that occupies a document path but cannot be one is an error, not a +// skip: a symlinked or submoduled *.md would otherwise vanish from the index +// and the merge with nothing recording that it was ever there. +func (r *Repo) collectDocs(ctx context.Context, t *object.Tree, prefix string, depth int, b *walkBudget, out *[]docEntry) error { + if err := ctx.Err(); err != nil { + return err + } + if depth > maxTreeDepth { + return fmt.Errorf("%w: tree nesting deeper than %d at %q", ErrTooLarge, maxTreeDepth, prefix) + } + for _, e := range t.Entries { + path := e.Name + if prefix != "" { + path = prefix + "/" + e.Name + } + if err := b.entry(path); err != nil { + return err + } + switch e.Mode { + case filemode.Dir: + sub, err := object.GetTree(r.repo.Storer, e.Hash) + if err != nil { + return fmt.Errorf("gitx: read tree %s at %q in %s: %w", e.Hash, path, r.ref, err) + } + if err := r.collectDocs(ctx, sub, path, depth+1, b, out); err != nil { + return err + } + case filemode.Regular, filemode.Executable: + if !strings.HasSuffix(e.Name, core.DocExt) { + continue // an attachment, or .spec.yml + } + if err := core.ValidateDocPath(path); err != nil { + return fmt.Errorf("gitx: %s carries an unusable document path: %w", r.ref, err) + } + *out = append(*out, docEntry{path: path, hash: e.Hash}) + default: + if strings.HasSuffix(e.Name, core.DocExt) { + return fmt.Errorf("%w: %q in %s is a %s, not a document blob", + ErrUnsupportedEntry, path, r.ref, e.Mode) + } + } + } + return nil +} + +// readBlob reads a blob, refusing anything over the per-blob cap. The cap is +// checked against the object header first so an oversized blob is never +// materialized, and again against what was actually read so a lying header +// cannot get past it. +func (r *Repo) readBlob(h plumbing.Hash, path string) ([]byte, error) { + obj, err := r.repo.Storer.EncodedObject(plumbing.BlobObject, h) + if err != nil { + return nil, fmt.Errorf("%w: blob %s at %q in %s: %v", ErrNotFound, h, path, r.ref, err) + } + limit := r.blobLimit() + if obj.Size() > limit { + return nil, fmt.Errorf("%w: %q in %s is %d bytes (limit %d)", + ErrTooLarge, path, r.ref, obj.Size(), limit) + } + rd, err := obj.Reader() + if err != nil { + return nil, fmt.Errorf("gitx: read blob %s at %q in %s: %w", h, path, r.ref, err) + } + defer rd.Close() + + data, err := io.ReadAll(io.LimitReader(rd, limit+1)) + if err != nil { + return nil, fmt.Errorf("gitx: read blob %s at %q in %s: %w", h, path, r.ref, err) + } + if int64(len(data)) > limit { + return nil, fmt.Errorf("%w: %q in %s exceeds %d bytes", ErrTooLarge, path, r.ref, limit) + } + return data, nil +} + +// WalkDocuments calls fn for every markdown document at rev, in tree order. +// This is the seam that replaces warren's filesystem scan: the caller feeds the +// yielded documents to vault.FromPages and nothing downstream of Archive +// changes. +// +// fn's error stops the walk and is returned unwrapped, so a caller can use a +// sentinel of its own to stop early. +func (r *Repo) WalkDocuments(ctx context.Context, rev string, fn func(Document) error) error { + ctx, cancel := r.withTimeout(ctx) + defer cancel() + + t, err := r.treeAt(ctx, rev) + if err != nil { + return err + } + return r.walkTreeDocs(ctx, t, fn) +} + +func (r *Repo) walkTreeDocs(ctx context.Context, t *object.Tree, fn func(Document) error) error { + budget := r.newBudget() + var entries []docEntry + if err := r.collectDocs(ctx, t, "", 0, budget, &entries); err != nil { + return err + } + for _, e := range entries { + if err := ctx.Err(); err != nil { + return err + } + data, err := r.readBlob(e.hash, e.path) + if err != nil { + return err + } + if err := budget.read(e.path, int64(len(data))); err != nil { + return err + } + if err := fn(Document{Path: e.path, Blob: e.hash, Data: data}); err != nil { + return err + } + } + return nil +} + +// ListDocuments returns every markdown document at rev. It is WalkDocuments +// with the collection done for you; prefer WalkDocuments when the caller can +// stream. +func (r *Repo) ListDocuments(ctx context.Context, rev string) ([]Document, error) { + var docs []Document + if err := r.WalkDocuments(ctx, rev, func(d Document) error { + docs = append(docs, d) + return nil + }); err != nil { + return nil, err + } + return docs, nil +} + +// ReadDocument reads one markdown document by path at a revision. The path must +// be a valid document path; a path that exists but is not a document blob is +// ErrUnsupportedEntry rather than a silent miss. +func (r *Repo) ReadDocument(ctx context.Context, rev, path string) (Document, error) { + if err := core.ValidateDocPath(path); err != nil { + return Document{}, err + } + data, hash, err := r.ReadBlob(ctx, rev, path) + if err != nil { + return Document{}, err + } + return Document{Path: path, Blob: hash, Data: data}, nil +} + +// ReadBlob reads any blob by path at a revision — a document, .spec.yml, or an +// attachment — and returns its bytes and sha. Directories and non-blob entries +// are refused rather than reported as missing, because "you asked for a file +// and that is a directory" is a different bug from "it is not there". +func (r *Repo) ReadBlob(ctx context.Context, rev, path string) ([]byte, plumbing.Hash, error) { + ctx, cancel := r.withTimeout(ctx) + defer cancel() + + if err := core.ValidatePath(path); err != nil { + return nil, plumbing.ZeroHash, err + } + t, err := r.treeAt(ctx, rev) + if err != nil { + return nil, plumbing.ZeroHash, err + } + entry, err := t.FindEntry(path) + if err != nil { + if errors.Is(err, object.ErrEntryNotFound) || errors.Is(err, object.ErrDirectoryNotFound) { + return nil, plumbing.ZeroHash, fmt.Errorf("%w: %q at %q in %s", ErrNotFound, path, rev, r.ref) + } + return nil, plumbing.ZeroHash, fmt.Errorf("gitx: find %q at %q in %s: %w", path, rev, r.ref, err) + } + switch entry.Mode { + case filemode.Regular, filemode.Executable: + default: + return nil, plumbing.ZeroHash, fmt.Errorf("%w: %q at %q in %s is a %s", + ErrUnsupportedEntry, path, rev, r.ref, entry.Mode) + } + data, err := r.readBlob(entry.Hash, path) + if err != nil { + return nil, plumbing.ZeroHash, err + } + return data, entry.Hash, nil +} diff --git a/gitx/read_test.go b/gitx/read_test.go new file mode 100644 index 0000000000000000000000000000000000000000..3b3b524051b74088c1421337026cff8a9d7140c1 --- /dev/null +++ b/gitx/read_test.go @@ -0,0 +1,232 @@ +package gitx + +import ( + "context" + "errors" + "fmt" + "strings" + "testing" + + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/filemode" + "github.com/go-git/go-git/v5/plumbing/object" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +func TestWalkDocumentsYieldsOnlyMarkdown(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007-storage.md", Content: doc("SPEC-0007", "Storage", "alpha")}, + Write{Path: "notes/daily.md", Content: doc("NOTE-0001", "Daily", "beta")}, + Write{Path: core.PolicyFile, Content: []byte("review:\n auto_merge: [notes/**]\n")}, + Write{Path: "specs/diagram.png", Content: []byte{0x89, 'P', 'N', 'G'}}, + ) + + docs, err := repo.ListDocuments(ctx, DefaultApprovedBranch) + if err != nil { + t.Fatalf("ListDocuments: %v", err) + } + var paths []string + for _, d := range docs { + paths = append(paths, d.Path) + if d.Blob.IsZero() { + t.Fatalf("%q has no blob sha; it is the render cache key", d.Path) + } + if len(d.Data) == 0 { + t.Fatalf("%q has no data", d.Path) + } + } + want := []string{"notes/daily.md", "specs/0007-storage.md"} + if strings.Join(paths, ",") != strings.Join(want, ",") { + t.Fatalf("documents = %v, want %v (attachments and %s are not documents)", paths, want, core.PolicyFile) + } + + // .spec.yml is reachable as a blob even though it is not a document. + data, _, err := repo.ReadBlob(ctx, DefaultApprovedBranch, core.PolicyFile) + if err != nil { + t.Fatalf("ReadBlob(%s): %v", core.PolicyFile, err) + } + if _, err := core.ParsePolicy(data); err != nil { + t.Fatalf("ParsePolicy: %v", err) + } +} + +func TestReadIsTheSamePathForEveryRevision(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + first := pushApproved(t, repo, ownerMeta("v1", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "first")}) + pushApproved(t, repo, ownerMeta("v2", 2), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "second")}) + + // A pinned ?rev= of a superseded revision renders that revision, not the head. + if got := mustRead(t, repo, first.String(), "specs/0007.md"); !strings.Contains(got, "first") { + t.Fatalf("pinned read = %q, want the superseded revision", got) + } + if got := mustRead(t, repo, DefaultApprovedBranch, "specs/0007.md"); !strings.Contains(got, "second") { + t.Fatalf("branch read = %q, want the head revision", got) + } + + // A proposal branch reads through the very same call. + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + openProposal(t, repo, "proposals/1", base.String(), meta("propose", 3), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "draft")}) + if got := mustRead(t, repo, "proposals/1", "specs/0007.md"); !strings.Contains(got, "draft") { + t.Fatalf("proposal read = %q, want the draft", got) + } +} + +func TestReadMissingAndMalformedPaths(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "body")}) + + if _, err := repo.ReadDocument(ctx, DefaultApprovedBranch, "specs/nope.md"); !errors.Is(err, ErrNotFound) { + t.Fatalf("missing document = %v, want ErrNotFound", err) + } + // A directory is refused as a distinct class from "absent". + if _, _, err := repo.ReadBlob(ctx, DefaultApprovedBranch, "specs"); !errors.Is(err, ErrUnsupportedEntry) { + t.Fatalf("reading a directory = %v, want ErrUnsupportedEntry", err) + } + // core owns path validation; gitx does not re-derive it. + for _, p := range []string{"../escape.md", "/abs.md", "specs/.git/x.md", "specs/0007.txt"} { + if _, err := repo.ReadDocument(ctx, DefaultApprovedBranch, p); !errors.Is(err, core.ErrInvalidPath) { + t.Fatalf("ReadDocument(%q) = %v, want core.ErrInvalidPath", p, err) + } + } +} + +func TestOversizedBlobIsRefusedNotTruncated(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + body := strings.Repeat("x", 4096) + pushApproved(t, repo, ownerMeta("big", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", body)}, + Write{Path: "specs/small.md", Content: doc("SPEC-0008", "Small", "tiny")}, + ) + + // Squeeze the per-blob cap below the big document. A truncated markdown + // document would be indexed and served as though it were whole, so the read + // has to fail instead of returning a prefix. + repo.docLimit = 512 + + _, err := repo.ReadDocument(ctx, DefaultApprovedBranch, "specs/0007.md") + if !errors.Is(err, ErrTooLarge) { + t.Fatalf("oversized ReadDocument = %v, want ErrTooLarge", err) + } + if _, _, err := repo.ReadBlob(ctx, DefaultApprovedBranch, "specs/0007.md"); !errors.Is(err, ErrTooLarge) { + t.Fatalf("oversized ReadBlob = %v, want ErrTooLarge", err) + } + if _, err := repo.ListDocuments(ctx, DefaultApprovedBranch); !errors.Is(err, ErrTooLarge) { + t.Fatalf("walk over an oversized document = %v, want ErrTooLarge", err) + } + // The small document is unaffected. + if got := mustRead(t, repo, DefaultApprovedBranch, "specs/small.md"); !strings.Contains(got, "tiny") { + t.Fatalf("small document = %q", got) + } + + // The write path refuses the same content rather than storing something the + // read path could never return. + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); err != nil { + t.Fatal(err) + } + _, err = repo.CommitProposal(ctx, "proposals/1", + []Write{{Path: "specs/0009.md", Content: doc("SPEC-0009", "Huge", body)}}, meta("too big", 2)) + if !errors.Is(err, ErrTooLarge) { + t.Fatalf("oversized CommitProposal = %v, want ErrTooLarge", err) + } +} + +func TestWalkBudgetsBoundTotalBytesAndEntries(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + var writes []Write + for i := 1; i <= 6; i++ { + writes = append(writes, Write{ + Path: fmt.Sprintf("notes/%d.md", i), + Content: doc(fmt.Sprintf("NOTE-%04d", i), "Note", strings.Repeat("y", 200)), + }) + } + pushApproved(t, repo, ownerMeta("many", 1), writes...) + + repo.walkLimit = 300 + if _, err := repo.ListDocuments(ctx, DefaultApprovedBranch); !errors.Is(err, ErrTooLarge) { + t.Fatalf("walk over the byte budget = %v, want ErrTooLarge", err) + } + repo.walkLimit = 0 + repo.entryLimit = 3 + if _, err := repo.ListDocuments(ctx, DefaultApprovedBranch); !errors.Is(err, ErrTooLarge) { + t.Fatalf("walk over the entry budget = %v, want ErrTooLarge", err) + } +} + +func TestWalkRefusesADocumentThatIsNotABlob(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + // Build a tree carrying a symlink at a document path. The update hook is + // what keeps these out, but --push-option=skip-validation means one can + // exist, and quietly skipping it would drop a document out of the index + // with nothing recording that it was ever there. + target, err := repo.writeBlob("link", []byte("specs/0007.md")) + if err != nil { + t.Fatal(err) + } + head, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + tree, err := repo.treeOf(head) + if err != nil { + t.Fatal(err) + } + node, err := repo.loadTree(tree, 0) + if err != nil { + t.Fatal(err) + } + node.files["alias.md"] = object.TreeEntry{Name: "alias.md", Mode: filemode.Symlink, Hash: target} + treeHash, err := node.write(repo.repo.Storer) + if err != nil { + t.Fatal(err) + } + commit, err := repo.writeCommit(ownerMeta("symlink", 1), treeHash, []plumbing.Hash{head}) + if err != nil { + t.Fatal(err) + } + ref := plumbing.NewBranchReferenceName(repo.ApprovedBranch()) + if err := repo.repo.Storer.SetReference(plumbing.NewHashReference(ref, commit)); err != nil { + t.Fatal(err) + } + + if _, err := repo.ListDocuments(ctx, DefaultApprovedBranch); !errors.Is(err, ErrUnsupportedEntry) { + t.Fatalf("walk over a symlinked document = %v, want ErrUnsupportedEntry", err) + } +} + +func TestReadHonoursContextCancellation(t *testing.T) { + repo, _ := newSpace(t) + + pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "body")}) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := repo.ListDocuments(ctx, DefaultApprovedBranch); !errors.Is(err, context.Canceled) { + t.Fatalf("walk with a cancelled context = %v, want context.Canceled", err) + } +} diff --git a/gitx/refsrule.go b/gitx/refsrule.go new file mode 100644 index 0000000000000000000000000000000000000000..84553952a0a57d3fcc3fcbd659da8fc2ceae5a61 --- /dev/null +++ b/gitx/refsrule.go @@ -0,0 +1,252 @@ +package gitx + +import ( + "fmt" + "strconv" + "strings" + "unicode/utf8" + + "github.com/go-git/go-git/v5/plumbing" +) + +// ProposalPrefix is the namespace agents may write and nothing else. +const ProposalPrefix = "proposals/" + +// branchRefPrefix is the only ref namespace a space repository uses. Tags and +// notes are not part of the model, and a branch outside these two namespaces +// would be invisible to the reader, the reconciler and the index. +const branchRefPrefix = "refs/heads/" + +// maxRefLen caps a ref name. Well under any filesystem limit; the point is to +// keep a pathological name out of the hook's error message and the loose-ref +// directory, not to be permissive. +const maxRefLen = 255 + +// PrincipalKind is who is pushing. There is exactly one human on this instance +// and many agents, so this is the whole of the identity the refs rule needs: +// the boundary that matters is not human-versus-human, it is what an agent may +// move. +type PrincipalKind string + +const ( + // PrincipalHuman is the owner, pushing over SSH through receive-pack. Their + // push is the approval — there is nobody to review it. + PrincipalHuman PrincipalKind = "human" + + // PrincipalAgent is any agent token. One token or many, the constraint is + // the same and it is the one that bounds the damage a runaway agent can do. + PrincipalAgent PrincipalKind = "agent" +) + +// ParsePrincipalKind validates a principal kind arriving from a hook +// environment or a token row. +func ParsePrincipalKind(s string) (PrincipalKind, error) { + switch PrincipalKind(s) { + case PrincipalHuman, PrincipalAgent: + return PrincipalKind(s), nil + } + return "", fmt.Errorf("%w: principal %q is not one of human|agent", ErrRefRejected, s) +} + +// RefUpdate is one proposed ref move, as the update hook sees it. +type RefUpdate struct { + // Ref is the full ref name, e.g. "refs/heads/main". + Ref string + + // Old is the value the ref currently holds; zero means the ref is being + // created. + Old plumbing.Hash + + // New is the value proposed; zero means the ref is being deleted. + New plumbing.Hash + + // FastForward reports whether New is reachable from Old. The caller must + // compute it — Old.IsZero() || Repo.IsAncestor(ctx, Old, New) — because + // ancestry needs the object database and CheckRefUpdate is a pure function + // so that it can be exhaustively tested. It is ignored for deletions. + FastForward bool +} + +// CheckRefUpdate is the refs rule: may this principal move this ref? +// +// The human pushes to the approved branch. Agents may only write proposal +// branches. +// +// Concretely: +// +// - Only branches exist. A tag, a note or any other ref namespace is refused +// for both principals, because nothing in the model reads one and a ref the +// reader and reconciler do not know about is a place for content to rot. +// - The approved branch: the human only, fast-forward only, never deleted. +// A force-update is refused even from the owner — it would orphan every +// proposal's recorded base and silently rewrite approved text. +// - proposals/*: either principal, any update including a force-update or a +// delete. A proposal branch is scratch space; nothing reads it as canonical +// and rewriting one is how an agent revises its own work. +// +// It is a pure function of its arguments so hooks/ can call it without a +// repository and so every combination can be tested. A nil error means the +// update is permitted; every rejection wraps ErrRefRejected with a message fit +// to send back to the pushing client. +func CheckRefUpdate(principal PrincipalKind, approvedBranch string, u RefUpdate) error { + if _, err := ParsePrincipalKind(string(principal)); err != nil { + return err + } + if err := ValidateBranch(approvedBranch); err != nil { + return fmt.Errorf("%w: approved branch %q is unusable: %v", ErrRefRejected, approvedBranch, err) + } + if err := validateRefName(u.Ref); err != nil { + return fmt.Errorf("%w: %v", ErrRefRejected, err) + } + if u.Old.IsZero() && u.New.IsZero() { + return fmt.Errorf("%w: %s: update moves nothing (old and new are both zero)", ErrRefRejected, u.Ref) + } + if !strings.HasPrefix(u.Ref, branchRefPrefix) { + return fmt.Errorf("%w: %s: only branches under %s may be updated in a space", + ErrRefRejected, u.Ref, branchRefPrefix) + } + branch := strings.TrimPrefix(u.Ref, branchRefPrefix) + if err := ValidateBranch(branch); err != nil { + return fmt.Errorf("%w: %v", ErrRefRejected, err) + } + + switch { + case branch == approvedBranch: + if principal != PrincipalHuman { + return fmt.Errorf("%w: %s: an agent may only write %s*, not the approved branch", + ErrRefRejected, u.Ref, ProposalPrefix) + } + if u.New.IsZero() { + return fmt.Errorf("%w: %s: the approved branch may not be deleted", ErrRefRejected, u.Ref) + } + if !u.FastForward { + return fmt.Errorf("%w: %s: the approved branch takes fast-forwards only, not a force-update", + ErrRefRejected, u.Ref) + } + return nil + + case IsProposalBranch(branch): + return nil + + default: + return fmt.Errorf("%w: %s: a space carries the approved branch %q and %s* and nothing else", + ErrRefRejected, u.Ref, approvedBranch, ProposalPrefix) + } +} + +// IsProposalBranch reports whether a short branch name is in the proposal +// namespace. The bare name "proposals" is not: it is the namespace itself, and +// a branch by that name would block every proposal branch under it. +func IsProposalBranch(branch string) bool { + if !strings.HasPrefix(branch, ProposalPrefix) { + return false + } + if ValidateBranch(branch) != nil { + return false + } + return strings.TrimPrefix(branch, ProposalPrefix) != "" +} + +// ProposalBranch is the branch name for a proposal id, "proposals/42". The id +// is the proposal row's primary key, which is what the branch and the stable +// proposal URL share. +func ProposalBranch(id int64) (string, error) { + if id <= 0 { + return "", fmt.Errorf("%w: proposal id %d must be positive", ErrBadRev, id) + } + return ProposalPrefix + strconv.FormatInt(id, 10), nil +} + +// ParseProposalBranch recovers the proposal id from a branch name produced by +// ProposalBranch. A proposal branch with a non-numeric suffix is valid as a ref +// but carries no id, so ok is false rather than the id being guessed. +func ParseProposalBranch(branch string) (int64, bool) { + if !IsProposalBranch(branch) { + return 0, false + } + id, err := strconv.ParseInt(strings.TrimPrefix(branch, ProposalPrefix), 10, 64) + if err != nil || id <= 0 { + return 0, false + } + return id, true +} + +// ValidateBranch checks a short branch name ("main", "proposals/42"). +func ValidateBranch(branch string) error { + if err := validateRefComponent("branch", branch); err != nil { + return err + } + // Rejecting the full-ref spelling here is what stops "refs/heads/main" from + // being accepted as a branch and expanding to refs/heads/refs/heads/main. + if strings.HasPrefix(branch, "refs/") { + return fmt.Errorf("%w: branch %q must be a short name, not a full ref", ErrBadRev, branch) + } + return validateRefName(branchRefPrefix + branch) +} + +// validateRefName applies git's ref-name rules to a full ref, plus a length cap +// and a UTF-8 check that git-check-ref-format does not make. +func validateRefName(ref string) error { + if err := validateRefComponent("ref", ref); err != nil { + return err + } + if !strings.HasPrefix(ref, "refs/") { + return fmt.Errorf("%w: ref %q must start with \"refs/\"", ErrBadRev, ref) + } + if err := plumbing.ReferenceName(ref).Validate(); err != nil { + return fmt.Errorf("%w: ref %q: %v", ErrBadRev, ref, err) + } + return nil +} + +// validateRefComponent holds the checks shared by revisions, branches and full +// refs: length, UTF-8, no control characters, no traversal, and none of the +// bytes that make a name mean something else to a shell, to git's revision +// parser, or to a reader looking at a review page. +func validateRefComponent(kind, s string) error { + if s == "" { + return fmt.Errorf("%w: empty %s", ErrBadRev, kind) + } + if len(s) > maxRefLen { + return fmt.Errorf("%w: %s is too long (%d > %d)", ErrBadRev, kind, len(s), maxRefLen) + } + if !utf8.ValidString(s) { + return fmt.Errorf("%w: %s %q is not valid UTF-8", ErrBadRev, kind, s) + } + for _, r := range s { + if r < 0x20 || r == 0x7f { + return fmt.Errorf("%w: %s %q contains a control character", ErrBadRev, kind, s) + } + switch r { + case ' ', '~', '^', ':', '?', '*', '[', '\\', '"', '\'', '<', '>', '|', ';', '&', '$', '`', '\t': + return fmt.Errorf("%w: %s %q contains a disallowed character %q", ErrBadRev, kind, s, r) + } + } + if s[0] == '-' { + return fmt.Errorf("%w: %s %q must not start with '-'", ErrBadRev, kind, s) + } + if strings.HasPrefix(s, "/") || strings.HasSuffix(s, "/") { + return fmt.Errorf("%w: %s %q must not start or end with '/'", ErrBadRev, kind, s) + } + if strings.Contains(s, "..") { + return fmt.Errorf("%w: %s %q must not contain '..'", ErrBadRev, kind, s) + } + if strings.Contains(s, "//") { + return fmt.Errorf("%w: %s %q must not contain an empty component", ErrBadRev, kind, s) + } + if strings.Contains(s, "@{") { + return fmt.Errorf("%w: %s %q must not contain \"@{\"", ErrBadRev, kind, s) + } + if strings.HasSuffix(s, ".lock") || strings.Contains(s, ".lock/") { + return fmt.Errorf("%w: %s %q must not have a \".lock\" component", ErrBadRev, kind, s) + } + if s == "@" { + return fmt.Errorf("%w: %s must not be \"@\"", ErrBadRev, kind) + } + for _, comp := range strings.Split(s, "/") { + if strings.HasPrefix(comp, ".") || strings.HasSuffix(comp, ".") { + return fmt.Errorf("%w: %s %q component %q must not start or end with '.'", ErrBadRev, kind, s, comp) + } + } + return nil +} diff --git a/gitx/refsrule_test.go b/gitx/refsrule_test.go new file mode 100644 index 0000000000000000000000000000000000000000..05c992d39cc2d0a1d2dcec4434a9182e42b406ac --- /dev/null +++ b/gitx/refsrule_test.go @@ -0,0 +1,266 @@ +package gitx + +import ( + "errors" + "strings" + "testing" + + "github.com/go-git/go-git/v5/plumbing" +) + +var ( + hashA = plumbing.NewHash("1111111111111111111111111111111111111111") + hashB = plumbing.NewHash("2222222222222222222222222222222222222222") + zero = plumbing.ZeroHash +) + +// TestCheckRefUpdate is the table for the one rule the whole write model rests +// on: the human pushes to the approved branch, agents may only write +// proposals/*. +func TestCheckRefUpdate(t *testing.T) { + cases := []struct { + name string + principal PrincipalKind + update RefUpdate + allow bool + // want, when set, must appear in the rejection message: a hook error + // that does not say why is a support ticket. + want string + }{ + // --- the human and the approved branch ------------------------------- + { + name: "human fast-forwards the approved branch", + principal: PrincipalHuman, + update: RefUpdate{Ref: "refs/heads/main", Old: hashA, New: hashB, FastForward: true}, + allow: true, + }, + { + name: "human force-updates the approved branch", + principal: PrincipalHuman, + update: RefUpdate{Ref: "refs/heads/main", Old: hashA, New: hashB, FastForward: false}, + allow: false, + want: "fast-forwards only", + }, + { + name: "human deletes the approved branch", + principal: PrincipalHuman, + update: RefUpdate{Ref: "refs/heads/main", Old: hashA, New: zero}, + allow: false, + want: "may not be deleted", + }, + { + name: "human creates the approved branch", + principal: PrincipalHuman, + update: RefUpdate{Ref: "refs/heads/main", Old: zero, New: hashB, FastForward: true}, + allow: true, + }, + + // --- the agent and the approved branch ------------------------------- + { + name: "agent fast-forwards the approved branch", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/main", Old: hashA, New: hashB, FastForward: true}, + allow: false, + want: "not the approved branch", + }, + { + name: "agent deletes the approved branch", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/main", Old: hashA, New: zero}, + allow: false, + want: "not the approved branch", + }, + + // --- proposal branches ------------------------------------------------ + { + name: "agent creates a proposal branch", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/proposals/42", Old: zero, New: hashB, FastForward: true}, + allow: true, + }, + { + name: "agent force-updates its own proposal branch", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/proposals/42", Old: hashA, New: hashB, FastForward: false}, + allow: true, + }, + { + name: "agent deletes a proposal branch", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/proposals/42", Old: hashA, New: zero}, + allow: true, + }, + { + name: "human updates a proposal branch", + principal: PrincipalHuman, + update: RefUpdate{Ref: "refs/heads/proposals/42", Old: hashA, New: hashB, FastForward: true}, + allow: true, + }, + { + name: "a nested proposal branch is still a proposal branch", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/proposals/agent/7", Old: zero, New: hashB, FastForward: true}, + allow: true, + }, + { + name: "the bare proposals namespace is not a proposal branch", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/proposals", Old: zero, New: hashB, FastForward: true}, + allow: false, + want: "and nothing else", + }, + { + name: "a branch that merely starts with the word proposals", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/proposals-evil", Old: zero, New: hashB, FastForward: true}, + allow: false, + want: "and nothing else", + }, + + // --- everything else -------------------------------------------------- + { + name: "agent writes some other branch", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/scratch", Old: zero, New: hashB, FastForward: true}, + allow: false, + want: "and nothing else", + }, + { + name: "human writes some other branch", + principal: PrincipalHuman, + update: RefUpdate{Ref: "refs/heads/scratch", Old: zero, New: hashB, FastForward: true}, + allow: false, + want: "and nothing else", + }, + { + name: "human pushes a tag", + principal: PrincipalHuman, + update: RefUpdate{Ref: "refs/tags/v1.0.0", Old: zero, New: hashB, FastForward: true}, + allow: false, + want: "only branches", + }, + { + name: "notes ref", + principal: PrincipalHuman, + update: RefUpdate{Ref: "refs/notes/commits", Old: zero, New: hashB, FastForward: true}, + allow: false, + want: "only branches", + }, + { + name: "a bare name is not a ref", + principal: PrincipalHuman, + update: RefUpdate{Ref: "main", Old: hashA, New: hashB, FastForward: true}, + allow: false, + want: "must start with", + }, + { + name: "a traversing ref", + principal: PrincipalAgent, + update: RefUpdate{Ref: "refs/heads/proposals/../../main", Old: zero, New: hashB, FastForward: true}, + allow: false, + want: "'..'", + }, + { + name: "an update that moves nothing", + principal: PrincipalHuman, + update: RefUpdate{Ref: "refs/heads/main", Old: zero, New: zero}, + allow: false, + want: "moves nothing", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := CheckRefUpdate(tc.principal, "main", tc.update) + if tc.allow { + if err != nil { + t.Fatalf("CheckRefUpdate = %v, want allowed", err) + } + return + } + if err == nil { + t.Fatal("CheckRefUpdate allowed an update it should have rejected") + } + if !errors.Is(err, ErrRefRejected) && !errors.Is(err, ErrBadRev) { + t.Fatalf("rejection %v is neither ErrRefRejected nor ErrBadRev", err) + } + if tc.want != "" && !strings.Contains(err.Error(), tc.want) { + t.Fatalf("rejection %q does not mention %q", err, tc.want) + } + }) + } +} + +func TestCheckRefUpdateHonoursANonDefaultApprovedBranch(t *testing.T) { + u := RefUpdate{Ref: "refs/heads/approved", Old: hashA, New: hashB, FastForward: true} + if err := CheckRefUpdate(PrincipalHuman, "approved", u); err != nil { + t.Fatalf("human on the configured approved branch = %v, want allowed", err) + } + if err := CheckRefUpdate(PrincipalAgent, "approved", u); !errors.Is(err, ErrRefRejected) { + t.Fatalf("agent on the configured approved branch = %v, want rejected", err) + } + // "main" is nothing special once the space says otherwise. + main := RefUpdate{Ref: "refs/heads/main", Old: hashA, New: hashB, FastForward: true} + if err := CheckRefUpdate(PrincipalHuman, "approved", main); !errors.Is(err, ErrRefRejected) { + t.Fatalf("human on a non-approved branch = %v, want rejected", err) + } +} + +func TestCheckRefUpdateRejectsUnknownPrincipals(t *testing.T) { + u := RefUpdate{Ref: "refs/heads/proposals/1", Old: zero, New: hashB, FastForward: true} + for _, p := range []PrincipalKind{"", "root", "Human", "HUMAN"} { + if err := CheckRefUpdate(p, "main", u); !errors.Is(err, ErrRefRejected) { + t.Fatalf("CheckRefUpdate with principal %q = %v, want rejected", p, err) + } + } + if _, err := ParsePrincipalKind("agent"); err != nil { + t.Fatalf("ParsePrincipalKind(agent): %v", err) + } +} + +func TestCheckRefUpdateRejectsAnUnusableApprovedBranch(t *testing.T) { + u := RefUpdate{Ref: "refs/heads/main", Old: hashA, New: hashB, FastForward: true} + for _, branch := range []string{"", "refs/heads/main", "ma in", "-main", "a..b"} { + if err := CheckRefUpdate(PrincipalHuman, branch, u); !errors.Is(err, ErrRefRejected) { + t.Fatalf("approved branch %q = %v, want rejected", branch, err) + } + } +} + +func TestValidateBranch(t *testing.T) { + good := []string{"main", "approved", "proposals/42", "proposals/agent/7", "release-1.0"} + for _, b := range good { + if err := ValidateBranch(b); err != nil { + t.Fatalf("ValidateBranch(%q) = %v, want nil", b, err) + } + } + bad := []string{ + "", "refs/heads/main", "-main", "main/", "/main", "a//b", "a..b", + "a b", "a~b", "a^b", "a:b", "a?b", "a*b", "a[b", `a\b`, "a@{b", "@", + "main.lock", ".hidden", "trailing.", "a\tb", "a\x00b", + strings.Repeat("a", maxRefLen+1), + } + for _, b := range bad { + if err := ValidateBranch(b); err == nil { + t.Fatalf("ValidateBranch(%q) = nil, want an error", b) + } + } +} + +func TestValidateRev(t *testing.T) { + good := []string{ + "main", "proposals/42", "HEAD", + "1111111111111111111111111111111111111111", "1111111", + } + for _, rev := range good { + if err := ValidateRev(rev); err != nil { + t.Fatalf("ValidateRev(%q) = %v, want nil", rev, err) + } + } + bad := []string{"", "main^", "main~1", "main@{0}", "a b", "-main", "a..b", ".hidden"} + for _, rev := range bad { + if err := ValidateRev(rev); !errors.Is(err, ErrBadRev) { + t.Fatalf("ValidateRev(%q) = %v, want ErrBadRev", rev, err) + } + } +} diff --git a/gitx/write.go b/gitx/write.go new file mode 100644 index 0000000000000000000000000000000000000000..c208d8644ac6c99c0bfadc97f24a835c2dcd0ec2 --- /dev/null +++ b/gitx/write.go @@ -0,0 +1,436 @@ +package gitx + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + + "github.com/go-git/go-git/v5/plumbing" + "github.com/go-git/go-git/v5/plumbing/filemode" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/go-git/go-git/v5/plumbing/storer" + "github.com/go-git/go-git/v5/storage" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +// Trailer is one git trailer line, "Key: Value". +// +// This package renders trailers; it does not decide which ones exist. Which +// keys are required, what an agent identity string looks like and what goes in +// X-Agent-Session are authn/'s to own — putting that policy here would give the +// git layer an opinion about identity and give the two write surfaces two +// places to drift apart. What is enforced here is only that the rendered +// message cannot be forged: a value carrying a newline could otherwise +// manufacture trailers nobody supplied. +type Trailer struct { + Key string + Value string +} + +func (t Trailer) validate() error { + if t.Key == "" { + return fmt.Errorf("gitx: trailer key is required") + } + for i := 0; i < len(t.Key); i++ { + c := t.Key[i] + ok := (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' + if !ok { + return fmt.Errorf("gitx: trailer key %q contains a disallowed byte %q", t.Key, c) + } + } + if strings.ContainsAny(t.Value, "\n\r\x00") { + return fmt.Errorf("gitx: trailer %q value must be a single line", t.Key) + } + return nil +} + +// CommitMeta is everything a commit records besides its tree and parents. The +// caller supplies all of it: provenance is the product here, so nothing about +// authorship is defaulted or derived. +type CommitMeta struct { + // Message is the commit subject and body, without a trailer block. + Message string + + // Trailers are appended after a blank line, in order. Provenance lives here + // rather than in a Postgres-only audit table so it is visible in plain + // git log on any clone and cannot drift from the content it describes. + Trailers []Trailer + + // Author is who wrote the change — for an agent commit, the agent. Committer + // is who applied it, which is the service acting for the owner. + Author Signature + Committer Signature +} + +func (m CommitMeta) validate() error { + if strings.TrimSpace(m.Message) == "" { + return fmt.Errorf("gitx: commit message is required") + } + if strings.TrimSpace(strings.SplitN(m.Message, "\n", 2)[0]) == "" { + return fmt.Errorf("gitx: commit message must open with a non-empty subject line") + } + if err := m.Author.validate("author"); err != nil { + return err + } + if err := m.Committer.validate("committer"); err != nil { + return err + } + for _, t := range m.Trailers { + if err := t.validate(); err != nil { + return err + } + } + return nil +} + +// text renders the full commit message: the body, then a blank line, then the +// trailer block, then a trailing newline. +func (m CommitMeta) text() string { + var b strings.Builder + b.WriteString(strings.TrimRight(strings.ReplaceAll(m.Message, "\r\n", "\n"), "\n")) + if len(m.Trailers) > 0 { + b.WriteString("\n\n") + for i, t := range m.Trailers { + if i > 0 { + b.WriteByte('\n') + } + b.WriteString(t.Key) + b.WriteString(": ") + b.WriteString(t.Value) + } + } + b.WriteByte('\n') + return b.String() +} + +// Write is a whole-document replacement at a path. There is no patch form: the +// write plane takes whole documents because that is how agents work, and it is +// what makes the merge model pure plumbing. +type Write struct { + Path string + Content []byte +} + +// CommitResult describes a commit this package created. +type CommitResult struct { + Commit plumbing.Hash + Tree plumbing.Hash + Parents []plumbing.Hash + // Blobs maps each written path to its blob sha — the render cache key for + // the content that was just committed. + Blobs map[string]plumbing.Hash +} + +// mutableTree is a tree being built: subdirectories by name, plus the non- +// directory entries at this level. Trees are loaded whole and rewritten whole, +// which at this service's volume (tens of documents a day) costs a handful of +// tree-object reads and removes every incremental-rewrite bug class. +type mutableTree struct { + subs map[string]*mutableTree + files map[string]object.TreeEntry +} + +func newMutableTree() *mutableTree { + return &mutableTree{subs: map[string]*mutableTree{}, files: map[string]object.TreeEntry{}} +} + +// loadTree reads an existing tree into a mutableTree, recursively. +func (r *Repo) loadTree(t *object.Tree, depth int) (*mutableTree, error) { + if depth > maxTreeDepth { + return nil, fmt.Errorf("%w: tree nesting deeper than %d", ErrTooLarge, maxTreeDepth) + } + n := newMutableTree() + for _, e := range t.Entries { + if e.Mode == filemode.Dir { + sub, err := object.GetTree(r.repo.Storer, e.Hash) + if err != nil { + return nil, fmt.Errorf("gitx: read tree %s in %s: %w", e.Hash, r.ref, err) + } + child, err := r.loadTree(sub, depth+1) + if err != nil { + return nil, err + } + n.subs[e.Name] = child + continue + } + n.files[e.Name] = e + } + return n, nil +} + +// set places a blob at path, creating intermediate trees. A component that +// collides with an existing file, or a path whose final component is an +// existing directory, is an error: silently shadowing one would replace a +// document with something that is not one. +func (n *mutableTree) set(path string, hash plumbing.Hash) error { + comps := strings.Split(path, "/") + cur := n + for i, comp := range comps[:len(comps)-1] { + if _, clash := cur.files[comp]; clash { + return fmt.Errorf("gitx: cannot write %q: %q is a file", path, strings.Join(comps[:i+1], "/")) + } + next, ok := cur.subs[comp] + if !ok { + next = newMutableTree() + cur.subs[comp] = next + } + cur = next + } + last := comps[len(comps)-1] + if _, clash := cur.subs[last]; clash { + return fmt.Errorf("gitx: cannot write %q: it is a directory", path) + } + cur.files[last] = object.TreeEntry{Name: last, Mode: filemode.Regular, Hash: hash} + return nil +} + +// remove deletes the blob at path if present, pruning nothing else. It reports +// whether anything was removed. +func (n *mutableTree) remove(path string) bool { + comps := strings.Split(path, "/") + cur := n + for _, comp := range comps[:len(comps)-1] { + next, ok := cur.subs[comp] + if !ok { + return false + } + cur = next + } + last := comps[len(comps)-1] + if _, ok := cur.files[last]; !ok { + return false + } + delete(cur.files, last) + return true +} + +// empty reports whether the tree would encode to nothing. Git has no +// representation for an empty subtree, so those are dropped on write. +func (n *mutableTree) empty() bool { + if len(n.files) > 0 { + return false + } + for _, sub := range n.subs { + if !sub.empty() { + return false + } + } + return true +} + +// write encodes the tree and every non-empty subtree, returning the root hash. +func (n *mutableTree) write(store storer.EncodedObjectStorer) (plumbing.Hash, error) { + entries := make([]object.TreeEntry, 0, len(n.files)+len(n.subs)) + for name, e := range n.files { + e.Name = name + entries = append(entries, e) + } + for name, sub := range n.subs { + if sub.empty() { + continue + } + h, err := sub.write(store) + if err != nil { + return plumbing.ZeroHash, err + } + entries = append(entries, object.TreeEntry{Name: name, Mode: filemode.Dir, Hash: h}) + } + // Encode refuses unsorted entries, and git compares directory names as if + // they carried a trailing slash — TreeEntrySorter is that comparison. + sort.Sort(object.TreeEntrySorter(entries)) + + t := &object.Tree{Entries: entries} + obj := store.NewEncodedObject() + if err := t.Encode(obj); err != nil { + return plumbing.ZeroHash, fmt.Errorf("gitx: encode tree: %w", err) + } + h, err := store.SetEncodedObject(obj) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("gitx: store tree: %w", err) + } + return h, nil +} + +// writeBlob stores content as a blob, refusing anything over the document cap. +func (r *Repo) writeBlob(path string, content []byte) (plumbing.Hash, error) { + if limit := r.blobLimit(); int64(len(content)) > limit { + return plumbing.ZeroHash, fmt.Errorf("%w: %q is %d bytes (limit %d)", + ErrTooLarge, path, len(content), limit) + } + obj := r.repo.Storer.NewEncodedObject() + obj.SetType(plumbing.BlobObject) + obj.SetSize(int64(len(content))) + w, err := obj.Writer() + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("gitx: write blob for %q: %w", path, err) + } + if _, err := w.Write(content); err != nil { + w.Close() + return plumbing.ZeroHash, fmt.Errorf("gitx: write blob for %q: %w", path, err) + } + if err := w.Close(); err != nil { + return plumbing.ZeroHash, fmt.Errorf("gitx: write blob for %q: %w", path, err) + } + h, err := r.repo.Storer.SetEncodedObject(obj) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("gitx: store blob for %q: %w", path, err) + } + return h, nil +} + +// writeCommit stores a commit object. Parents are written in the order given, +// which is load-bearing for a merge: the first parent is the approved head. +func (r *Repo) writeCommit(meta CommitMeta, tree plumbing.Hash, parents []plumbing.Hash) (plumbing.Hash, error) { + if err := meta.validate(); err != nil { + return plumbing.ZeroHash, err + } + c := &object.Commit{ + Author: meta.Author.toGit(), + Committer: meta.Committer.toGit(), + Message: meta.text(), + TreeHash: tree, + ParentHashes: parents, + } + obj := r.repo.Storer.NewEncodedObject() + if err := c.Encode(obj); err != nil { + return plumbing.ZeroHash, fmt.Errorf("gitx: encode commit: %w", err) + } + h, err := r.repo.Storer.SetEncodedObject(obj) + if err != nil { + return plumbing.ZeroHash, fmt.Errorf("gitx: store commit: %w", err) + } + return h, nil +} + +// CreateProposalBranch cuts a new proposal branch at base. +// +// base is the agent's If-Match value: the space's approved-head sha at the time +// it read. Whether that value is still an ancestor of the approved head is the +// caller's 409 to raise (Repo.IsAncestor answers it); this function only cuts +// the branch, because the same check has to be spelled identically for REST and +// MCP and so belongs above the git layer. +func (r *Repo) CreateProposalBranch(ctx context.Context, branch, base string) (plumbing.Hash, error) { + ctx, cancel := r.withTimeout(ctx) + defer cancel() + + if !IsProposalBranch(branch) { + return plumbing.ZeroHash, fmt.Errorf("%w: %q is not a %s* branch", ErrBadRev, branch, ProposalPrefix) + } + head, err := r.ResolveRev(ctx, base) + if err != nil { + return plumbing.ZeroHash, err + } + + unlock, err := r.lock(ctx) + if err != nil { + return plumbing.ZeroHash, err + } + defer unlock() + + name := plumbing.NewBranchReferenceName(branch) + if _, err := r.repo.Reference(name, false); err == nil { + return plumbing.ZeroHash, fmt.Errorf("%w: branch %q in %s", ErrExists, branch, r.ref) + } else if !errors.Is(err, plumbing.ErrReferenceNotFound) { + return plumbing.ZeroHash, fmt.Errorf("gitx: read %s in %s: %w", name, r.ref, err) + } + if err := r.repo.Storer.SetReference(plumbing.NewHashReference(name, head)); err != nil { + return plumbing.ZeroHash, fmt.Errorf("gitx: create %s in %s: %w", name, r.ref, err) + } + return head, nil +} + +// CommitProposal commits whole-document blobs onto a proposal branch. +// +// It refuses any branch outside proposals/*: the approved branch moves in +// exactly two ways — a human push through receive-pack, or Merge — and a third +// door into it would be a way to land unreviewed agent output without a merge +// commit recording that it happened. +// +// The branch head is read, spliced and compare-and-swapped under the space +// lock, and the whole build is retried if the swap loses to a concurrent +// writer. +func (r *Repo) CommitProposal(ctx context.Context, branch string, writes []Write, meta CommitMeta) (CommitResult, error) { + ctx, cancel := r.withTimeout(ctx) + defer cancel() + + if !IsProposalBranch(branch) { + return CommitResult{}, fmt.Errorf("%w: %q is not a %s* branch; only Merge writes the approved branch", + ErrBadRev, branch, ProposalPrefix) + } + if len(writes) == 0 { + return CommitResult{}, fmt.Errorf("gitx: commit to %q has no writes", branch) + } + if err := meta.validate(); err != nil { + return CommitResult{}, err + } + seen := make(map[string]bool, len(writes)) + for _, w := range writes { + if err := core.ValidateDocPath(w.Path); err != nil { + return CommitResult{}, err + } + if seen[w.Path] { + return CommitResult{}, fmt.Errorf("gitx: commit to %q writes %q twice", branch, w.Path) + } + seen[w.Path] = true + } + + unlock, err := r.lock(ctx) + if err != nil { + return CommitResult{}, err + } + defer unlock() + + name := plumbing.NewBranchReferenceName(branch) + var lastErr error + for attempt := 0; attempt < r.casBudget(); attempt++ { + if err := ctx.Err(); err != nil { + return CommitResult{}, err + } + old, err := r.repo.Reference(name, false) + if err != nil { + return CommitResult{}, fmt.Errorf("%w: branch %q in %s: %v", ErrNotFound, branch, r.ref, err) + } + tree, err := r.treeOf(old.Hash()) + if err != nil { + return CommitResult{}, err + } + node, err := r.loadTree(tree, 0) + if err != nil { + return CommitResult{}, err + } + blobs := make(map[string]plumbing.Hash, len(writes)) + for _, w := range writes { + h, err := r.writeBlob(w.Path, w.Content) + if err != nil { + return CommitResult{}, err + } + if err := node.set(w.Path, h); err != nil { + return CommitResult{}, err + } + blobs[w.Path] = h + } + treeHash, err := node.write(r.repo.Storer) + if err != nil { + return CommitResult{}, err + } + parents := []plumbing.Hash{old.Hash()} + commit, err := r.writeCommit(meta, treeHash, parents) + if err != nil { + return CommitResult{}, err + } + r.raceHook() + err = r.repo.Storer.CheckAndSetReference(plumbing.NewHashReference(name, commit), old) + if err == nil { + return CommitResult{Commit: commit, Tree: treeHash, Parents: parents, Blobs: blobs}, nil + } + if !errors.Is(err, storage.ErrReferenceHasChanged) { + return CommitResult{}, fmt.Errorf("gitx: update %s in %s: %w", name, r.ref, err) + } + lastErr = err + } + return CommitResult{}, fmt.Errorf("%w: %s in %s after %d attempts: %v", + ErrRefRace, name, r.ref, r.casBudget(), lastErr) +} diff --git a/gitx/write_test.go b/gitx/write_test.go new file mode 100644 index 0000000000000000000000000000000000000000..8ca8b9c67192b3471fb5040051d1755daae3306b --- /dev/null +++ b/gitx/write_test.go @@ -0,0 +1,300 @@ +package gitx + +import ( + "context" + "errors" + "strings" + "testing" + + "sourcecraft.dev/bigbes/sr-ht-spec/core" +) + +func TestCreateProposalBranch(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}) + + head, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()) + if err != nil { + t.Fatalf("CreateProposalBranch: %v", err) + } + if head != base { + t.Fatalf("branch cut at %s, want the base %s", head, base) + } + got, err := repo.BranchHead(ctx, "proposals/1") + if err != nil { + t.Fatalf("BranchHead: %v", err) + } + if got != base { + t.Fatalf("proposals/1 = %s, want %s", got, base) + } + + // Cutting the same branch twice is a caller bug, not an idempotent retry: + // the second call would silently discard whatever the first accumulated. + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); !errors.Is(err, ErrExists) { + t.Fatalf("second CreateProposalBranch = %v, want ErrExists", err) + } + // Only proposals/* branches exist to be cut. + for _, b := range []string{"main", "scratch", "proposals", ""} { + if _, err := repo.CreateProposalBranch(ctx, b, base.String()); !errors.Is(err, ErrBadRev) { + t.Fatalf("CreateProposalBranch(%q) = %v, want ErrBadRev", b, err) + } + } + if _, err := repo.CreateProposalBranch(ctx, "proposals/2", "nosuchrev"); !errors.Is(err, ErrNotFound) { + t.Fatalf("CreateProposalBranch at an unknown base = %v, want ErrNotFound", err) + } +} + +func TestCommitProposalSplicesWholeDocuments(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base := pushApproved(t, repo, ownerMeta("seed", 1), + Write{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}, + Write{Path: "specs/deep/nested/0008.md", Content: doc("SPEC-0008", "Nested", "n1")}, + Write{Path: "specs/diagram.png", Content: []byte{0x89, 'P', 'N', 'G'}}, + ) + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); err != nil { + t.Fatal(err) + } + + res, err := repo.CommitProposal(ctx, "proposals/1", []Write{ + {Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v2")}, + {Path: "notes/new.md", Content: doc("NOTE-0001", "New", "fresh")}, + }, meta("revise 0007 and add a note", 2, + Trailer{Key: "X-Agent-Session", Value: "8fb9c9a4"}, + Trailer{Key: "X-Agent-Base", Value: base.String()}, + )) + if err != nil { + t.Fatalf("CommitProposal: %v", err) + } + + if len(res.Parents) != 1 || res.Parents[0] != base { + t.Fatalf("parents = %v, want [%s]", res.Parents, base) + } + if len(res.Blobs) != 2 { + t.Fatalf("Blobs = %v, want one per write", res.Blobs) + } + + c, err := repo.repo.CommitObject(res.Commit) + if err != nil { + t.Fatal(err) + } + // Author is the agent, committer the owner: the provenance the design + // requires, supplied whole by the caller. + if !strings.HasPrefix(c.Author.Name, "claude-code/") || c.Committer.Name != "bigbes" { + t.Fatalf("identities = author %q, committer %q", c.Author.Name, c.Committer.Name) + } + wantMsg := "revise 0007 and add a note\n\nX-Agent-Session: 8fb9c9a4\nX-Agent-Base: " + base.String() + "\n" + if c.Message != wantMsg { + t.Fatalf("message = %q, want %q", c.Message, wantMsg) + } + + // The splice replaced one document, added another, and left everything else + // — including the nested subtree and the attachment — exactly as it was. + if got := mustRead(t, repo, "proposals/1", "specs/0007.md"); !strings.Contains(got, "v2") { + t.Fatalf("edited document = %q", got) + } + if got := mustRead(t, repo, "proposals/1", "notes/new.md"); !strings.Contains(got, "fresh") { + t.Fatalf("added document = %q", got) + } + if got := mustRead(t, repo, "proposals/1", "specs/deep/nested/0008.md"); !strings.Contains(got, "n1") { + t.Fatalf("untouched nested document = %q", got) + } + png, _, err := repo.ReadBlob(ctx, "proposals/1", "specs/diagram.png") + if err != nil || len(png) != 4 { + t.Fatalf("attachment survived as %v, %v", png, err) + } + // The approved branch did not move. + if head, _ := repo.ApprovedHead(ctx); head != base { + t.Fatalf("a proposal commit moved the approved branch to %s", head) + } +} + +// TestCommitProposalRefusesTheApprovedBranch is the second half of the refs +// rule, enforced in-process: the approved branch moves by a human push or by +// Merge, and by nothing else. +func TestCommitProposalRefusesTheApprovedBranch(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + w := []Write{{Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v1")}} + for _, branch := range []string{DefaultApprovedBranch, "scratch", "proposals", "refs/heads/proposals/1"} { + _, err := repo.CommitProposal(ctx, branch, w, meta("nope", 2)) + if !errors.Is(err, ErrBadRev) { + t.Fatalf("CommitProposal(%q) = %v, want ErrBadRev", branch, err) + } + } +} + +func TestCommitProposalValidatesItsWrites(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); err != nil { + t.Fatal(err) + } + good := doc("SPEC-0007", "Storage", "v1") + + // Only document paths. .spec.yml is a real file in a space but it is not a + // document, and the write plane is a whole-document PUT. + for _, p := range []string{core.PolicyFile, "specs/0007.txt", "../escape.md", "/abs.md", ""} { + _, err := repo.CommitProposal(ctx, "proposals/1", []Write{{Path: p, Content: good}}, meta("bad path", 2)) + if !errors.Is(err, core.ErrInvalidPath) { + t.Fatalf("CommitProposal to %q = %v, want core.ErrInvalidPath", p, err) + } + } + // A commit with nothing in it is a caller bug. + if _, err := repo.CommitProposal(ctx, "proposals/1", nil, meta("empty", 2)); err == nil { + t.Fatal("CommitProposal with no writes succeeded") + } + // The same path twice in one commit: the second would silently win. + _, err = repo.CommitProposal(ctx, "proposals/1", []Write{ + {Path: "specs/0007.md", Content: good}, + {Path: "specs/0007.md", Content: doc("SPEC-0007", "Storage", "v2")}, + }, meta("twice", 2)) + if err == nil { + t.Fatal("CommitProposal writing one path twice succeeded") + } + // Identities are required. + if _, err := repo.CommitProposal(ctx, "proposals/1", []Write{{Path: "specs/0007.md", Content: good}}, + CommitMeta{Message: "no identity"}); err == nil { + t.Fatal("CommitProposal with no identities succeeded") + } + // The branch has to exist first. + if _, err := repo.CommitProposal(ctx, "proposals/9", []Write{{Path: "specs/0007.md", Content: good}}, + meta("absent", 2)); !errors.Is(err, ErrNotFound) { + t.Fatalf("CommitProposal to an absent branch = %v, want ErrNotFound", err) + } +} + +func TestCommitProposalAccumulatesEdits(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); err != nil { + t.Fatal(err) + } + first, err := repo.CommitProposal(ctx, "proposals/1", + []Write{{Path: "notes/a.md", Content: doc("NOTE-0001", "A", "one")}}, meta("first", 2)) + if err != nil { + t.Fatal(err) + } + second, err := repo.CommitProposal(ctx, "proposals/1", + []Write{{Path: "notes/b.md", Content: doc("NOTE-0002", "B", "two")}}, meta("second", 3)) + if err != nil { + t.Fatal(err) + } + if second.Parents[0] != first.Commit { + t.Fatalf("second commit parent = %s, want %s", second.Parents[0], first.Commit) + } + paths := docPaths(t, repo, "proposals/1") + if strings.Join(paths, ",") != "notes/a.md,notes/b.md" { + t.Fatalf("proposal branch holds %v", paths) + } +} + +func TestCommitProposalRetriesALostRefCAS(t *testing.T) { + repo, _ := newSpace(t) + ctx := context.Background() + + base, err := repo.ApprovedHead(ctx) + if err != nil { + t.Fatal(err) + } + if _, err := repo.CreateProposalBranch(ctx, "proposals/1", base.String()); err != nil { + t.Fatal(err) + } + var raced bool + repo.beforeCAS = func() { + if raced { + return + } + raced = true + pushBranch(t, repo, "proposals/1", meta("someone else", 3), + Write{Path: "notes/other.md", Content: doc("NOTE-0009", "Other", "landed first")}) + } + res, err := repo.CommitProposal(ctx, "proposals/1", + []Write{{Path: "notes/a.md", Content: doc("NOTE-0001", "A", "one")}}, meta("mine", 4)) + if err != nil { + t.Fatalf("CommitProposal that lost a CAS = %v, want a retry", err) + } + if !raced { + t.Fatal("the race hook never fired") + } + if res.Parents[0] == base { + t.Fatal("the retry kept the stale parent instead of rebuilding") + } + paths := docPaths(t, repo, "proposals/1") + if strings.Join(paths, ",") != "notes/a.md,notes/other.md" { + t.Fatalf("proposal branch holds %v; the concurrent write was lost", paths) + } +} + +func TestMutableTreeRefusesFileDirectoryCollisions(t *testing.T) { + repo, _ := newSpace(t) + + h, err := repo.writeBlob("x", []byte("x")) + if err != nil { + t.Fatal(err) + } + n := newMutableTree() + if err := n.set("specs/0007.md", h); err != nil { + t.Fatal(err) + } + // "specs/0007.md" is a file, so it cannot also be a directory... + if err := n.set("specs/0007.md/inner.md", h); err == nil { + t.Fatal("set under an existing file succeeded") + } + // ...and "specs" is a directory, so it cannot also be a file. + if err := n.set("specs", h); err == nil { + t.Fatal("set over an existing directory succeeded") + } +} + +func TestMutableTreeDropsEmptySubtrees(t *testing.T) { + repo, _ := newSpace(t) + + h, err := repo.writeBlob("x", []byte("x")) + if err != nil { + t.Fatal(err) + } + n := newMutableTree() + if err := n.set("a/b/c.md", h); err != nil { + t.Fatal(err) + } + withChild, err := n.write(repo.repo.Storer) + if err != nil { + t.Fatal(err) + } + if !n.remove("a/b/c.md") { + t.Fatal("remove reported nothing removed") + } + if n.remove("a/b/c.md") { + t.Fatal("remove reported a second removal") + } + empty, err := n.write(repo.repo.Storer) + if err != nil { + t.Fatal(err) + } + if empty == withChild { + t.Fatal("emptying the tree did not change its hash") + } + tree, err := repo.repo.TreeObject(empty) + if err != nil { + t.Fatal(err) + } + if len(tree.Entries) != 0 { + t.Fatalf("empty subtrees survived: %+v", tree.Entries) + } +}