~bigbes/sr-ht-spec

7f779fef12194d49b9ce97ad4e2a80af1c3d6358 — Eugene Blikh 26 days ago 3c563e4
feat(web): review queue — inbox + policy-merged digest (Phase 4)

The backstop for work no link reached. /inbox lists every open proposal
on the instance ("waiting on you") and, below it, the digest of recently
policy-merged content — the firehose a human sees after the fact, which
is the whole reason approval=policy is kept distinct from human.

- service.InboxProposals / DigestProposals list instance-wide (one
  reviewer, so a per-space inbox would make them hunt), mapping each
  stored proposal's space_id back to a reference once from the space list.
- web/inbox.go + inbox.html render the two sections; the landing page
  links the queue for a logged-in owner.

Follow-up: the digest currently shows recent policy-merges rather than
"since you last looked" — the digest_mark table exists to track that, but
advancing it is a write and GET stays pure. Filed separately.
M service/review.go => service/review.go +59 -0
@@ 94,6 94,65 @@ func bytesEqual(a, b []byte) bool {
	return true
}

// InboxProposals is every open proposal on the instance, newest first — the
// reviewer's queue, "N proposals waiting on you". It is instance-wide because
// there is one reviewer: a per-space inbox would make them visit each space to
// find what a link never reached them about.
//
// The digest and this share the mapping from a stored proposal's space_id back
// to a reference, done once from the space list rather than a lookup per row.
func (s *Service) InboxProposals(ctx context.Context) ([]Proposal, error) {
	return s.proposalsInState(ctx, core.StateOpen, false, 0)
}

// DigestProposals is the recently policy-merged proposals — the firehose
// digest. Auto-merged content never stops for review, so this is where a human
// sees it after the fact; the design's whole reason for keeping approval=policy
// distinct from human is so this list can exist. limit bounds it; <= 0 is a
// sane default.
func (s *Service) DigestProposals(ctx context.Context, limit int) ([]Proposal, error) {
	if limit <= 0 {
		limit = 20
	}
	return s.proposalsInState(ctx, core.StateMerged, true, limit)
}

// proposalsInState lists proposals in one state instance-wide, optionally
// keeping only the policy-approved ones (the digest), and maps each onto its
// space reference. A row whose space no longer lists is skipped rather than
// errored: a deleted space takes its proposals out of every human-facing view,
// and a dangling row is the reconciler's to notice, not this read's to fail on.
func (s *Service) proposalsInState(ctx context.Context, state core.ProposalState, policyOnly bool, limit int) ([]Proposal, error) {
	spaces, err := s.ListSpaces(ctx)
	if err != nil {
		return nil, err
	}
	refByID := make(map[int]core.SpaceRef, len(spaces))
	for _, sp := range spaces {
		refByID[sp.ID] = sp.Ref
	}

	rows, err := s.store.ListProposalsByState(ctx, state, 0)
	if err != nil {
		return nil, fmt.Errorf("service: list %s proposals: %w", state, err)
	}
	out := make([]Proposal, 0, len(rows))
	for _, p := range rows {
		if policyOnly && p.Approval != core.ApprovalPolicy {
			continue
		}
		ref, ok := refByID[p.SpaceID]
		if !ok {
			continue
		}
		out = append(out, proposalView(p, ref))
		if limit > 0 && len(out) == limit {
			break
		}
	}
	return out, nil
}

// MergeHuman lands a proposal on the owner's approval — the review page's
// approve button. It is Merge with the approval kind fixed, so the surface does
// not choose it: a browser approve is always human, and a caller that could pass

M service/review_test.go => service/review_test.go +55 -0
@@ 4,8 4,63 @@ import (
	"bytes"
	"context"
	"testing"

	"sourcecraft.dev/bigbes/sr-ht-spec/core"
)

// TestInboxAndDigest proves the instance-wide queues: an open proposal lands in
// the inbox, a policy-auto-merged one lands in the digest, and neither shows in
// the other.
func TestInboxAndDigest(t *testing.T) {
	svc, _ := newTestService(t)
	ctx := context.Background()
	sp, err := svc.CreateSpace(ctx, fxSpace)
	if err != nil {
		t.Fatalf("CreateSpace: %v", err)
	}
	// A policy that auto-merges notes/, so one proposal lands and one waits.
	commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
		".spec.yml": []byte("review:\n  auto_merge: [notes/**]\n"),
	})
	base, err := sp.Repo.ApprovedHead(ctx)
	if err != nil {
		t.Fatalf("ApprovedHead: %v", err)
	}

	// Auto-merges (notes/).
	auto, err := svc.Propose(ctx, ProposeRequest{
		Space: fxSpace, Principal: agentPrincipal(), Title: "note", IfMatch: base.String(),
		Message: "note", Writes: []DocumentWrite{{Path: "notes/a.md", Content: mdDoc("N-1", "A", "b")}},
	})
	if err != nil || !auto.Merged {
		t.Fatalf("auto-merge propose: merged=%v err=%v", auto.Merged, err)
	}
	// Stays open (specs/).
	open, err := svc.Propose(ctx, ProposeRequest{
		Space: fxSpace, Principal: agentPrincipal(), Title: "spec", IfMatch: base.String(),
		Message: "spec", Writes: []DocumentWrite{{Path: "specs/b.md", Content: mdDoc("S-1", "B", "c")}},
	})
	if err != nil || open.Merged {
		t.Fatalf("open propose: merged=%v err=%v", open.Merged, err)
	}

	inbox, err := svc.InboxProposals(ctx)
	if err != nil {
		t.Fatalf("InboxProposals: %v", err)
	}
	if len(inbox) != 1 || inbox[0].ID != open.Proposal.ID {
		t.Fatalf("inbox = %+v, want only the open proposal %d", inbox, open.Proposal.ID)
	}

	digest, err := svc.DigestProposals(ctx, 0)
	if err != nil {
		t.Fatalf("DigestProposals: %v", err)
	}
	if len(digest) != 1 || digest[0].ID != auto.Proposal.ID || digest[0].Approval != core.ApprovalPolicy {
		t.Fatalf("digest = %+v, want only the policy-merged proposal %d", digest, auto.Proposal.ID)
	}
}

// TestProposalDiffReturnsChangedDocuments proves ProposalDiff returns exactly
// the documents a proposal changes — a modified one with its base and proposed
// content, and an added one marked new — and not the documents it leaves alone.

A web/inbox.go => web/inbox.go +79 -0
@@ 0,0 1,79 @@
package web

import (
	"net/http"
	"strconv"

	"sourcecraft.dev/bigbes/sr-ht-spec/service"
)

// inboxData is the review-queue page: the open proposals waiting on the owner,
// and the digest of what auto-merged without stopping for review.
type inboxData struct {
	Open   []proposalRow
	Digest []proposalRow
}

// proposalRow is one proposal as a listing line: enough to decide whether to
// open it, and the link that does.
type proposalRow struct {
	Href     string
	ID       int
	Space    string
	Title    string
	Agent    string
	Approval string
}

// handleInbox renders the review queue: every open proposal on the instance,
// plus the digest of recently policy-merged content. It is the backstop the
// design describes — the link an agent hands you is the normal way in, and this
// catches the work no link reached.
func (s *Server) handleInbox(w http.ResponseWriter, r *http.Request) {
	if !mayRead(r) {
		s.loginRedirect(w, r)
		return
	}
	open, err := s.reader.Inbox(r.Context())
	if err != nil {
		s.fail(w, r, err)
		return
	}
	digest, err := s.reader.Digest(r.Context())
	if err != nil {
		s.fail(w, r, err)
		return
	}

	vd := s.chrome(r)
	vd.Title = "Review queue"
	vd.Data = inboxData{
		Open:   proposalRows(open),
		Digest: proposalRows(digest),
	}
	s.render(w, http.StatusOK, "inbox", vd)
}

// proposalRows turns service proposals into listing rows, building each one's
// link from its space and id — the same stable /~owner/space/p/<id> shape the
// write plane hands back.
func proposalRows(ps []service.Proposal) []proposalRow {
	rows := make([]proposalRow, 0, len(ps))
	for _, p := range ps {
		rows = append(rows, proposalRow{
			Href:     proposalHref(p),
			ID:       p.ID,
			Space:    p.Space.String(),
			Title:    p.Title,
			Agent:    p.Agent,
			Approval: string(p.Approval),
		})
	}
	return rows
}

// proposalHref is the review-page link for a proposal: the same path the
// proposal URL uses, minus the origin, so it works as a relative link in the UI.
func proposalHref(p service.Proposal) string {
	return "/" + p.Space.String() + "/p/" + strconv.Itoa(p.ID)
}

A web/inbox_test.go => web/inbox_test.go +68 -0
@@ 0,0 1,68 @@
package web

import (
	"net/http"
	"strings"
	"testing"

	"sourcecraft.dev/bigbes/sr-ht-spec/core"
	"sourcecraft.dev/bigbes/sr-ht-spec/service"
)

// TestInboxListsOpenAndDigest proves the review queue lists open proposals under
// "waiting on you" and policy-merged ones under the digest, each linking to its
// review page.
func TestInboxListsOpenAndDigest(t *testing.T) {
	r := newFakeReader()
	seedProposal(r, service.Proposal{
		ID: 3, Space: demoSpace, Title: "Open one", State: core.StateOpen,
		Agent: "claude-code/a",
	}, nil)
	seedProposal(r, service.Proposal{
		ID: 4, Space: demoSpace, Title: "Auto-merged one", State: core.StateMerged,
		Approval: core.ApprovalPolicy, Agent: "claude-code/b",
	}, nil)
	// A human-merged proposal is neither waiting nor part of the digest.
	seedProposal(r, service.Proposal{
		ID: 5, Space: demoSpace, Title: "Human-merged", State: core.StateMerged,
		Approval: core.ApprovalHuman,
	}, nil)
	h, _, _ := testServerWith(t, r)

	rec := get(t, h, "/inbox", "bigbes")
	if rec.Code != http.StatusOK {
		t.Fatalf("status = %d, want 200; body:\n%s", rec.Code, rec.Body)
	}
	body := rec.Body.String()
	if !strings.Contains(body, "/~bigbes/rfcs/p/3") || !strings.Contains(body, "Open one") {
		t.Errorf("open proposal missing from the queue; body:\n%s", body)
	}
	if !strings.Contains(body, "/~bigbes/rfcs/p/4") || !strings.Contains(body, "Auto-merged one") {
		t.Errorf("policy-merged proposal missing from the digest; body:\n%s", body)
	}
	if strings.Contains(body, "Human-merged") {
		t.Errorf("a human-merged proposal leaked into the review queue")
	}
}

// TestInboxEmpty proves the page renders with no proposals rather than erroring.
func TestInboxEmpty(t *testing.T) {
	h, _, _ := testServerWith(t, newFakeReader())
	rec := get(t, h, "/inbox", "bigbes")
	if rec.Code != http.StatusOK {
		t.Fatalf("status = %d, want 200", rec.Code)
	}
	if !strings.Contains(rec.Body.String(), "queue is clear") {
		t.Errorf("empty inbox does not say the queue is clear")
	}
}

// TestInboxAnonymousRedirected proves a viewer with no read authority is sent to
// login rather than shown the queue.
func TestInboxAnonymousRedirected(t *testing.T) {
	h, _, _ := testServerWith(t, newFakeReader())
	rec := get(t, h, "/inbox", "")
	if rec.Code != http.StatusSeeOther && rec.Code != http.StatusFound {
		t.Fatalf("status = %d, want a login redirect", rec.Code)
	}
}

M web/reader.go => web/reader.go +14 -0
@@ 67,6 67,12 @@ type Reader interface {
	// stands.
	Approve(ctx context.Context, ref core.SpaceRef, id int) (service.Proposal, error)
	Reject(ctx context.Context, ref core.SpaceRef, id int) (service.Proposal, error)

	// Inbox is every open proposal on the instance, newest first — the review
	// queue. Digest is the recently policy-merged proposals, the firehose a human
	// sees after the fact.
	Inbox(ctx context.Context) ([]service.Proposal, error)
	Digest(ctx context.Context) ([]service.Proposal, error)
}

// Searcher is the keyword index. *search.Index satisfies it as declared.


@@ 136,3 142,11 @@ func (r serviceReader) Approve(ctx context.Context, ref core.SpaceRef, id int) (
func (r serviceReader) Reject(ctx context.Context, ref core.SpaceRef, id int) (service.Proposal, error) {
	return r.svc.Reject(ctx, ref, id)
}

func (r serviceReader) Inbox(ctx context.Context) ([]service.Proposal, error) {
	return r.svc.InboxProposals(ctx)
}

func (r serviceReader) Digest(ctx context.Context) ([]service.Proposal, error) {
	return r.svc.DigestProposals(ctx, 0)
}

M web/router.go => web/router.go +1 -0
@@ 40,6 40,7 @@ func (s *Server) Register(r chi.Router) {
	r.Get("/healthz", s.handleHealthz)
	r.Get("/static/*", s.handleStatic)
	r.Get("/search", s.handleSearch)
	r.Get("/inbox", s.handleInbox)

	// The proposal routes are registered before the document wildcard. chi gives
	// the static "p" segment priority over the "*" catch-all regardless, but

M web/templates.go => web/templates.go +1 -1
@@ 46,7 46,7 @@ var funcMap = template.FuncMap{
}

// pageNames are the content templates; each is parsed with layout.html.
var pageNames = []string{"index", "space", "document", "search", "error", "proposal"}
var pageNames = []string{"index", "space", "document", "search", "error", "proposal", "inbox"}

// pages maps a page name to its parsed template set (layout + that page).
var pages = func() map[string]*template.Template {

A web/templates/inbox.html => web/templates/inbox.html +53 -0
@@ 0,0 1,53 @@
{{define "content"}}
<div class="row">
  <div class="col-md-12">
    <h2>Review queue</h2>

    <h3 class="h5">
      Waiting on you
      <span class="badge badge-primary">{{len .Data.Open}}</span>
    </h3>
    {{if .Data.Open}}
    <table class="table">
      <thead>
        <tr><th>Proposal</th><th>Space</th><th>Agent</th></tr>
      </thead>
      <tbody>
        {{range .Data.Open}}
        <tr>
          <td><a href="{{.Href}}">#{{.ID}} — {{.Title}}</a></td>
          <td class="text-muted"><code>{{.Space}}</code></td>
          <td class="text-muted"><code>{{.Agent}}</code></td>
        </tr>
        {{end}}
      </tbody>
    </table>
    {{else}}
    <p class="text-muted">No open proposals. Your queue is clear.</p>
    {{end}}

    <h3 class="h5">Recently auto-merged</h3>
    <p class="text-muted">
      <small>Content the space's policy merged without stopping for review.</small>
    </p>
    {{if .Data.Digest}}
    <table class="table">
      <thead>
        <tr><th>Proposal</th><th>Space</th><th>Agent</th></tr>
      </thead>
      <tbody>
        {{range .Data.Digest}}
        <tr>
          <td><a href="{{.Href}}">#{{.ID}} — {{.Title}}</a></td>
          <td class="text-muted"><code>{{.Space}}</code></td>
          <td class="text-muted"><code>{{.Agent}}</code></td>
        </tr>
        {{end}}
      </tbody>
    </table>
    {{else}}
    <p class="text-muted">Nothing has auto-merged recently.</p>
    {{end}}
  </div>
</div>
{{end}}

M web/templates/index.html => web/templates/index.html +1 -0
@@ 13,6 13,7 @@
{{if .Data.LoggedIn}}
<div class="row">
  <div class="col-md-12">
    <p><a href="/inbox">Review queue</a> &mdash; proposals waiting on you.</p>
    <form method="GET" action="/search" class="form-inline">
      <div class="form-group">
        <label class="sr-only" for="q">Query</label>

M web/web_test.go => web/web_test.go +22 -0
@@ 251,6 251,28 @@ func (f *fakeReader) Reject(_ context.Context, _ core.SpaceRef, id int) (service
	return p, nil
}

func (f *fakeReader) Inbox(_ context.Context) ([]service.Proposal, error) {
	var out []service.Proposal
	for _, p := range f.proposals {
		if p.State == core.StateOpen {
			out = append(out, p)
		}
	}
	sort.Slice(out, func(i, j int) bool { return out[i].ID > out[j].ID })
	return out, nil
}

func (f *fakeReader) Digest(_ context.Context) ([]service.Proposal, error) {
	var out []service.Proposal
	for _, p := range f.proposals {
		if p.State == core.StateMerged && p.Approval == core.ApprovalPolicy {
			out = append(out, p)
		}
	}
	sort.Slice(out, func(i, j int) bool { return out[i].ID > out[j].ID })
	return out, nil
}

// fakeSearcher returns one fixed hit whose snippet carries the <mark> tags
// bleve's highlighter emits.
type fakeSearcher struct {