From 865a21fa60c6230f79a17ef880767c87729c0435 Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Fri, 24 Jul 2026 19:53:58 +0300 Subject: [PATCH] feat(web): digest tracks 'since you last looked' via digest_mark (spec-mfm) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The policy-merged digest showed the last N auto-merges by count; the design intends "what auto-merged since you last saw it", backed by the digest_mark table that existed but nothing read. The inbox GET now reads the mark to flag each digest row that merged after it as new, count them, and draw a divider before the already-seen rows — staying a pure read. Advancing the mark is a write, so it is an explicit POST /inbox/seen behind the owner-only + same-origin guard approve/reject already use, not a side-effecting GET. service.DigestMark/MarkDigestSeen wrap the store, mapping "no mark yet" to (zero, false) so the first-ever view reads the whole digest as new. Closes spec-mfm --- service/review.go | 33 ++++++++++++ service/review_test.go | 26 ++++++++++ web/inbox.go | 100 ++++++++++++++++++++++++++++++------ web/inbox_test.go | 107 +++++++++++++++++++++++++++++++++++++++ web/reader.go | 16 ++++++ web/router.go | 1 + web/templates/inbox.html | 28 ++++++++-- web/web_test.go | 15 ++++++ 8 files changed, 305 insertions(+), 21 deletions(-) diff --git a/service/review.go b/service/review.go index 617ad463d79c19d59adcb867fca2630b2fe22700..458ce160b33083fd4c8444800629bcfc156de0f2 100644 --- a/service/review.go +++ b/service/review.go @@ -2,10 +2,13 @@ package service import ( "context" + "errors" "fmt" "sort" + "time" "sourcecraft.dev/bigbes/sr-ht-spec/core" + "sourcecraft.dev/bigbes/sr-ht-spec/db" ) // ProposalDoc is one document a proposal touches, as the review page needs it: @@ -117,6 +120,36 @@ func (s *Service) DigestProposals(ctx context.Context, limit int) ([]Proposal, e return s.proposalsInState(ctx, core.StateMerged, true, limit) } +// DigestMark reports when the owner last marked the digest seen, and whether a +// mark exists yet. No mark — the owner has never cleared the digest — is +// (zero, false, nil), and on that first view every auto-merge counts as new. +// +// The digest itself (DigestProposals) stays a pure read; this is the timestamp +// the inbox compares each row's merge time against to draw the "new since you +// last looked" line, without the render advancing anything. +func (s *Service) DigestMark(ctx context.Context) (time.Time, bool, error) { + t, err := s.store.GetDigestMark(ctx, s.cfg.Instance.OwnerName) + switch { + case errors.Is(err, db.ErrNotFound): + return time.Time{}, false, nil + case err != nil: + return time.Time{}, false, fmt.Errorf("service: read digest mark: %w", err) + } + return t, true, nil +} + +// MarkDigestSeen advances the owner's digest mark to seenAt, so the next inbox +// render counts only what auto-merged after this moment as new. Advancing the +// mark is the one write the review queue makes, kept behind an explicit action +// so the inbox GET stays pure; the caller passes the timestamp so the clock +// lives at the edge and the write stays testable. +func (s *Service) MarkDigestSeen(ctx context.Context, seenAt time.Time) error { + if err := s.store.SetDigestMark(ctx, s.cfg.Instance.OwnerName, seenAt); err != nil { + return fmt.Errorf("service: advance digest mark: %w", err) + } + return nil +} + // 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 diff --git a/service/review_test.go b/service/review_test.go index 84cad3915b86591d95d65af0aad56885a5492075..772d47c58caee624e2ff6649a6162b5e25ac6928 100644 --- a/service/review_test.go +++ b/service/review_test.go @@ -4,10 +4,36 @@ import ( "bytes" "context" "testing" + "time" "sourcecraft.dev/bigbes/sr-ht-spec/core" ) +// TestDigestMark proves the digest mark round-trips through the owner-keyed +// store and that its absence is reported as "no mark yet" rather than an error — +// the distinction the inbox turns into "every auto-merge is new on first view". +func TestDigestMark(t *testing.T) { + svc, _ := newTestService(t) + ctx := context.Background() + + if _, marked, err := svc.DigestMark(ctx); err != nil || marked { + t.Fatalf("DigestMark before any write: marked=%v err=%v, want (false, nil)", marked, err) + } + + seen := time.Date(2026, 7, 20, 15, 0, 0, 0, time.UTC) + if err := svc.MarkDigestSeen(ctx, seen); err != nil { + t.Fatalf("MarkDigestSeen: %v", err) + } + + got, marked, err := svc.DigestMark(ctx) + if err != nil || !marked { + t.Fatalf("DigestMark after write: marked=%v err=%v, want (true, nil)", marked, err) + } + if !got.Equal(seen) { + t.Errorf("mark = %s, want %s", got, seen) + } +} + // 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. diff --git a/web/inbox.go b/web/inbox.go index 0817f190d3be0a232fc57f6ce88e0de7153a8784..72519d237b91047796f46d26faf5e5439137120b 100644 --- a/web/inbox.go +++ b/web/inbox.go @@ -3,19 +3,29 @@ package web import ( "net/http" "strconv" + "time" + "sourcecraft.dev/bigbes/sr-ht-spec/authn" "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. +// +// NewCount is how many leading digest rows auto-merged since the owner last +// marked it seen. The digest is newest-first and "new" means merged after the +// mark, so the new rows are exactly the first NewCount — the template draws the +// "since you last looked" divider after them and shows the mark-as-seen action +// only when there is something new to clear. type inboxData struct { - Open []proposalRow - Digest []proposalRow + Open []proposalRow + Digest []proposalRow + NewCount int } // proposalRow is one proposal as a listing line: enough to decide whether to -// open it, and the link that does. +// open it, and the link that does. New marks a digest row that auto-merged +// since the owner last looked; it is always false for the open queue. type proposalRow struct { Href string ID int @@ -23,6 +33,7 @@ type proposalRow struct { Title string Agent string Approval string + New bool } // handleInbox renders the review queue: every open proposal on the instance, @@ -44,34 +55,91 @@ func (s *Server) handleInbox(w http.ResponseWriter, r *http.Request) { s.fail(w, r, err) return } + // The GET stays pure: it reads the mark to draw the divider but never moves + // it. Advancing is handleInboxSeen's job, behind a POST. + mark, marked, err := s.reader.DigestMark(r.Context()) + if err != nil { + s.fail(w, r, err) + return + } + + digestRows, newCount := digestRows(digest, mark, marked) vd := s.chrome(r) vd.Title = "Review queue" vd.Data = inboxData{ - Open: proposalRows(open), - Digest: proposalRows(digest), + Open: proposalRows(open), + Digest: digestRows, + NewCount: newCount, } 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/ shape the -// write plane hands back. +// handleInboxSeen advances the owner's digest mark to now, then redirects back +// to the queue so a reload does not re-submit. It is the one write the review +// queue makes; keeping it a POST is what lets handleInbox stay a pure read. +// +// Only the owner may move their own mark, and the cross-site guard is the same +// one approve/reject use — the CSRF defense a form post needs when the session +// cookie is meta's and this service cannot set its SameSite. +func (s *Server) handleInboxSeen(w http.ResponseWriter, r *http.Request) { + if !authn.PrincipalFromContext(r.Context()).IsOwner() { + s.renderError(w, r, http.StatusForbidden, "only the instance owner may mark the digest seen") + return + } + if !s.sameOrigin(r) { + s.renderError(w, r, http.StatusForbidden, "this request did not originate from this site") + return + } + if err := s.reader.MarkDigestSeen(r.Context(), time.Now()); err != nil { + s.fail(w, r, err) + return + } + http.Redirect(w, r, "/inbox", http.StatusSeeOther) +} + +// proposalRows turns service proposals into listing rows for the open queue, +// where nothing is ever "new". 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), - }) + rows = append(rows, proposalRowOf(p)) } return rows } +// digestRows turns the digest proposals into rows, flagging each that +// auto-merged after the mark as new and counting them. With no mark yet +// (marked false) the whole digest is new — the owner has never cleared it. The +// digest arrives newest-first and a row is new iff its merge time is after the +// mark, so the new rows are the leading run and newCount is their length. +func digestRows(ps []service.Proposal, mark time.Time, marked bool) ([]proposalRow, int) { + rows := make([]proposalRow, 0, len(ps)) + newCount := 0 + for _, p := range ps { + row := proposalRowOf(p) + row.New = !marked || (p.Resolved != nil && p.Resolved.After(mark)) + if row.New { + newCount++ + } + rows = append(rows, row) + } + return rows, newCount +} + +// proposalRowOf builds one listing row, deriving its link from the space and id +// — the same stable /~owner/space/p/ shape the write plane hands back. +func proposalRowOf(p service.Proposal) proposalRow { + return proposalRow{ + Href: proposalHref(p), + ID: p.ID, + Space: p.Space.String(), + Title: p.Title, + Agent: p.Agent, + Approval: string(p.Approval), + } +} + // 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 { diff --git a/web/inbox_test.go b/web/inbox_test.go index 8bd739b70def389c9ed07ca8d4c69d4c382509af..e4b13331d1ef2f5929f2ebfff4dfc3853ff55727 100644 --- a/web/inbox_test.go +++ b/web/inbox_test.go @@ -2,13 +2,24 @@ package web import ( "net/http" + "net/http/httptest" "strings" "testing" + "time" "sourcecraft.dev/bigbes/sr-ht-spec/core" "sourcecraft.dev/bigbes/sr-ht-spec/service" ) +// mergedAt is a digest proposal that auto-merged at t — the timestamp the "since +// you last looked" divider compares against the owner's mark. +func mergedAt(id int, title string, t time.Time) service.Proposal { + return service.Proposal{ + ID: id, Space: demoSpace, Title: title, State: core.StateMerged, + Approval: core.ApprovalPolicy, Agent: "claude-code/x", Resolved: &t, + } +} + // 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. @@ -66,3 +77,99 @@ func TestInboxAnonymousRedirected(t *testing.T) { t.Fatalf("status = %d, want a login redirect", rec.Code) } } + +// TestInboxDividesNewFromSeen proves the digest flags what auto-merged after the +// mark as new, counts it, offers the mark-as-seen action, and draws the divider +// before the already-seen rows. +func TestInboxDividesNewFromSeen(t *testing.T) { + r := newFakeReader() + base := time.Date(2026, 7, 20, 12, 0, 0, 0, time.UTC) + r.mark, r.marked = base, true + // ID 9 merged after the mark (new); ID 8 before it (seen). The digest is + // newest-first, so 9 leads. + seedProposal(r, mergedAt(9, "Fresh merge", base.Add(time.Hour)), nil) + seedProposal(r, mergedAt(8, "Old merge", base.Add(-time.Hour)), 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, "1 new") { + t.Errorf("digest does not report one new item; body:\n%s", body) + } + if !strings.Contains(body, "/inbox/seen") { + t.Errorf("no mark-as-seen action offered when there is new content") + } + if !strings.Contains(body, "seen before your last visit") { + t.Errorf("no divider drawn between new and seen; body:\n%s", body) + } +} + +// TestInboxAllNewWithoutMark proves that before the owner has ever cleared the +// digest, everything in it counts as new. +func TestInboxAllNewWithoutMark(t *testing.T) { + r := newFakeReader() // marked is false: no mark yet. + seedProposal(r, mergedAt(4, "First", time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC)), nil) + seedProposal(r, mergedAt(5, "Second", time.Date(2026, 7, 2, 0, 0, 0, 0, time.UTC)), nil) + h, _, _ := testServerWith(t, r) + + 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(), "2 new") { + t.Errorf("with no mark, the whole digest should read as new; body:\n%s", rec.Body) + } +} + +// TestInboxSeenAdvancesMark proves the POST advances the owner's mark and +// redirects back, so the next render shows the just-seen items as no longer new. +func TestInboxSeenAdvancesMark(t *testing.T) { + r := newFakeReader() + h, reader, _ := testServerWith(t, r) + + rec := post(t, h, "/inbox/seen", "bigbes", "https://spec.example") + if rec.Code != http.StatusSeeOther { + t.Fatalf("status = %d, want 303", rec.Code) + } + if !reader.marked { + t.Errorf("the digest mark was not advanced by a valid POST") + } +} + +// TestInboxSeenForbiddenForAgent proves an agent — authenticated but not the +// owner — may not move the owner's mark. +func TestInboxSeenForbiddenForAgent(t *testing.T) { + r := newFakeReader() + h, reader, _ := testServerWith(t, r) + + req := httptest.NewRequest(http.MethodPost, "/inbox/seen", 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 marking seen", rec.Code) + } + if reader.marked { + t.Errorf("the mark moved despite the agent being refused") + } +} + +// TestInboxSeenRefusedCrossOrigin proves the mark-as-seen POST gets the same +// CSRF defense as approve/reject. +func TestInboxSeenRefusedCrossOrigin(t *testing.T) { + r := newFakeReader() + h, reader, _ := testServerWith(t, r) + + rec := post(t, h, "/inbox/seen", "bigbes", "https://evil.example") + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want 403 for a cross-origin POST", rec.Code) + } + if reader.marked { + t.Errorf("the mark moved despite the cross-origin refusal") + } +} diff --git a/web/reader.go b/web/reader.go index d5d17cc301e5b86e4aaa1fe65e1b18481073fdcc..b2086650706c79fcfd3ccef0fc01efe93827782f 100644 --- a/web/reader.go +++ b/web/reader.go @@ -2,6 +2,7 @@ package web import ( "context" + "time" "sourcecraft.dev/bigbes/sr-ht-spec/core" "sourcecraft.dev/bigbes/sr-ht-spec/doc" @@ -73,6 +74,13 @@ type Reader interface { // sees after the fact. Inbox(ctx context.Context) ([]service.Proposal, error) Digest(ctx context.Context) ([]service.Proposal, error) + + // DigestMark reports when the owner last marked the digest seen, and whether + // a mark exists yet; the inbox reads it to divide new auto-merges from seen + // ones and itself never writes. MarkDigestSeen advances it — the one write + // the review queue makes, behind an explicit POST so the GET stays pure. + DigestMark(ctx context.Context) (time.Time, bool, error) + MarkDigestSeen(ctx context.Context, seenAt time.Time) error } // Searcher is the keyword index. *search.Index satisfies it as declared. @@ -150,3 +158,11 @@ func (r serviceReader) Inbox(ctx context.Context) ([]service.Proposal, error) { func (r serviceReader) Digest(ctx context.Context) ([]service.Proposal, error) { return r.svc.DigestProposals(ctx, 0) } + +func (r serviceReader) DigestMark(ctx context.Context) (time.Time, bool, error) { + return r.svc.DigestMark(ctx) +} + +func (r serviceReader) MarkDigestSeen(ctx context.Context, seenAt time.Time) error { + return r.svc.MarkDigestSeen(ctx, seenAt) +} diff --git a/web/router.go b/web/router.go index 28e421f97118e9bde0e4743b6e353a09c4d599f7..183251237d5ff9dd877589ba98dbc92531f4737f 100644 --- a/web/router.go +++ b/web/router.go @@ -41,6 +41,7 @@ func (s *Server) Register(r chi.Router) { r.Get("/static/*", s.handleStatic) r.Get("/search", s.handleSearch) r.Get("/inbox", s.handleInbox) + r.Post("/inbox/seen", s.handleInboxSeen) // The proposal routes are registered before the document wildcard. chi gives // the static "p" segment priority over the "*" catch-all regardless, but diff --git a/web/templates/inbox.html b/web/templates/inbox.html index abba65baf7d11127b25c0f1c2bfe470c772b00eb..c991d304cb625a3874866adbef64d8e0861e1a1f 100644 --- a/web/templates/inbox.html +++ b/web/templates/inbox.html @@ -26,21 +26,39 @@

No open proposals. Your queue is clear.

{{end}} -

Recently auto-merged

+

+ Recently auto-merged + {{if .Data.NewCount}}{{.Data.NewCount}} new{{end}} +

Content the space's policy merged without stopping for review.

+ {{if .Data.NewCount}} +
+ +
+ {{end}} {{if .Data.Digest}} - {{range .Data.Digest}} + {{range $i, $r := .Data.Digest}} + {{if eq $i $.Data.NewCount}} + + + + {{end}} - - - + + + {{end}} diff --git a/web/web_test.go b/web/web_test.go index 91a990614f3507a3902efa6078250cbf9a153f13..1fc068536f327e042cff932f48e7f819670b5530 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -14,6 +14,7 @@ import ( "sort" "strings" "testing" + "time" "github.com/fernet/fernet-go" "github.com/vaughan0/go-ini" @@ -116,6 +117,11 @@ type fakeReader struct { proposals map[int]service.Proposal diffs map[int][]service.ProposalDoc actErr error + + // mark/marked back the digest_mark tests: the owner's "last looked at" + // timestamp and whether one has been set. MarkDigestSeen moves them. + mark time.Time + marked bool } func newFakeReader() *fakeReader { @@ -273,6 +279,15 @@ func (f *fakeReader) Digest(_ context.Context) ([]service.Proposal, error) { return out, nil } +func (f *fakeReader) DigestMark(context.Context) (time.Time, bool, error) { + return f.mark, f.marked, nil +} + +func (f *fakeReader) MarkDigestSeen(_ context.Context, seenAt time.Time) error { + f.mark, f.marked = seenAt, true + return nil +} + // fakeSearcher returns one fixed hit whose snippet carries the tags // bleve's highlighter emits. type fakeSearcher struct {
ProposalSpaceAgent
+ — seen before your last visit — +
#{{.ID}} — {{.Title}}{{.Space}}{{.Agent}} + #{{$r.ID}} — {{$r.Title}} + {{if $r.New}}new{{end}} + {{$r.Space}}{{$r.Agent}}