M scss/main.scss => scss/main.scss +149 -0
@@ 89,3 89,152 @@
color: $gray-300;
}
}
+
+// ---- Proposal review ------------------------------------------------------
+
+.proposal-actions {
+ margin: 1rem 0;
+
+ form {
+ margin-right: 0.5rem;
+ }
+}
+
+.proposal-doc {
+ margin-top: 1.5rem;
+ padding-top: 1rem;
+ border-top: 1px solid $gray-300;
+
+ @media (prefers-color-scheme: dark) {
+ border-top-color: $gray-700;
+ }
+}
+
+// ---- Prose diff -----------------------------------------------------------
+//
+// The renderer (web/diff.go) emits this markup: a hunk per heading path, a
+// block per change, and inline <del>/<ins> spans or a two-column view depending
+// on how badly the block was rewritten. Colours follow Bootstrap's success and
+// danger so added and removed read the same here as everywhere else.
+
+.prosediff {
+ font-size: 0.95rem;
+
+ .ph-hunk {
+ margin-bottom: 1rem;
+ }
+
+ .ph-path {
+ font-family: $font-family-monospace;
+ font-size: 0.8rem;
+ color: $gray-600;
+ padding: 0.15rem 0;
+
+ @media (prefers-color-scheme: dark) {
+ color: $gray-400;
+ }
+ }
+
+ .ph-block {
+ padding: 0.35rem 0.6rem;
+ margin: 0.2rem 0;
+ border-left: 3px solid transparent;
+ border-radius: 2px;
+ }
+
+ .ph-label {
+ display: block;
+ font-size: 0.72rem;
+ text-transform: uppercase;
+ letter-spacing: 0.03em;
+ color: $gray-600;
+ margin-bottom: 0.15rem;
+
+ @media (prefers-color-scheme: dark) {
+ color: $gray-400;
+ }
+ }
+
+ .ph-insert {
+ border-left-color: $success;
+ background: rgba($success, 0.08);
+ }
+
+ .ph-delete {
+ border-left-color: $danger;
+ background: rgba($danger, 0.08);
+ }
+
+ .ph-modify {
+ border-left-color: $gray-400;
+ background: rgba($gray-500, 0.06);
+ }
+
+ .ph-move {
+ border-left-color: $info;
+ color: $gray-600;
+ font-style: italic;
+ }
+
+ // Inline marks: a deletion is struck through in danger, an insertion is
+ // underlined in success. Both keep a faint background so a one-word change is
+ // visible without reading the colour.
+ del {
+ text-decoration: line-through;
+ color: $danger;
+ background: rgba($danger, 0.12);
+ text-decoration-thickness: 1px;
+ }
+
+ ins {
+ text-decoration: none;
+ color: darken($success, 8%);
+ background: rgba($success, 0.14);
+
+ @media (prefers-color-scheme: dark) {
+ color: lighten($success, 8%);
+ }
+ }
+
+ // The two-column fallback for shredded blocks: old on the left, new on the
+ // right, stacking on a narrow screen so it never overflows the page.
+ .ph-cols {
+ display: flex;
+ flex-wrap: wrap;
+ gap: 0.75rem;
+ }
+
+ .ph-col {
+ flex: 1 1 20rem;
+ min-width: 0;
+ padding: 0.4rem 0.6rem;
+ border-radius: 2px;
+ }
+
+ .ph-old {
+ background: rgba($danger, 0.06);
+ }
+
+ .ph-new {
+ background: rgba($success, 0.06);
+ }
+
+ // Code / frontmatter line diffs keep their wrapping and are read line by line.
+ .ph-code {
+ margin: 0;
+ padding: 0.25rem 0;
+ background: transparent;
+ white-space: pre-wrap;
+ word-break: break-word;
+
+ .ph-line-del {
+ display: block;
+ background: rgba($danger, 0.1);
+ }
+
+ .ph-line-ins {
+ display: block;
+ background: rgba($success, 0.1);
+ }
+ }
+}
A service/review.go => service/review.go +103 -0
@@ 0,0 1,103 @@
+package service
+
+import (
+ "context"
+ "fmt"
+ "sort"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/core"
+)
+
+// ProposalDoc is one document a proposal touches, as the review page needs it:
+// its path, the approved content it was based on, and the proposed content on
+// the branch. It is the input to the prose diff, which is the web layer's to
+// render — this layer reads git and hands over bytes.
+type ProposalDoc struct {
+ // Path is the document's path on the proposal branch.
+ Path string
+
+ // Base is the document's content at the proposal's base — the approved text
+ // the change was made against. Nil for a document the proposal adds, which
+ // is the signal to render it as wholly new rather than as a diff.
+ Base []byte
+
+ // Proposed is the document's content on the proposal branch.
+ Proposed []byte
+
+ // New reports whether the document did not exist at the base.
+ New bool
+}
+
+// ProposalDiff returns every document a proposal changes, each with the base and
+// proposed content the review page diffs.
+//
+// It reads the base and the proposal branch through the normal pinned-revision
+// path, not the ReadDocumentAtRef bypass: the branch tip is resolved to a commit
+// sha first, and an object name is a legitimate read whatever it points at. The
+// bypass exists for reading a branch *by name*; here the review already holds
+// the proposal and can pin it.
+//
+// Only genuinely changed documents are returned — a proposal branch is cut from
+// the base, so most of its documents are byte-identical to it and are not diffs.
+// A proposal changes only documents (agents cannot rename or delete), so a
+// document present at the base is present on the branch; the reverse asymmetry,
+// a document added by the proposal, is marked New.
+func (s *Service) ProposalDiff(ctx context.Context, p Proposal) ([]ProposalDoc, error) {
+ sp, err := s.OpenSpace(ctx, p.Space)
+ if err != nil {
+ return nil, err
+ }
+
+ baseDocs, err := s.ListDocuments(ctx, sp, p.BaseRev)
+ if err != nil {
+ return nil, fmt.Errorf("service: read base %s of proposal %d: %w", short(p.BaseRev), p.ID, err)
+ }
+ base := make(map[string][]byte, len(baseDocs))
+ for _, d := range baseDocs {
+ base[d.Path] = d.Data
+ }
+
+ head, err := sp.Repo.BranchHead(ctx, p.Branch)
+ if err != nil {
+ return nil, readErr(err, "read head of %s in %s", p.Branch, p.Space)
+ }
+ branchDocs, err := s.ListDocuments(ctx, sp, head.String())
+ if err != nil {
+ return nil, fmt.Errorf("service: read proposal branch %s: %w", p.Branch, err)
+ }
+
+ var out []ProposalDoc
+ for _, d := range branchDocs {
+ prior, existed := base[d.Path]
+ switch {
+ case !existed:
+ out = append(out, ProposalDoc{Path: d.Path, Proposed: d.Data, New: true})
+ case !bytesEqual(prior, d.Data):
+ out = append(out, ProposalDoc{Path: d.Path, Base: prior, Proposed: d.Data})
+ }
+ }
+ sort.Slice(out, func(i, j int) bool { return out[i].Path < out[j].Path })
+ return out, nil
+}
+
+// bytesEqual reports byte equality. It exists so ProposalDiff does not pull in
+// bytes for a single comparison, and reads as intent at the call site.
+func bytesEqual(a, b []byte) bool {
+ if len(a) != len(b) {
+ return false
+ }
+ for i := range a {
+ if a[i] != b[i] {
+ return false
+ }
+ }
+ return true
+}
+
+// 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
+// ApprovalPolicy here would be able to launder a firehose merge as reviewed.
+func (s *Service) MergeHuman(ctx context.Context, ref core.SpaceRef, proposalID int) (Proposal, error) {
+ return s.Merge(ctx, ref, proposalID, core.ApprovalHuman)
+}
A service/review_test.go => service/review_test.go +77 -0
@@ 0,0 1,77 @@
+package service
+
+import (
+ "bytes"
+ "context"
+ "testing"
+)
+
+// 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.
+func TestProposalDiffReturnsChangedDocuments(t *testing.T) {
+ svc, _ := newTestService(t)
+ ctx := context.Background()
+ sp, err := svc.CreateSpace(ctx, fxSpace)
+ if err != nil {
+ t.Fatalf("CreateSpace: %v", err)
+ }
+ // The approved head carries two documents; the proposal edits one, adds a
+ // third, and leaves the second untouched.
+ commitFiles(t, sp, sp.ApprovedBranch(), 1, map[string][]byte{
+ "specs/a.md": mdDoc("S-1", "A", "original body"),
+ "specs/keep.md": mdDoc("S-2", "Keep", "unchanged body"),
+ })
+ base, err := sp.Repo.ApprovedHead(ctx)
+ if err != nil {
+ t.Fatalf("ApprovedHead: %v", err)
+ }
+ res, err := svc.Propose(ctx, ProposeRequest{
+ Space: fxSpace,
+ Principal: agentPrincipal(),
+ Title: "edit and add",
+ IfMatch: base.String(),
+ Message: "two changes",
+ Writes: []DocumentWrite{
+ {Path: "specs/a.md", Content: mdDoc("S-1", "A", "revised body")},
+ {Path: "specs/new.md", Content: mdDoc("S-3", "New", "brand new body")},
+ },
+ })
+ if err != nil {
+ t.Fatalf("Propose: %v", err)
+ }
+
+ docs, err := svc.ProposalDiff(ctx, res.Proposal)
+ if err != nil {
+ t.Fatalf("ProposalDiff: %v", err)
+ }
+ byPath := make(map[string]ProposalDoc, len(docs))
+ for _, d := range docs {
+ byPath[d.Path] = d
+ }
+ if _, ok := byPath["specs/keep.md"]; ok {
+ t.Errorf("ProposalDiff returned the untouched specs/keep.md")
+ }
+
+ edited, ok := byPath["specs/a.md"]
+ if !ok {
+ t.Fatalf("ProposalDiff missing the edited document")
+ }
+ if edited.New {
+ t.Errorf("specs/a.md marked new, want an edit")
+ }
+ if !bytes.Contains(edited.Base, []byte("original body")) {
+ t.Errorf("edited doc base = %q, want the approved content", edited.Base)
+ }
+ if !bytes.Contains(edited.Proposed, []byte("revised body")) {
+ t.Errorf("edited doc proposed = %q, want the proposal content", edited.Proposed)
+ }
+
+ added, ok := byPath["specs/new.md"]
+ if !ok {
+ t.Fatalf("ProposalDiff missing the added document")
+ }
+ if !added.New || added.Base != nil {
+ t.Errorf("added doc = %+v, want New with a nil base", added)
+ }
+}
A web/diff.go => web/diff.go +239 -0
@@ 0,0 1,239 @@
+package web
+
+import (
+ "fmt"
+ "html/template"
+ "strings"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/prosediff"
+)
+
+// inlineSimilarityThreshold is the Phase 0 verdict's presentation switch: a
+// modified prose block whose token similarity is at or above it renders as an
+// inline word diff, and one below it renders as a two-column old/new view.
+//
+// 13% of real prose modifications shred into interleaved fragments — those
+// paragraphs really were rewritten sentence by sentence — and every one of them
+// scores at or below 0.73. Rendering them inline makes one review in eight
+// unreadable, which is the one where the agent changed the most. prosediff
+// exports BlockChange.Similarity for exactly this decision and computes no HTML
+// itself; this is where the decision is made.
+const inlineSimilarityThreshold = 0.75
+
+// diffView is the whole rendered diff of one document, ready for the proposal
+// template. Unchanged reports the degenerate case — a proposal that touches a
+// document without changing it — so the page can say so rather than show an
+// empty diff.
+type diffView struct {
+ HTML template.HTML
+ Stats prosediff.Stats
+ Unchanged bool
+}
+
+// renderDocDiff diffs the approved (old) and proposed (new) source of one
+// document and renders it to HTML.
+//
+// It walks prosediff's block-change model rather than its text renderer: the
+// text renderer is for a terminal, and the review page needs headings grouped,
+// word edits marked with <ins>/<del>, and — the load-bearing part — the
+// two-column fallback for shredded blocks. Every piece of document content is
+// HTML-escaped before it reaches the output; the only markup this produces is
+// its own structure.
+func renderDocDiff(oldSrc, newSrc []byte) diffView {
+ d := prosediff.Compare(oldSrc, newSrc)
+ view := diffView{Stats: d.Stats, Unchanged: !d.Stats.Changed()}
+ if view.Unchanged {
+ return view
+ }
+
+ var b strings.Builder
+ b.WriteString(`<div class="prosediff">`)
+ lastPath := "\x00" // impossible path, so the first real one always prints
+ inHunk := false
+ closeHunk := func() {
+ if inHunk {
+ b.WriteString(`</div></section>`)
+ inHunk = false
+ }
+ }
+
+ for _, c := range d.Changes {
+ if c.Kind == prosediff.ChangeEqual {
+ continue // the review shows only what changed
+ }
+ blk := c.New
+ if blk == nil {
+ blk = c.Old
+ }
+ if path := strings.Join(blk.HeadingPath, " › "); path != lastPath {
+ closeHunk()
+ lastPath = path
+ shown := path
+ if shown == "" {
+ shown = "(document preamble)"
+ }
+ b.WriteString(`<section class="ph-hunk"><div class="ph-path">`)
+ b.WriteString(template.HTMLEscapeString(shown))
+ b.WriteString(`</div><div class="ph-blocks">`)
+ inHunk = true
+ }
+ writeBlock(&b, c)
+ }
+ closeHunk()
+ b.WriteString(`</div>`)
+ view.HTML = template.HTML(b.String())
+ return view
+}
+
+// writeBlock renders one changed block. Insert/delete/move show the whole
+// block; a modify chooses among a code line diff, an inline word diff and the
+// two-column view, on the rules the design pins.
+func writeBlock(b *strings.Builder, c prosediff.BlockChange) {
+ switch c.Kind {
+ case prosediff.ChangeInsert:
+ writeWholeBlock(b, "ph-insert", "added", c.New)
+ case prosediff.ChangeDelete:
+ writeWholeBlock(b, "ph-delete", "removed", c.Old)
+ case prosediff.ChangeMoveIn:
+ fmt.Fprintf(b, `<div class="ph-block ph-move"><span class="ph-label">%s moved here (was line %d)</span></div>`,
+ template.HTMLEscapeString(c.New.Label()), c.Old.StartLine)
+ case prosediff.ChangeMoveOut:
+ fmt.Fprintf(b, `<div class="ph-block ph-move"><span class="ph-label">%s moved away (now line %d)</span></div>`,
+ template.HTMLEscapeString(c.Old.Label()), c.New.StartLine)
+ case prosediff.ChangeModify:
+ writeModify(b, c)
+ }
+}
+
+// writeWholeBlock renders an inserted or deleted block: its whole text, in a
+// <pre> for a non-prose kind so code keeps its wrapping, and reflowed prose
+// otherwise.
+func writeWholeBlock(b *strings.Builder, class, verb string, blk *prosediff.Block) {
+ label := verb + " " + blk.Label()
+ fmt.Fprintf(b, `<div class="ph-block %s"><span class="ph-label">%s</span>`,
+ class, template.HTMLEscapeString(label))
+ writeBody(b, blk.Text, blk.Kind.Prose())
+ b.WriteString(`</div>`)
+}
+
+// writeModify renders a modified block. A code, frontmatter or HTML block has a
+// line-oriented edit script and renders line by line; a prose block has a word
+// edit script and renders inline when it stayed similar enough to follow, and
+// two-column when it did not.
+func writeModify(b *strings.Builder, c prosediff.BlockChange) {
+ label := "changed " + c.New.Label()
+ if c.StructureOnly {
+ label = fmt.Sprintf("changed %s → %s (structure)", c.Old.Label(), c.New.Label())
+ }
+ if c.Moved {
+ label += fmt.Sprintf(" (moved from line %d)", c.Old.StartLine)
+ }
+
+ if len(c.Lines) > 0 {
+ fmt.Fprintf(b, `<div class="ph-block ph-modify"><span class="ph-label">%s</span><pre class="ph-code">`,
+ template.HTMLEscapeString(label))
+ writeLineSpans(b, c.Lines)
+ b.WriteString(`</pre></div>`)
+ return
+ }
+
+ if c.Similarity >= inlineSimilarityThreshold {
+ fmt.Fprintf(b, `<div class="ph-block ph-modify"><span class="ph-label">%s</span><div class="ph-body ph-inline">`,
+ template.HTMLEscapeString(label))
+ writeInlineSpans(b, c.Words)
+ b.WriteString(`</div></div>`)
+ return
+ }
+
+ // The two-column fallback: the block was rewritten enough that inline marks
+ // would shred it. The old column keeps deletions, the new keeps insertions,
+ // each still marked, so a reviewer reads two coherent paragraphs side by side.
+ fmt.Fprintf(b, `<div class="ph-block ph-modify ph-columns"><span class="ph-label">%s (rewritten)</span><div class="ph-cols"><div class="ph-col ph-old">`,
+ template.HTMLEscapeString(label))
+ writeColumnSpans(b, c.Words, true)
+ b.WriteString(`</div><div class="ph-col ph-new">`)
+ writeColumnSpans(b, c.Words, false)
+ b.WriteString(`</div></div></div>`)
+}
+
+// writeBody renders whole-block text: escaped, in a <pre> when it is not prose.
+func writeBody(b *strings.Builder, text string, prose bool) {
+ if prose {
+ b.WriteString(`<div class="ph-body">`)
+ b.WriteString(template.HTMLEscapeString(text))
+ b.WriteString(`</div>`)
+ return
+ }
+ b.WriteString(`<pre class="ph-code">`)
+ b.WriteString(template.HTMLEscapeString(text))
+ b.WriteString(`</pre>`)
+}
+
+// writeInlineSpans renders a word edit script inline, marking deletions and
+// insertions where they sit. The Space flag reproduces prosediff's own spacing:
+// a span that replaces the one before it carries Space=false, so a one-word
+// substitution does not render with a gap in the middle.
+func writeInlineSpans(b *strings.Builder, spans []prosediff.Span) {
+ emitted := false
+ for _, s := range spans {
+ if s.Space && emitted {
+ b.WriteByte(' ')
+ }
+ emitted = true
+ writeSpan(b, s)
+ }
+}
+
+// writeColumnSpans renders one side of the two-column view: the old side keeps
+// equal and deleted words, the new side keeps equal and inserted ones. Both
+// still mark their changes, so each column is a readable paragraph that also
+// shows what moved.
+func writeColumnSpans(b *strings.Builder, spans []prosediff.Span, old bool) {
+ emitted := false
+ for _, s := range spans {
+ keep := s.Op == prosediff.OpEqual ||
+ (old && s.Op == prosediff.OpDelete) ||
+ (!old && s.Op == prosediff.OpInsert)
+ if !keep {
+ continue
+ }
+ if s.Space && emitted {
+ b.WriteByte(' ')
+ }
+ emitted = true
+ writeSpan(b, s)
+ }
+}
+
+// writeSpan writes one span's escaped text, wrapped in <del> or <ins> for a
+// change and bare for an equal run.
+func writeSpan(b *strings.Builder, s prosediff.Span) {
+ esc := template.HTMLEscapeString(s.Text)
+ switch s.Op {
+ case prosediff.OpDelete:
+ b.WriteString("<del>")
+ b.WriteString(esc)
+ b.WriteString("</del>")
+ case prosediff.OpInsert:
+ b.WriteString("<ins>")
+ b.WriteString(esc)
+ b.WriteString("</ins>")
+ default:
+ b.WriteString(esc)
+ }
+}
+
+// writeLineSpans renders a line-oriented edit script (a code fence, frontmatter
+// or HTML block) one line per row inside a <pre>, each line class-marked.
+func writeLineSpans(b *strings.Builder, spans []prosediff.Span) {
+ for _, s := range spans {
+ class := "ph-line-eq"
+ switch s.Op {
+ case prosediff.OpDelete:
+ class = "ph-line-del"
+ case prosediff.OpInsert:
+ class = "ph-line-ins"
+ }
+ fmt.Fprintf(b, `<span class="%s">%s</span>`+"\n", class, template.HTMLEscapeString(s.Text))
+ }
+}
A web/diff_internal_test.go => web/diff_internal_test.go +90 -0
@@ 0,0 1,90 @@
+package web
+
+import (
+ "strings"
+ "testing"
+)
+
+// TestRenderDocDiffUnchanged proves a proposal that does not change a document
+// is reported as unchanged rather than as an empty diff.
+func TestRenderDocDiffUnchanged(t *testing.T) {
+ src := []byte("# Title\n\nOne paragraph.\n")
+ v := renderDocDiff(src, src)
+ if !v.Unchanged {
+ t.Fatalf("Unchanged = false, want true for identical input")
+ }
+ if v.HTML != "" {
+ t.Errorf("HTML = %q, want empty for an unchanged document", v.HTML)
+ }
+}
+
+// TestRenderDocDiffInlineWordChange proves a small edit renders inline with
+// <del>/<ins> marks and not as two columns.
+func TestRenderDocDiffInlineWordChange(t *testing.T) {
+ old := []byte("# Title\n\nThe quick brown fox jumps over the lazy dog.\n")
+ nw := []byte("# Title\n\nThe quick red fox jumps over the lazy dog.\n")
+ v := renderDocDiff(old, nw)
+ if v.Unchanged {
+ t.Fatalf("Unchanged = true, want a change")
+ }
+ html := string(v.HTML)
+ if !strings.Contains(html, "<del>brown</del>") {
+ t.Errorf("missing inline deletion of 'brown'; got:\n%s", html)
+ }
+ if !strings.Contains(html, "<ins>red</ins>") {
+ t.Errorf("missing inline insertion of 'red'; got:\n%s", html)
+ }
+ if strings.Contains(html, "ph-columns") {
+ t.Errorf("a one-word edit rendered as two columns; got:\n%s", html)
+ }
+}
+
+// TestRenderDocDiffTwoColumnBelowThreshold proves a block rewritten enough to
+// fall below the similarity threshold renders as the two-column old/new view —
+// the Phase 0 verdict's hard requirement.
+func TestRenderDocDiffTwoColumnBelowThreshold(t *testing.T) {
+ // A block rewritten to ~0.58 similarity: paired as a modify (above the 0.40
+ // pairing floor) but shredded enough to fall below the 0.75 inline switch.
+ old := []byte("# Title\n\nThe committee approved the annual budget after a long and " +
+ "contentious debate that lasted well into the evening.\n")
+ nw := []byte("# Title\n\nThe committee rejected the annual budget after a brief and " +
+ "quiet discussion that ended early in the afternoon.\n")
+ v := renderDocDiff(old, nw)
+ html := string(v.HTML)
+ if !strings.Contains(html, "ph-columns") {
+ t.Fatalf("a wholesale rewrite did not render as two columns; got:\n%s", html)
+ }
+ if !strings.Contains(html, "ph-col ph-old") || !strings.Contains(html, "ph-col ph-new") {
+ t.Errorf("two-column view missing an old or new column; got:\n%s", html)
+ }
+}
+
+// TestRenderDocDiffEscapesContent proves document content is HTML-escaped: a
+// document that contains markup cannot inject it into the review page.
+func TestRenderDocDiffEscapesContent(t *testing.T) {
+ old := []byte("# Title\n\nplain text here.\n")
+ nw := []byte("# Title\n\nplain <script>alert(1)</script> text here.\n")
+ v := renderDocDiff(old, nw)
+ html := string(v.HTML)
+ if strings.Contains(html, "<script>") {
+ t.Fatalf("unescaped <script> reached the output; got:\n%s", html)
+ }
+ if !strings.Contains(html, "<script>") {
+ t.Errorf("expected the escaped script tag in the output; got:\n%s", html)
+ }
+}
+
+// TestRenderDocDiffInsertAndDelete proves an added and a removed block are each
+// shown whole, labelled.
+func TestRenderDocDiffInsertAndDelete(t *testing.T) {
+ old := []byte("# Title\n\nKept paragraph.\n\nDoomed paragraph.\n")
+ nw := []byte("# Title\n\nKept paragraph.\n\nBrand new paragraph.\n")
+ v := renderDocDiff(old, nw)
+ html := string(v.HTML)
+ if !strings.Contains(html, "ph-insert") {
+ t.Errorf("missing an inserted block; got:\n%s", html)
+ }
+ if !strings.Contains(html, "ph-delete") {
+ t.Errorf("missing a deleted block; got:\n%s", html)
+ }
+}
M web/handlers.go => web/handlers.go +11 -0
@@ 84,6 84,17 @@ func httpStatusFor(err error) int {
return http.StatusNotFound
case errors.Is(err, core.ErrInvalidName), errors.Is(err, core.ErrInvalidPath):
return http.StatusBadRequest
+ // The write-plane sentinels the review page's approve/reject can return. A
+ // base that moved under a proposal, a proposal already resolved, and an
+ // already-merged one are all 409; the owner-only refusal is 403.
+ case errors.Is(err, service.ErrForbidden):
+ return http.StatusForbidden
+ case errors.Is(err, service.ErrStale),
+ errors.Is(err, service.ErrAlreadyMerged),
+ errors.Is(err, service.ErrProposalNotOpen):
+ return http.StatusConflict
+ case errors.Is(err, service.ErrInvalid):
+ return http.StatusUnprocessableEntity
default:
return http.StatusInternalServerError
}
A web/proposal.go => web/proposal.go +203 -0
@@ 0,0 1,203 @@
+package web
+
+import (
+ "context"
+ "fmt"
+ "net/http"
+ "net/url"
+ "strconv"
+
+ "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"
+)
+
+// proposalData is the review page's payload: the proposal, the per-document
+// prose diffs, and whether the viewer may act on it.
+type proposalData struct {
+ Proposal service.Proposal
+ // SpaceHref links back to the space at its approved head.
+ SpaceHref string
+ // StateBadge is the Bootstrap badge class for the proposal's state, so the
+ // template does not branch on the state string.
+ StateBadge string
+ // Docs is one entry per document the proposal changes, in path order.
+ Docs []proposalDocDiff
+ // CanApprove reports whether the viewer is the owner and the proposal is
+ // still open — the only case the approve/reject controls are shown.
+ CanApprove bool
+ // Approved / Rejected phrase the outcome for a terminal proposal.
+ Merged bool
+ Rejected bool
+}
+
+// proposalDocDiff is one document's rendered diff on the review page.
+type proposalDocDiff struct {
+ Path string
+ New bool
+ Diff diffView
+}
+
+// proposalIDFrom parses the {id} path parameter. A non-numeric or non-positive
+// id is not a proposal — "/p/" is the proposal namespace, and a document that
+// happens to sit under a "p/" path is addressed through the document route, not
+// here — so it is a 404 rather than a 400.
+func proposalIDFrom(r *http.Request) (int, bool) {
+ id, err := strconv.Atoi(chi.URLParam(r, "id"))
+ if err != nil || id <= 0 {
+ return 0, false
+ }
+ return id, true
+}
+
+// handleProposal renders the review page for one proposal: its metadata, its
+// status, and a prose diff of every document it changes.
+//
+// The diff reads the proposal branch, which the normal read plane refuses — so
+// it goes through service.ProposalDiff, which resolves the branch tip to a sha
+// and reads content the review is entitled to see. A proposal whose space does
+// not match the URL is a 404: the id is global, but the link names its space,
+// and answering for the wrong space would let one space's URL surface another's
+// proposal.
+func (s *Server) handleProposal(w http.ResponseWriter, r *http.Request) {
+ if !mayRead(r) {
+ s.loginRedirect(w, r)
+ return
+ }
+ ref, err := spaceRefFrom(r)
+ if err != nil {
+ s.fail(w, r, err)
+ return
+ }
+ id, ok := proposalIDFrom(r)
+ if !ok {
+ s.renderError(w, r, http.StatusNotFound, "no such proposal")
+ return
+ }
+
+ p, err := s.reader.GetProposal(r.Context(), id)
+ if err != nil {
+ s.fail(w, r, err)
+ return
+ }
+ if p.Space != ref {
+ s.renderError(w, r, http.StatusNotFound, "no such proposal in this space")
+ return
+ }
+
+ docs, err := s.reader.ProposalDiff(r.Context(), p)
+ if err != nil {
+ s.fail(w, r, err)
+ return
+ }
+ views := make([]proposalDocDiff, 0, len(docs))
+ for _, d := range docs {
+ // A new document diffs against nothing, which renders as an all-inserted
+ // block set — the same renderer, so the page has one code path.
+ views = append(views, proposalDocDiff{
+ Path: d.Path,
+ New: d.New,
+ Diff: renderDocDiff(d.Base, d.Proposed),
+ })
+ }
+
+ owner := authn.PrincipalFromContext(r.Context()).IsOwner()
+ vd := s.chrome(r)
+ vd.Title = fmt.Sprintf("Proposal #%d — %s", p.ID, p.Title)
+ vd.Data = proposalData{
+ Proposal: p,
+ SpaceHref: "/" + ref.String(),
+ StateBadge: stateBadge(p.State),
+ Docs: views,
+ CanApprove: owner && p.State == core.StateOpen,
+ Merged: p.State == core.StateMerged,
+ Rejected: p.State == core.StateRejected,
+ }
+ s.render(w, http.StatusOK, "proposal", vd)
+}
+
+// handleProposalApprove merges a proposal on the owner's approval, then redirects
+// back to the proposal page so a reload does not re-submit.
+func (s *Server) handleProposalApprove(w http.ResponseWriter, r *http.Request) {
+ s.actOnProposal(w, r, s.reader.Approve)
+}
+
+// handleProposalReject resolves a proposal to rejected, then redirects back.
+func (s *Server) handleProposalReject(w http.ResponseWriter, r *http.Request) {
+ s.actOnProposal(w, r, s.reader.Reject)
+}
+
+// actOnProposal is the shared approve/reject path: the owner-only gate, the
+// cross-site guard, the action, and the post-redirect-get back to the page.
+//
+// Only the owner may approve or reject — that is the one authority the whole
+// authorization model turns on, and an agent, though authenticated, has it no
+// more than an anonymous viewer. The cross-site guard refuses a state change
+// whose Origin is not this instance, which is the CSRF defense a form post needs
+// when the session cookie is meta's and this service cannot set its SameSite.
+func (s *Server) actOnProposal(w http.ResponseWriter, r *http.Request,
+ act func(context.Context, core.SpaceRef, int) (service.Proposal, error)) {
+
+ if !authn.PrincipalFromContext(r.Context()).IsOwner() {
+ s.renderError(w, r, http.StatusForbidden, "only the instance owner may approve or reject a proposal")
+ return
+ }
+ if !s.sameOrigin(r) {
+ s.renderError(w, r, http.StatusForbidden, "this request did not originate from this site")
+ return
+ }
+ ref, err := spaceRefFrom(r)
+ if err != nil {
+ s.fail(w, r, err)
+ return
+ }
+ id, ok := proposalIDFrom(r)
+ if !ok {
+ s.renderError(w, r, http.StatusNotFound, "no such proposal")
+ return
+ }
+ if _, err := act(r.Context(), ref, id); err != nil {
+ s.fail(w, r, err)
+ return
+ }
+ http.Redirect(w, r, fmt.Sprintf("/%s/p/%d", ref, id), http.StatusSeeOther)
+}
+
+// sameOrigin reports whether a state-changing request came from this site. It
+// checks the Origin header — which browsers send on every form POST — and falls
+// back to Referer, refusing a request that carries neither. A cross-site forgery
+// carries the attacker's origin and fails; this instance's own form carries its
+// own and passes.
+func (s *Server) sameOrigin(r *http.Request) bool {
+ claimed := r.Header.Get("Origin")
+ if claimed == "" {
+ claimed = r.Header.Get("Referer")
+ }
+ if claimed == "" {
+ return false
+ }
+ got, err := url.Parse(claimed)
+ if err != nil || got.Host == "" {
+ return false
+ }
+ want, err := url.Parse(s.origin)
+ if err != nil {
+ return false
+ }
+ return got.Scheme == want.Scheme && got.Host == want.Host
+}
+
+// stateBadge maps a proposal state onto the Bootstrap badge class the template
+// tags it with, so the presentation choice lives in one place.
+func stateBadge(state core.ProposalState) string {
+ switch state {
+ case core.StateMerged:
+ return "badge-success"
+ case core.StateRejected:
+ return "badge-danger"
+ default:
+ return "badge-primary"
+ }
+}
A web/proposal_test.go => web/proposal_test.go +221 -0
@@ 0,0 1,221 @@
+package web
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "strings"
+ "testing"
+
+ "sourcecraft.dev/bigbes/sr-ht-spec/core"
+ "sourcecraft.dev/bigbes/sr-ht-spec/service"
+)
+
+// seedProposal registers a proposal and its diff on the fake reader.
+func seedProposal(r *fakeReader, p service.Proposal, docs []service.ProposalDoc) {
+ r.proposals[p.ID] = p
+ r.diffs[p.ID] = docs
+}
+
+func openProposal() service.Proposal {
+ return service.Proposal{
+ ID: 7, Space: demoSpace, Title: "Revise storage model",
+ Rationale: "clearer wording", BaseRev: headRev, Branch: "proposals/7",
+ State: core.StateOpen, Agent: "claude-code/spec-writer", AgentSession: "sess-1",
+ }
+}
+
+// post issues a form POST as a user, with the Origin header set unless overridden.
+func post(t *testing.T, h http.Handler, target, user, origin string) *httptest.ResponseRecorder {
+ t.Helper()
+ req := httptest.NewRequest(http.MethodPost, target, nil)
+ if user != "" {
+ login(req, user)
+ }
+ if origin != "" {
+ req.Header.Set("Origin", origin)
+ }
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ return rec
+}
+
+// TestProposalPageRendersDiffAndControls proves the owner sees the diff and the
+// approve/reject controls on an open proposal.
+func TestProposalPageRendersDiffAndControls(t *testing.T) {
+ r := newFakeReader()
+ seedProposal(r, openProposal(), []service.ProposalDoc{{
+ Path: "specs/0007-storage.md",
+ Base: []byte("# Storage\n\nGit is authoritative here.\n"),
+ Proposed: []byte("# Storage\n\nGit is the authoritative source here.\n"),
+ }})
+ h, _, _ := testServerWith(t, r)
+
+ rec := get(t, h, "/~bigbes/rfcs/p/7", "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, "prosediff") {
+ t.Errorf("page has no rendered diff; body:\n%s", body)
+ }
+ if !strings.Contains(body, "/p/7/approve") || !strings.Contains(body, "/p/7/reject") {
+ t.Errorf("owner viewing an open proposal has no approve/reject controls")
+ }
+ if !strings.Contains(body, "specs/0007-storage.md") {
+ t.Errorf("page does not name the changed document")
+ }
+}
+
+// TestProposalPageHidesControlsWhenMerged proves a terminal proposal shows no
+// controls and states its outcome.
+func TestProposalPageHidesControlsWhenMerged(t *testing.T) {
+ r := newFakeReader()
+ p := openProposal()
+ p.State = core.StateMerged
+ p.Approval = core.ApprovalHuman
+ p.MergedRev = oldRev
+ seedProposal(r, p, nil)
+ h, _, _ := testServerWith(t, r)
+
+ rec := get(t, h, "/~bigbes/rfcs/p/7", "bigbes")
+ if rec.Code != http.StatusOK {
+ t.Fatalf("status = %d, want 200", rec.Code)
+ }
+ if strings.Contains(rec.Body.String(), "/p/7/approve") {
+ t.Errorf("a merged proposal still shows the approve control")
+ }
+}
+
+// TestProposalPageWrongSpaceIs404 proves a proposal id addressed through the
+// wrong space's URL is not found.
+func TestProposalPageWrongSpaceIs404(t *testing.T) {
+ r := newFakeReader()
+ p := openProposal()
+ p.Space = core.SpaceRef{Owner: "bigbes", Name: "other"}
+ seedProposal(r, p, nil)
+ h, _, _ := testServerWith(t, r)
+
+ rec := get(t, h, "/~bigbes/rfcs/p/7", "bigbes")
+ if rec.Code != http.StatusNotFound {
+ t.Fatalf("status = %d, want 404 for a proposal in another space", rec.Code)
+ }
+}
+
+// TestProposalPageNonNumericIs404 proves the "/p/" namespace refuses a
+// non-numeric id rather than treating it as a document.
+func TestProposalPageNonNumericIs404(t *testing.T) {
+ h, _, _ := testServerWith(t, newFakeReader())
+ rec := get(t, h, "/~bigbes/rfcs/p/not-a-number", "bigbes")
+ if rec.Code != http.StatusNotFound {
+ t.Fatalf("status = %d, want 404", rec.Code)
+ }
+}
+
+// TestProposalPageAnonymousRedirected proves a viewer with no read authority is
+// sent to login, not shown the proposal.
+func TestProposalPageAnonymousRedirected(t *testing.T) {
+ r := newFakeReader()
+ seedProposal(r, openProposal(), nil)
+ h, _, _ := testServerWith(t, r)
+ rec := get(t, h, "/~bigbes/rfcs/p/7", "")
+ if rec.Code != http.StatusSeeOther && rec.Code != http.StatusFound {
+ t.Fatalf("status = %d, want a login redirect", rec.Code)
+ }
+}
+
+// TestApproveMergesAsOwner proves the owner's approve POST merges the proposal
+// and redirects back.
+func TestApproveMergesAsOwner(t *testing.T) {
+ r := newFakeReader()
+ seedProposal(r, openProposal(), nil)
+ h, reader, _ := testServerWith(t, r)
+
+ rec := post(t, h, "/~bigbes/rfcs/p/7/approve", "bigbes", "https://spec.example")
+ if rec.Code != http.StatusSeeOther {
+ t.Fatalf("status = %d, want 303; body:\n%s", rec.Code, rec.Body)
+ }
+ if got := reader.proposals[7].State; got != core.StateMerged {
+ t.Errorf("proposal state = %s, want merged", got)
+ }
+ if loc := rec.Header().Get("Location"); loc != "/~bigbes/rfcs/p/7" {
+ t.Errorf("redirect = %q, want the proposal page", loc)
+ }
+}
+
+// TestRejectResolvesAsOwner proves the owner's reject POST rejects the proposal.
+func TestRejectResolvesAsOwner(t *testing.T) {
+ r := newFakeReader()
+ seedProposal(r, openProposal(), nil)
+ h, reader, _ := testServerWith(t, r)
+
+ rec := post(t, h, "/~bigbes/rfcs/p/7/reject", "bigbes", "https://spec.example")
+ if rec.Code != http.StatusSeeOther {
+ t.Fatalf("status = %d, want 303", rec.Code)
+ }
+ if got := reader.proposals[7].State; got != core.StateRejected {
+ t.Errorf("proposal state = %s, want rejected", got)
+ }
+}
+
+// TestApproveForbiddenForAgent proves an agent — authenticated but not the owner
+// — may not approve.
+func TestApproveForbiddenForAgent(t *testing.T) {
+ r := newFakeReader()
+ seedProposal(r, openProposal(), nil)
+ h, reader, _ := testServerWith(t, r)
+
+ req := httptest.NewRequest(http.MethodPost, "/~bigbes/rfcs/p/7/approve", nil)
+ req.Header.Set("Authorization", "Bearer "+agentTk)
+ req.Header.Set("Origin", "https://spec.example")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+
+ if rec.Code != http.StatusForbidden {
+ t.Fatalf("status = %d, want 403 for an agent approving", rec.Code)
+ }
+ if reader.proposals[7].State != core.StateOpen {
+ t.Errorf("the proposal was resolved despite the agent being refused")
+ }
+}
+
+// TestApproveRefusedCrossOrigin proves a POST whose Origin is not this site is
+// refused — the CSRF defense.
+func TestApproveRefusedCrossOrigin(t *testing.T) {
+ r := newFakeReader()
+ seedProposal(r, openProposal(), nil)
+ h, reader, _ := testServerWith(t, r)
+
+ rec := post(t, h, "/~bigbes/rfcs/p/7/approve", "bigbes", "https://evil.example")
+ if rec.Code != http.StatusForbidden {
+ t.Fatalf("status = %d, want 403 for a cross-origin POST", rec.Code)
+ }
+ if reader.proposals[7].State != core.StateOpen {
+ t.Errorf("the proposal was resolved despite the cross-origin refusal")
+ }
+}
+
+// TestApproveMissingOriginRefused proves a POST with no Origin or Referer is
+// refused rather than trusted.
+func TestApproveMissingOriginRefused(t *testing.T) {
+ r := newFakeReader()
+ seedProposal(r, openProposal(), nil)
+ h, _, _ := testServerWith(t, r)
+ rec := post(t, h, "/~bigbes/rfcs/p/7/approve", "bigbes", "")
+ if rec.Code != http.StatusForbidden {
+ t.Fatalf("status = %d, want 403 when no Origin is presented", rec.Code)
+ }
+}
+
+// TestApproveStaleIs409 proves a merge that the service reports stale surfaces as
+// a 409, not a 500.
+func TestApproveStaleIs409(t *testing.T) {
+ r := newFakeReader()
+ seedProposal(r, openProposal(), nil)
+ r.actErr = service.ErrStale
+ h, _, _ := testServerWith(t, r)
+
+ rec := post(t, h, "/~bigbes/rfcs/p/7/approve", "bigbes", "https://spec.example")
+ if rec.Code != http.StatusConflict {
+ t.Fatalf("status = %d, want 409 for a stale approve", rec.Code)
+ }
+}
M web/reader.go => web/reader.go +45 -3
@@ 27,12 27,15 @@ type Snapshot struct {
Bodies map[string][]byte
}
-// Reader is the read surface this package needs. *service.Service provides it
-// through NewReader.
+// Reader is the service surface this package needs. *service.Service provides
+// it through NewReader.
//
// It is an interface for the same reason compare.sr.ht's authz.Authorizer is:
// so the handlers can be tested against a document set rather than against a
-// Postgres instance and a tree of bare repositories.
+// Postgres instance and a tree of bare repositories. It is named Reader for the
+// read plane it began as; Phase 4's review page adds the proposal reads and the
+// two approve/reject writes, because the review page is where this package first
+// mutates anything and one seam is simpler than two.
type Reader interface {
// ListSpaces returns every space on the instance. Single-user with no
// visibility levels means there is nothing to filter — the list is the
@@ 45,6 48,25 @@ type Reader interface {
// ReadDocument reads one document by tree path at a revision.
ReadDocument(ctx context.Context, ref core.SpaceRef, rev, path string) (service.Document, error)
+
+ // GetProposal resolves one proposal by id, in any state — the stable
+ // proposal URL still resolves after merge or rejection.
+ GetProposal(ctx context.Context, id int) (service.Proposal, error)
+
+ // ListProposals returns a space's proposals in one state, newest first: the
+ // inbox (open) and the digest (merged) are the same call with a different
+ // state.
+ ListProposals(ctx context.Context, ref core.SpaceRef, state core.ProposalState) ([]service.Proposal, error)
+
+ // ProposalDiff returns each document a proposal changes, with the base and
+ // proposed content the page diffs.
+ ProposalDiff(ctx context.Context, p service.Proposal) ([]service.ProposalDoc, error)
+
+ // Approve merges a proposal on the owner's approval — always human, never
+ // policy. Reject resolves it to rejected. Both return the proposal as it now
+ // 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)
}
// Searcher is the keyword index. *search.Index satisfies it as declared.
@@ 94,3 116,23 @@ func (r serviceReader) ReadDocument(ctx context.Context, ref core.SpaceRef, rev,
}
return r.svc.ReadDocument(ctx, sp, rev, p)
}
+
+func (r serviceReader) GetProposal(ctx context.Context, id int) (service.Proposal, error) {
+ return r.svc.GetProposal(ctx, id)
+}
+
+func (r serviceReader) ListProposals(ctx context.Context, ref core.SpaceRef, state core.ProposalState) ([]service.Proposal, error) {
+ return r.svc.ListProposals(ctx, ref, state)
+}
+
+func (r serviceReader) ProposalDiff(ctx context.Context, p service.Proposal) ([]service.ProposalDoc, error) {
+ return r.svc.ProposalDiff(ctx, p)
+}
+
+func (r serviceReader) Approve(ctx context.Context, ref core.SpaceRef, id int) (service.Proposal, error) {
+ return r.svc.MergeHuman(ctx, ref, id)
+}
+
+func (r serviceReader) Reject(ctx context.Context, ref core.SpaceRef, id int) (service.Proposal, error) {
+ return r.svc.Reject(ctx, ref, id)
+}
M web/router.go => web/router.go +8 -0
@@ 41,6 41,14 @@ func (s *Server) Register(r chi.Router) {
r.Get("/static/*", s.handleStatic)
r.Get("/search", s.handleSearch)
+ // The proposal routes are registered before the document wildcard. chi gives
+ // the static "p" segment priority over the "*" catch-all regardless, but
+ // keeping them adjacent makes the "/p/ is the proposal namespace" decision
+ // visible in one place.
+ r.Get("/~{owner}/{space}/p/{id}", s.handleProposal)
+ r.Post("/~{owner}/{space}/p/{id}/approve", s.handleProposalApprove)
+ r.Post("/~{owner}/{space}/p/{id}/reject", s.handleProposalReject)
+
r.Get("/~{owner}/{space}", s.handleSpace)
r.Get("/~{owner}/{space}/*", s.handleDocument)
}
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"}
+var pageNames = []string{"index", "space", "document", "search", "error", "proposal"}
// pages maps a page name to its parsed template set (layout + that page).
var pages = func() map[string]*template.Template {
A web/templates/proposal.html => web/templates/proposal.html +64 -0
@@ 0,0 1,64 @@
+{{define "content"}}
+{{$p := .Data.Proposal}}
+<div class="row">
+ <div class="col-md-12">
+ <p class="text-muted">
+ <a href="{{.Data.SpaceHref}}">{{$p.Space}}</a>
+ </p>
+ <h2>
+ Proposal #{{$p.ID}}
+ <span class="badge {{.Data.StateBadge}}">{{$p.State}}</span>
+ </h2>
+ <h3 class="h5">{{$p.Title}}</h3>
+ {{if $p.Rationale}}<p>{{$p.Rationale}}</p>{{end}}
+ <p class="text-muted">
+ <small>
+ proposed by <code>{{$p.Agent}}</code>
+ (session <code>{{$p.AgentSession}}</code>)
+ against <code title="{{$p.BaseRev}}">{{shortsha $p.BaseRev}}</code>
+ {{if .Data.Merged}}
+ — merged as <span class="badge badge-secondary">{{$p.Approval}}</span>
+ at <code title="{{$p.MergedRev}}">{{shortsha $p.MergedRev}}</code>
+ {{end}}
+ </small>
+ </p>
+
+ {{if .Data.CanApprove}}
+ <div class="proposal-actions">
+ <form method="POST" action="/{{$p.Space}}/p/{{$p.ID}}/approve" class="d-inline">
+ <button type="submit" class="btn btn-success">Approve & merge</button>
+ </form>
+ <form method="POST" action="/{{$p.Space}}/p/{{$p.ID}}/reject" class="d-inline">
+ <button type="submit" class="btn btn-outline-danger">Reject</button>
+ </form>
+ </div>
+ {{else if .Data.Merged}}
+ <p class="text-success">This proposal has been merged.</p>
+ {{else if .Data.Rejected}}
+ <p class="text-danger">This proposal was rejected.</p>
+ {{end}}
+ </div>
+</div>
+
+<div class="row">
+ <div class="col-md-12">
+ {{if .Data.Docs}}
+ {{range .Data.Docs}}
+ <div class="proposal-doc">
+ <h4 class="h6">
+ <code>{{.Path}}</code>
+ {{if .New}}<span class="badge badge-success">new</span>{{end}}
+ </h4>
+ {{if .Diff.Unchanged}}
+ <p class="text-muted"><small>No textual change.</small></p>
+ {{else}}
+ {{.Diff.HTML}}
+ {{end}}
+ </div>
+ {{end}}
+ {{else}}
+ <p class="text-muted">This proposal changes no documents.</p>
+ {{end}}
+ </div>
+</div>
+{{end}}
M web/web_test.go => web/web_test.go +66 -5
@@ 109,12 109,21 @@ An earlier sketch.
type fakeReader struct {
revs map[string]map[string]string // rev -> path -> content
head string
+
+ // proposals, diffs and actErr back the Phase 4 review-page tests. proposals
+ // is keyed by id; diffs by proposal id; actErr, when set, is what Approve
+ // and Reject return instead of acting.
+ proposals map[int]service.Proposal
+ diffs map[int][]service.ProposalDoc
+ actErr error
}
func newFakeReader() *fakeReader {
return &fakeReader{
- revs: map[string]map[string]string{headRev: headDocs, oldRev: oldDocs},
- head: headRev,
+ revs: map[string]map[string]string{headRev: headDocs, oldRev: oldDocs},
+ head: headRev,
+ proposals: map[int]service.Proposal{},
+ diffs: map[int][]service.ProposalDoc{},
}
}
@@ 198,6 207,50 @@ func (f *fakeReader) ReadDocument(_ context.Context, ref core.SpaceRef, rev, p s
}, nil
}
+func (f *fakeReader) GetProposal(_ context.Context, id int) (service.Proposal, error) {
+ p, ok := f.proposals[id]
+ if !ok {
+ return service.Proposal{}, fmt.Errorf("%w: proposal %d", service.ErrNotFound, id)
+ }
+ return p, nil
+}
+
+func (f *fakeReader) ListProposals(_ context.Context, ref core.SpaceRef, state core.ProposalState) ([]service.Proposal, error) {
+ var out []service.Proposal
+ for _, p := range f.proposals {
+ if p.Space == ref && p.State == state {
+ 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) ProposalDiff(_ context.Context, p service.Proposal) ([]service.ProposalDoc, error) {
+ return f.diffs[p.ID], nil
+}
+
+func (f *fakeReader) Approve(_ context.Context, _ core.SpaceRef, id int) (service.Proposal, error) {
+ if f.actErr != nil {
+ return service.Proposal{}, f.actErr
+ }
+ p := f.proposals[id]
+ p.State = core.StateMerged
+ p.Approval = core.ApprovalHuman
+ f.proposals[id] = p
+ return p, nil
+}
+
+func (f *fakeReader) Reject(_ context.Context, _ core.SpaceRef, id int) (service.Proposal, error) {
+ if f.actErr != nil {
+ return service.Proposal{}, f.actErr
+ }
+ p := f.proposals[id]
+ p.State = core.StateRejected
+ f.proposals[id] = p
+ return p, nil
+}
+
// fakeSearcher returns one fixed hit whose snippet carries the <mark> tags
// bleve's highlighter emits.
type fakeSearcher struct {
@@ 236,10 289,18 @@ func (stubTokenStore) LookupAgentToken(_ context.Context, hash []byte) (authn.Ag
return authn.AgentToken{ID: 1, Name: "test", Hash: want}, nil
}
-// testServer wires a Server with the fake reader/searcher behind the same
+// testServer wires a Server with a fresh fake reader/searcher behind the same
// middleware the daemon installs, and returns the handler.
func testServer(t *testing.T) (http.Handler, *fakeSearcher) {
t.Helper()
+ h, _, sr := testServerWith(t, newFakeReader())
+ return h, sr
+}
+
+// testServerWith is testServer with a caller-supplied reader, so the review-page
+// tests can seed proposals into it and still get the same middleware stack.
+func testServerWith(t *testing.T, reader *fakeReader) (http.Handler, *fakeReader, *fakeSearcher) {
+ t.Helper()
conf := ini.File{
"sr.ht": ini.Section{
"network-key": testConf.Section("sr.ht")["network-key"],
@@ 264,14 325,14 @@ func testServer(t *testing.T) (http.Handler, *fakeSearcher) {
searcher := &fakeSearcher{}
srv, err := New(Options{
Conf: conf,
- Reader: newFakeReader(),
+ Reader: reader,
Searcher: searcher,
Resolver: resolver,
})
if err != nil {
t.Fatalf("New: %v", err)
}
- return srv.Handler(), searcher
+ return srv.Handler(), reader, searcher
}
// login seals a unified-login cookie for the given user onto a request — the