A api/api.go => api/api.go +172 -0
@@ 0,0 1,172 @@
+// Package api is spec.sr.ht's REST write plane: the HTTP surface an agent PUTs
+// a whole document to in order to open or extend a proposal.
+//
+// It is one of the two agent-facing write surfaces — the other is mcpsrv's
+// spec_propose — and the design's rule is that both call the same
+// service.Propose rather than each implementing If-Match, provenance and
+// auto-merge for themselves. This package therefore holds no proposal logic: it
+// parses an HTTP request into a service.ProposeRequest, calls through, and maps
+// the result and the service's error sentinels onto status codes. A rule decided
+// here that service/ did not would be exactly the drift the shared layer exists
+// to prevent.
+//
+// # The contract
+//
+// PUT /api/v1/spaces/~owner/name/docs/<path>
+// If-Match: <base-rev-sha> # required: the approved head you read at
+// X-Proposal: <id> # optional: add to this open proposal
+//
+// The body is the whole document. `If-Match` is the base B — the approved-head
+// sha at the time the agent read — with one meaning shared across REST and MCP:
+// opening cuts the branch from it, adding is validated against the proposal's
+// fixed B. Title, rationale and the commit message ride in the query string, so
+// the body stays the document and nothing else. Every response carries the
+// proposal and its URL.
+//
+// # Who may write
+//
+// Proposing is agent-only; the human write path is native receive-pack. The
+// endpoint installs authn's principal middleware so the bearer token resolves,
+// and lets service.Propose refuse a non-agent — the ACL stays in service/,
+// spelled once, rather than here and there.
+package api
+
+import (
+ "context"
+ "encoding/json"
+ "errors"
+ "fmt"
+ "net/http"
+
+ "github.com/go-chi/chi/v5"
+ "github.com/go-chi/chi/v5/middleware"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/authn"
+ "sourcecraft.dev/bigbes/sr-ht-spec/service"
+)
+
+// maxBodyBytes caps the document a single PUT may carry. It is generous — a
+// document is prose, not an upload — and exists only so a runaway or hostile
+// client cannot make the daemon buffer an unbounded body into memory. gitx
+// enforces its own per-blob limit on the commit; this is the earlier, cheaper
+// refusal.
+const maxBodyBytes = 5 << 20 // 5 MiB
+
+// Writer is the write side of the orchestration layer this surface calls.
+// *service.Service satisfies it, and it is the same method mcpsrv's spec_propose
+// calls, which is what keeps the two write surfaces one implementation.
+type Writer interface {
+ Propose(ctx context.Context, req service.ProposeRequest) (service.ProposeResult, error)
+}
+
+// Options is everything a Server needs. New reports which one is missing rather
+// than failing later inside a handler.
+type Options struct {
+ // Writer is the orchestration layer. *service.Service satisfies it.
+ Writer Writer
+
+ // Resolver turns an agent bearer token into a principal. Handler installs
+ // its middleware; Register does not.
+ Resolver *authn.Resolver
+}
+
+// Server is the REST write endpoint. It is built once at startup and is safe
+// for concurrent use.
+type Server struct {
+ writer Writer
+ resolver *authn.Resolver
+}
+
+// New assembles the server over the seams in opts.
+func New(opts Options) (*Server, error) {
+ if opts.Writer == nil {
+ return nil, fmt.Errorf("api: Writer is required")
+ }
+ if opts.Resolver == nil {
+ return nil, fmt.Errorf("api: authn Resolver is required")
+ }
+ return &Server{writer: opts.Writer, resolver: opts.Resolver}, nil
+}
+
+// Handler returns the REST routes with panic recovery and authn's principal
+// middleware installed, so it can be mounted on a router that has none:
+//
+// router.Mount("/api", api.Handler())
+//
+// A caller whose router already resolves a principal uses Register instead;
+// installing the middleware twice is harmless — it is idempotent.
+func (s *Server) Handler() http.Handler {
+ r := chi.NewRouter()
+ r.Use(middleware.Recoverer)
+ r.Use(s.resolver.Middleware())
+ s.Register(r)
+ return r
+}
+
+// Register mounts the write routes onto r. It installs no middleware of its own;
+// the router it is handed must already resolve a principal into the request
+// context (authn.Resolver.Middleware), or every writer looks anonymous and is
+// refused.
+//
+// The document path is a single trailing wildcard: the space is "~owner/name"
+// and everything after "/docs/" is the document's path in the tree, decoded per
+// segment so a percent-encoded separator inside a name is not mistaken for one.
+func (s *Server) Register(r chi.Router) {
+ r.Put("/v1/spaces/~{owner}/{space}/docs/*", s.handlePut)
+}
+
+// proposeResponse is the JSON a successful write returns. It is the REST spelling
+// of mcpsrv's proposeOutput — the same fields, so an agent switching surfaces
+// reads the same answer — and it always carries the url, the whole point of the
+// review plane's entry contract.
+type proposeResponse struct {
+ Proposal int `json:"proposal"`
+ URL string `json:"url"`
+ Merged bool `json:"merged"`
+ State string `json:"state"`
+ Branch string `json:"branch"`
+ BaseRev string `json:"base_rev"`
+}
+
+// writeJSON writes v as the response body with the given status. A failure to
+// encode is logged into the void here — the header is already sent — but cannot
+// be helped, so it is deliberately not retried into a second WriteHeader.
+func writeJSON(w http.ResponseWriter, status int, v any) {
+ w.Header().Set("Content-Type", "application/json; charset=utf-8")
+ w.WriteHeader(status)
+ _ = json.NewEncoder(w).Encode(v)
+}
+
+// writeError maps a service error onto a status code and a JSON body an agent
+// can act on. The 4xx cases carry the service's message — the agent has to fix
+// and retry, and "which document failed the schema" is the whole point — while
+// a 5xx is a generic line, because an infrastructure failure's detail belongs in
+// the daemon's log, not a client's error field.
+func writeError(w http.ResponseWriter, err error) {
+ status := statusFor(err)
+ msg := err.Error()
+ if status >= 500 {
+ msg = "internal error"
+ }
+ writeJSON(w, status, map[string]string{"error": msg})
+}
+
+// statusFor maps the service sentinels onto HTTP. The staleness and
+// already-merged cases are the design's 409; a malformed document is 422, kept
+// distinct from the 403 of a principal that may not propose at all.
+func statusFor(err error) int {
+ switch {
+ case errors.Is(err, service.ErrForbidden):
+ return http.StatusForbidden
+ case errors.Is(err, service.ErrInvalid):
+ return http.StatusUnprocessableEntity
+ case errors.Is(err, service.ErrStale),
+ errors.Is(err, service.ErrAlreadyMerged),
+ errors.Is(err, service.ErrProposalNotOpen):
+ return http.StatusConflict
+ case errors.Is(err, service.ErrNotFound):
+ return http.StatusNotFound
+ default:
+ return http.StatusInternalServerError
+ }
+}
A api/api_test.go => api/api_test.go +218 -0
@@ 0,0 1,218 @@
+package api_test
+
+import (
+ "context"
+ "encoding/json"
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "github.com/go-chi/chi/v5"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/api"
+ "sourcecraft.dev/bigbes/sr-ht-spec/authn"
+ "sourcecraft.dev/bigbes/sr-ht-spec/core"
+ "sourcecraft.dev/bigbes/sr-ht-spec/service"
+)
+
+// fakeWriter captures the request the handler builds and returns a canned
+// result or error, so routing, header parsing and status mapping are checked
+// without a service, a repository or a database.
+type fakeWriter struct {
+ got service.ProposeRequest
+ res service.ProposeResult
+ err error
+}
+
+func (f *fakeWriter) Propose(_ context.Context, req service.ProposeRequest) (service.ProposeResult, error) {
+ f.got = req
+ return f.res, f.err
+}
+
+// router builds the write routes with a principal injected into every request,
+// bypassing token resolution — the handler reads the principal off the context,
+// and what put it there is not this package's concern.
+func router(t *testing.T, w api.Writer, p authn.Principal) http.Handler {
+ t.Helper()
+ srv, err := api.New(api.Options{Writer: w, Resolver: testResolver(t)})
+ if err != nil {
+ t.Fatalf("api.New: %v", err)
+ }
+ r := chi.NewRouter()
+ r.Use(func(next http.Handler) http.Handler {
+ return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+ next.ServeHTTP(rw, req.WithContext(authn.WithPrincipal(req.Context(), p)))
+ })
+ })
+ srv.Register(r)
+ return r
+}
+
+// testResolver is a resolver New requires but Register does not use. One token
+// store with no tokens is enough to construct it.
+func testResolver(t *testing.T) *authn.Resolver {
+ t.Helper()
+ res, err := authn.NewResolver("bigbes", stubTokens{})
+ if err != nil {
+ t.Fatalf("NewResolver: %v", err)
+ }
+ return res
+}
+
+type stubTokens struct{}
+
+func (stubTokens) LookupAgentToken(context.Context, []byte) (authn.AgentToken, error) {
+ return authn.AgentToken{}, authn.ErrUnknownToken
+}
+
+func agent() authn.Principal {
+ return authn.Principal{Kind: authn.KindAgent, Owner: "bigbes", Agent: "claude-code", Session: "s1"}
+}
+
+const specPath = "/v1/spaces/~bigbes/rfcs/docs/specs/0007-storage.md"
+
+// TestPutOpensProposal proves the whole open path: a PUT with a body and an
+// If-Match opens a proposal, returns 201 with the proposal and its url, and
+// forwards every field to the service.
+func TestPutOpensProposal(t *testing.T) {
+ w := &fakeWriter{res: service.ProposeResult{
+ Proposal: service.Proposal{ID: 42, Branch: "proposals/42", BaseRev: "deadbeef", State: core.StateOpen},
+ URL: "https://spec.srht.bigb.es/~bigbes/rfcs/p/42",
+ }}
+ h := router(t, w, agent())
+
+ req := httptest.NewRequest(http.MethodPut, specPath+"?title=Storage&message=add+it", strings.NewReader("the document"))
+ req.Header.Set("If-Match", "1f0c1d1a")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+
+ if rec.Code != http.StatusCreated {
+ t.Fatalf("status = %d, want 201; body %s", rec.Code, rec.Body)
+ }
+ var body struct {
+ Proposal int `json:"proposal"`
+ URL string `json:"url"`
+ State string `json:"state"`
+ }
+ if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
+ t.Fatalf("decode body: %v", err)
+ }
+ if body.Proposal != 42 || !strings.HasSuffix(body.URL, "/p/42") || body.State != "open" {
+ t.Fatalf("body = %+v, want proposal 42 open with its url", body)
+ }
+
+ // The request the service saw.
+ g := w.got
+ if g.Space != (core.SpaceRef{Owner: "bigbes", Name: "rfcs"}) {
+ t.Errorf("space = %v", g.Space)
+ }
+ if g.IfMatch != "1f0c1d1a" || g.Title != "Storage" || g.Message != "add it" {
+ t.Errorf("forwarded fields wrong: %+v", g)
+ }
+ if g.ProposalID != 0 {
+ t.Errorf("ProposalID = %d, want 0 for an open", g.ProposalID)
+ }
+ if len(g.Writes) != 1 || g.Writes[0].Path != "specs/0007-storage.md" || string(g.Writes[0].Content) != "the document" {
+ t.Errorf("writes = %+v", g.Writes)
+ }
+ if g.Principal != agent() {
+ t.Errorf("principal = %+v, want the agent on the context", g.Principal)
+ }
+}
+
+// TestPutAddsToExistingProposal proves X-Proposal routes to an add and returns
+// 200 rather than 201.
+func TestPutAddsToExistingProposal(t *testing.T) {
+ w := &fakeWriter{res: service.ProposeResult{
+ Proposal: service.Proposal{ID: 7, Branch: "proposals/7", State: core.StateOpen},
+ URL: "https://spec.srht.bigb.es/~bigbes/rfcs/p/7",
+ }}
+ h := router(t, w, agent())
+
+ req := httptest.NewRequest(http.MethodPut, specPath, strings.NewReader("doc"))
+ req.Header.Set("If-Match", "1f0c1d1a")
+ req.Header.Set("X-Proposal", "7")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+
+ if rec.Code != http.StatusOK {
+ t.Fatalf("status = %d, want 200; body %s", rec.Code, rec.Body)
+ }
+ if w.got.ProposalID != 7 {
+ t.Errorf("ProposalID = %d, want 7", w.got.ProposalID)
+ }
+}
+
+// TestPutRequiresIfMatch refuses a write with no base.
+func TestPutRequiresIfMatch(t *testing.T) {
+ w := &fakeWriter{}
+ h := router(t, w, agent())
+ req := httptest.NewRequest(http.MethodPut, specPath, strings.NewReader("doc"))
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code != http.StatusBadRequest {
+ t.Fatalf("status = %d, want 400", rec.Code)
+ }
+ if w.got.IfMatch != "" || w.got.Space != (core.SpaceRef{}) {
+ t.Fatal("service was called despite a missing If-Match")
+ }
+}
+
+// TestPutRejectsBadProposalHeader refuses a non-numeric X-Proposal.
+func TestPutRejectsBadProposalHeader(t *testing.T) {
+ h := router(t, &fakeWriter{}, agent())
+ req := httptest.NewRequest(http.MethodPut, specPath, strings.NewReader("doc"))
+ req.Header.Set("If-Match", "1f0c1d1a")
+ req.Header.Set("X-Proposal", "not-a-number")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code != http.StatusBadRequest {
+ t.Fatalf("status = %d, want 400", rec.Code)
+ }
+}
+
+// TestPutMapsServiceErrors proves each service sentinel reaches the right status.
+func TestPutMapsServiceErrors(t *testing.T) {
+ tests := []struct {
+ name string
+ err error
+ want int
+ }{
+ {"forbidden", service.ErrForbidden, http.StatusForbidden},
+ {"invalid", service.ErrInvalid, http.StatusUnprocessableEntity},
+ {"stale", service.ErrStale, http.StatusConflict},
+ {"already merged", service.ErrAlreadyMerged, http.StatusConflict},
+ {"not open", service.ErrProposalNotOpen, http.StatusConflict},
+ {"not found", service.ErrNotFound, http.StatusNotFound},
+ }
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ h := router(t, &fakeWriter{err: tc.err}, agent())
+ req := httptest.NewRequest(http.MethodPut, specPath, strings.NewReader("doc"))
+ req.Header.Set("If-Match", "1f0c1d1a")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code != tc.want {
+ t.Fatalf("status = %d, want %d", rec.Code, tc.want)
+ }
+ })
+ }
+}
+
+// TestPutDecodesEncodedPath proves a percent-encoded document path reaches the
+// service decoded, so a name with a space or a Cyrillic letter is addressable.
+func TestPutDecodesEncodedPath(t *testing.T) {
+ w := &fakeWriter{res: service.ProposeResult{Proposal: service.Proposal{ID: 1, State: core.StateOpen}}}
+ h := router(t, w, agent())
+ req := httptest.NewRequest(http.MethodPut, "/v1/spaces/~bigbes/rfcs/docs/notes/hello%20world.md", strings.NewReader("doc"))
+ req.Header.Set("If-Match", "1f0c1d1a")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code != http.StatusCreated {
+ t.Fatalf("status = %d, want 201; body %s", rec.Code, rec.Body)
+ }
+ if len(w.got.Writes) != 1 || w.got.Writes[0].Path != "notes/hello world.md" {
+ t.Fatalf("path = %q, want decoded", w.got.Writes[0].Path)
+ }
+}
A api/propose.go => api/propose.go +139 -0
@@ 0,0 1,139 @@
+package api
+
+import (
+ "io"
+ "net/http"
+ "net/url"
+ "strconv"
+ "strings"
+
+ "github.com/go-chi/chi/v5"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/authn"
+ "sourcecraft.dev/bigbes/sr-ht-spec/core"
+ "sourcecraft.dev/bigbes/sr-ht-spec/service"
+)
+
+// handlePut is the write plane's one handler: PUT a whole document to open or
+// extend a proposal.
+//
+// It parses the request into a service.ProposeRequest and calls through — the
+// document path from the URL, the base from If-Match, the optional target
+// proposal from X-Proposal, the title/rationale/message from the query string,
+// and the document from the body. The status is 201 when a new proposal was
+// opened and 200 when documents were added to an existing one; the merged field
+// says whether policy landed it immediately.
+func (s *Server) handlePut(w http.ResponseWriter, r *http.Request) {
+ ref := core.SpaceRef{Owner: chi.URLParam(r, "owner"), Name: chi.URLParam(r, "space")}
+
+ docPath, ok := unescapePath(chi.URLParam(r, "*"))
+ if !ok || docPath == "" {
+ writeJSON(w, http.StatusBadRequest, map[string]string{
+ "error": "the document path after /docs/ is missing or malformed",
+ })
+ return
+ }
+
+ base := strings.TrimSpace(r.Header.Get("If-Match"))
+ if base == "" {
+ writeJSON(w, http.StatusBadRequest, map[string]string{
+ "error": "If-Match is required: send the approved-head revision you read at, so the " +
+ "proposal has a base and a concurrent change cannot be clobbered",
+ })
+ return
+ }
+
+ proposalID, ok := parseProposalHeader(r.Header.Get("X-Proposal"))
+ if !ok {
+ writeJSON(w, http.StatusBadRequest, map[string]string{
+ "error": "X-Proposal must be a positive proposal id; omit it to open a new proposal",
+ })
+ return
+ }
+
+ body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, maxBodyBytes))
+ if err != nil {
+ // MaxBytesReader signals an over-limit body through a read error; there
+ // is no way to tell it from a truncated client here, so both are 413.
+ writeJSON(w, http.StatusRequestEntityTooLarge, map[string]string{
+ "error": "the document body could not be read or exceeds the size limit",
+ })
+ return
+ }
+
+ // The acting agent is resolved from the bearer token by the middleware.
+ // service.Propose refuses a non-agent, so an anonymous caller is a 403 there
+ // rather than a check duplicated here.
+ principal := authn.PrincipalFromContext(r.Context())
+
+ res, err := s.writer.Propose(r.Context(), service.ProposeRequest{
+ Space: ref,
+ Principal: principal,
+ ProposalID: proposalID,
+ Title: strings.TrimSpace(r.URL.Query().Get("title")),
+ Rationale: strings.TrimSpace(r.URL.Query().Get("rationale")),
+ IfMatch: base,
+ Message: strings.TrimSpace(r.URL.Query().Get("message")),
+ Writes: []service.DocumentWrite{{Path: docPath, Content: body}},
+ })
+ if err != nil {
+ writeError(w, err)
+ return
+ }
+
+ status := http.StatusOK
+ if proposalID == 0 {
+ status = http.StatusCreated
+ }
+ writeJSON(w, status, proposeResponse{
+ Proposal: res.Proposal.ID,
+ URL: res.URL,
+ Merged: res.Merged,
+ State: string(res.Proposal.State),
+ Branch: res.Proposal.Branch,
+ BaseRev: res.Proposal.BaseRev,
+ })
+}
+
+// parseProposalHeader reads the optional X-Proposal header. An empty header
+// means "open a new proposal" and is valid; a present value must be a positive
+// integer. It returns the id and whether the header was well-formed.
+func parseProposalHeader(v string) (int, bool) {
+ v = strings.TrimSpace(v)
+ if v == "" {
+ return 0, true
+ }
+ id, err := strconv.Atoi(v)
+ if err != nil || id <= 0 {
+ return 0, false
+ }
+ return id, true
+}
+
+// unescapePath decodes a chi trailing wildcard back into a document path,
+// per segment.
+//
+// chi routes on the raw (percent-encoded) path when the request had one, so a
+// document whose name carries a space or a Cyrillic letter arrives encoded.
+// Decoding per segment is deliberate: a %2F inside a segment is a literal slash
+// in a name, not a path separator, and joining decoded segments with "/" keeps
+// it from becoming one. It mirrors web/'s unescapePath so the read and write
+// surfaces address a document by exactly the same path grammar.
+func unescapePath(raw string) (string, bool) {
+ if raw == "" {
+ return "", true
+ }
+ segs := strings.Split(raw, "/")
+ for i, seg := range segs {
+ dec, err := url.PathUnescape(seg)
+ if err != nil {
+ return "", false
+ }
+ segs[i] = dec
+ }
+ return strings.Join(segs, "/"), true
+}
+
+// compile-time assertion that the production service satisfies the write this
+// surface needs.
+var _ Writer = (*service.Service)(nil)
M cmd/specsrht/main.go => cmd/specsrht/main.go +16 -5
@@ 68,6 68,7 @@ import (
"sourcecraft.dev/bigbes/sr-ht-core/config"
coreserver "sourcecraft.dev/bigbes/sr-ht-core/server"
+ "sourcecraft.dev/bigbes/sr-ht-spec/api"
"sourcecraft.dev/bigbes/sr-ht-spec/core"
"sourcecraft.dev/bigbes/sr-ht-spec/graph"
"sourcecraft.dev/bigbes/sr-ht-spec/hooks"
@@ 436,6 437,7 @@ type surfaces struct {
web *web.Server
mcp http.Handler
gql http.Handler
+ api http.Handler
}
// newSurfaces opens the index and builds the three read surfaces over it.
@@ 493,7 495,13 @@ func newSurfaces(conf ini.File, cfg service.Config, svc *service.Service, versio
return nil, fmt.Errorf("assemble the GraphQL surface: %w", err)
}
- return &surfaces{index: index, web: site, mcp: mcp, gql: gql.Handler()}, nil
+ rest, err := api.New(api.Options{Writer: svc, Resolver: svc.Resolver()})
+ if err != nil {
+ index.Close()
+ return nil, fmt.Errorf("assemble the REST write surface: %w", err)
+ }
+
+ return &surfaces{index: index, web: site, mcp: mcp, gql: gql.Handler(), api: rest.Handler()}, nil
}
func (s *surfaces) Close() error {
@@ 503,11 511,13 @@ func (s *surfaces) Close() error {
return s.index.Close()
}
-// mountWeb attaches the three read surfaces.
+// mountWeb attaches the HTTP surfaces: the three read surfaces (web, MCP,
+// GraphQL) and the REST write plane.
//
-// Order is load-bearing: /mcp and /query are registered before the web UI,
-// which mounts at "/" and would otherwise swallow them as document paths —
-// "spec" and "query" are legal space names as far as the router is concerned.
+// Order is load-bearing: /mcp, /query and /api are registered before the web
+// UI, which mounts at "/" and would otherwise swallow them as document paths —
+// "spec", "query" and "api" are all legal space names as far as the router is
+// concerned.
//
// Each surface installs its own authentication (they are fail-closed and agree
// on one ACL), so core-go's WithDefaultMiddleware is deliberately not used: its
@@ 519,6 529,7 @@ func mountWeb(router chi.Router, _ ini.File, s *surfaces) {
}
router.Handle("/mcp", s.mcp)
router.Handle("/query", s.gql)
+ router.Mount("/api", s.api)
router.Mount("/", s.web.Handler())
}
M service/propose.go => service/propose.go +10 -10
@@ 102,7 102,7 @@ func (s *Service) Propose(ctx context.Context, req ProposeRequest) (ProposeResul
return ProposeResult{}, fmt.Errorf("%w: %s may not propose; proposing is agent-only", ErrForbidden, req.Principal)
}
if len(req.Writes) == 0 {
- return ProposeResult{}, fmt.Errorf("%w: a proposal must write at least one document", ErrForbidden)
+ return ProposeResult{}, fmt.Errorf("%w: a proposal must write at least one document", ErrInvalid)
}
sp, err := s.OpenSpace(ctx, req.Space)
@@ 149,7 149,7 @@ func (s *Service) Propose(ctx context.Context, req ProposeRequest) (ProposeResul
// insert, branch cut, and the provenance-stamped commit.
func (s *Service) openNewProposal(ctx context.Context, sp *Space, req ProposeRequest, baseHash plumbing.Hash) (*db.Proposal, error) {
if req.Title == "" {
- return nil, fmt.Errorf("%w: opening a proposal requires a title", ErrForbidden)
+ return nil, fmt.Errorf("%w: opening a proposal requires a title", ErrInvalid)
}
base := baseHash.String()
@@ 255,11 255,11 @@ func (s *Service) addToProposal(ctx context.Context, sp *Space, req ProposeReque
func (s *Service) agentCommit(sp *Space, req ProposeRequest, base string) (gitx.CommitMeta, error) {
write, err := req.Principal.AgentWriteFor(base)
if err != nil {
- return gitx.CommitMeta{}, fmt.Errorf("%w: %v", ErrForbidden, err)
+ return gitx.CommitMeta{}, fmt.Errorf("%w: %v", ErrInvalid, err)
}
prov, err := s.cfg.Instance.Provenance(write)
if err != nil {
- return gitx.CommitMeta{}, fmt.Errorf("%w: %v", ErrForbidden, err)
+ return gitx.CommitMeta{}, fmt.Errorf("%w: %v", ErrInvalid, err)
}
message := req.Message
@@ 268,7 268,7 @@ func (s *Service) agentCommit(sp *Space, req ProposeRequest, base string) (gitx.
}
if message == "" {
return gitx.CommitMeta{},
- fmt.Errorf("%w: a write needs a commit message (or a title to borrow one from)", ErrForbidden)
+ fmt.Errorf("%w: a write needs a commit message (or a title to borrow one from)", ErrInvalid)
}
when := s.now().UTC()
@@ 302,21 302,21 @@ func (s *Service) validateWrites(ctx context.Context, sp *Space, writes []Docume
seen := make(map[string]string, len(writes))
for _, w := range writes {
if err := core.ValidateDocPath(w.Path); err != nil {
- return fmt.Errorf("%w: %v", ErrForbidden, err)
+ return fmt.Errorf("%w: %v", ErrInvalid, err)
}
fm, _, err := core.ParseDocument(w.Content)
if err != nil {
- return fmt.Errorf("%w: %s: %v", ErrForbidden, w.Path, err)
+ return fmt.Errorf("%w: %s: %v", ErrInvalid, w.Path, err)
}
if err := policy.Schema.ValidateFrontmatter(fm); err != nil {
- return fmt.Errorf("%w: %s: %v", ErrForbidden, w.Path, err)
+ return fmt.Errorf("%w: %s: %v", ErrInvalid, w.Path, err)
}
id, err := core.ParseDocID(fm.ID)
if err != nil {
- return fmt.Errorf("%w: %s: %v", ErrForbidden, w.Path, err)
+ return fmt.Errorf("%w: %s: %v", ErrInvalid, w.Path, err)
}
if prev, dup := seen[id.String()]; dup {
- return fmt.Errorf("%w: %s and %s both carry id %s", ErrForbidden, prev, w.Path, id)
+ return fmt.Errorf("%w: %s and %s both carry id %s", ErrInvalid, prev, w.Path, id)
}
seen[id.String()] = w.Path
}
M service/propose_test.go => service/propose_test.go +4 -4
@@ 53,8 53,8 @@ func TestProposeRejectsEmptyWrites(t *testing.T) {
Title: "t",
IfMatch: headRev,
})
- if !errors.Is(err, ErrForbidden) {
- t.Fatalf("Propose with no writes: err = %v, want ErrForbidden", err)
+ if !errors.Is(err, ErrInvalid) {
+ t.Fatalf("Propose with no writes: err = %v, want ErrInvalid", err)
}
}
@@ 124,8 124,8 @@ func TestValidateWrites(t *testing.T) {
if !tc.wantErr && err != nil {
t.Fatalf("validateWrites(%s) = %v, want nil", tc.name, err)
}
- if tc.wantErr && err != nil && !errors.Is(err, ErrForbidden) {
- t.Fatalf("validateWrites(%s) err = %v, want ErrForbidden", tc.name, err)
+ if tc.wantErr && err != nil && !errors.Is(err, ErrInvalid) {
+ t.Fatalf("validateWrites(%s) err = %v, want ErrInvalid", tc.name, err)
}
})
}
M service/service.go => service/service.go +9 -0
@@ 52,6 52,15 @@ var (
// a refusal of the principal, not of the request.
ErrForbidden = errors.New("service: forbidden")
+ // ErrInvalid marks a write the principal may make but the request itself is
+ // malformed: a document whose frontmatter does not parse or fails the
+ // schema, a path outside the tree, two uploads claiming one id, an open with
+ // no title, a write with no documents. It is the write plane's 400/422, kept
+ // apart from ErrForbidden so a surface does not answer "bad document" with
+ // "you are not allowed" — a distinction that matters most to the agent that
+ // has to fix and retry.
+ ErrInvalid = errors.New("service: invalid request")
+
// ErrStale marks a proposal whose base moved under it: the write plane's
// 409. It wraps a gitx staleness reason, so a caller that wants to tell the
// agent which document went stale type-asserts to *gitx.StaleError; one that