diff --git a/web/inbox.go b/web/inbox.go index d953f7c041a45c6f8c3bf4a41f7fe7302533349e..2ee35b8853ad5c90f8b3316f45caefb9a8251aa8 100644 --- a/web/inbox.go +++ b/web/inbox.go @@ -7,6 +7,7 @@ "strconv" "time" "go.bigb.es/auxilia/scribe" + "sourcecraft.dev/bigbes/sr-ht-ecore/chrome" "sourcecraft.dev/bigbes/sr-ht-spec/authn" "sourcecraft.dev/bigbes/sr-ht-spec/service" @@ -20,8 +21,18 @@ // 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. +// +// The two listings are not the same kind of thing, which is why only one of them +// is a chrome.RepoList. The open queue is a plain listing and renders through +// ecore's "srht-repo-table", so a proposal waiting on the owner looks like a +// repository on the sibling services. The digest is not a listing: it carries a +// per-row "new" badge and a divider row inserted between items at NewCount, and +// a partial over a flat []ListItem can express neither — Meta is plain text, so +// a badge would come out escaped, and nothing can interleave a row that is not +// an item. It stays this package's own markup rather than being flattened into +// something that loses the one thing the page is for. type inboxData struct { - Open []proposalRow + Open chrome.RepoList Digest []proposalRow NewCount int } @@ -69,7 +80,7 @@ digestRows, newCount := digestRows(digest, mark, marked) vd := s.view(r, "Review queue") vd.Data = inboxData{ - Open: proposalRows(open), + Open: openQueue(open), Digest: digestRows, NewCount: newCount, } @@ -97,14 +108,29 @@ } 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)) +// emptyQueue is what the open queue says when there is nothing waiting. It is +// the sentence the page used to carry inline, moved to where the partial reads +// it from. +const emptyQueue = "No open proposals. Your queue is clear." + +// openQueue turns the open proposals into ecore's listing shape. +// +// The title carries the id because a proposal is addressed by number and the +// number is what an agent quotes back; the space and the agent are Meta, which +// the table renders as its own columns in order. There is no Updated: a proposal +// row's useful timestamp is when it was opened, and the read layer does not +// carry one — see the report on this uplift. +func openQueue(ps []service.Proposal) chrome.RepoList { + list := chrome.RepoList{Empty: emptyQueue} for _, p := range ps { - rows = append(rows, proposalRowOf(p)) + row := proposalRowOf(p) + list.Items = append(list.Items, chrome.ListItem{ + Href: row.Href, + Title: "#" + strconv.Itoa(row.ID) + " — " + row.Title, + Meta: []string{row.Space, row.Agent}, + }) } - return rows + return list } // digestRows turns the digest proposals into rows, flagging each that diff --git a/web/inbox_test.go b/web/inbox_test.go index e4b13331d1ef2f5929f2ebfff4dfc3853ff55727..d0bf609c4a075e97792f0dd08cc7c4623b96f47e 100644 --- a/web/inbox_test.go +++ b/web/inbox_test.go @@ -7,6 +7,8 @@ "strings" "testing" "time" + "github.com/stretchr/testify/assert" + "sourcecraft.dev/bigbes/sr-ht-spec/core" "sourcecraft.dev/bigbes/sr-ht-spec/service" ) @@ -54,6 +56,24 @@ } if strings.Contains(body, "Human-merged") { t.Errorf("a human-merged proposal leaked into the review queue") } +} + +// The open queue is drawn by ecore's "srht-repo-table", so the space and the +// agent ride in the item's Meta rather than in columns this package writes. This +// is the seam: that both still reach the page, since "who proposed this, and +// where" is what the queue is scanned for. +func TestInboxOpenQueueCarriesSpaceAndAgent(t *testing.T) { + r := newFakeReader() + seedProposal(r, service.Proposal{ + ID: 3, Space: demoSpace, Title: "Open one", State: core.StateOpen, + Agent: "claude-code/a", + }, nil) + h, _, _ := testServerWith(t, r) + + body := get(t, h, "/inbox", "bigbes").Body.String() + assert.Contains(t, body, "#3 — Open one") + assert.Contains(t, body, demoSpace.String()) + assert.Contains(t, body, "claude-code/a") } // TestInboxEmpty proves the page renders with no proposals rather than erroring. diff --git a/web/server.go b/web/server.go index 6eef2f0ff78289f1079b9501f0802230d4a82c8f..0586024762f7f2b98e5cf97436565b8f5186e34f 100644 --- a/web/server.go +++ b/web/server.go @@ -87,6 +87,7 @@ package web import ( "fmt" + "html/template" "io/fs" "log/slog" "net/http" @@ -213,6 +214,19 @@ } // chrome.Page renders a bare page for an empty StyleHref rather than an // empty , so an unstyled build stays a presentation failure. chromeSvc.StyleHref = cssHref + + // This service ships its own icon, so it overrides chrome's built-in data: + // URI with it — resolved rather than spelled, so that the day logo.svg is + // hashed or renamed the href follows and an absent one is "" (no ) + // instead of a 404 on every page load. The glob is exact today; it is a glob + // so that a hashed name needs no second edit here. + iconHref, err := assets.Resolve(staticFS, "static/logo.*svg", assets.DefaultPrefix) + if err != nil { + return nil, fmt.Errorf("web: %w", err) + } + if iconHref != "" { + chromeSvc.FaviconHref = template.URL(iconHref) + } // A page that defines no "content" is refused here rather than serving a // 200 around a hole, so this error is a startup failure and not a warning. diff --git a/web/templates/inbox.html b/web/templates/inbox.html index c991d304cb625a3874866adbef64d8e0861e1a1f..f8454bf8e8d7407a08ecb1bd42f0b04c8272995b 100644 --- a/web/templates/inbox.html +++ b/web/templates/inbox.html @@ -5,26 +5,22 @@

Review queue

Waiting on you - {{len .Data.Open}} + {{len .Data.Open.Items}}

- {{if .Data.Open}} - - - - - - {{range .Data.Open}} - - - - - - {{end}} - -

Proposal	Space	Agent
#{{.ID}} — {{.Title}}	`{{.Space}}`	`{{.Agent}}`

- {{else}} -

No open proposals. Your queue is clear.

- {{end}} + {{/* The listing markup is ecore's, so a proposal waiting on the owner and a + repository on a sibling service look like the same kind of thing. The + columns after the title are the item's Meta — the space, then the agent + — and the partial carries the empty-queue sentence itself. + + What is lost against the hand-written table this replaces is the header + row: srht-repo-table draws no headings, because the meaning of the + columns it renders is known only to the service supplying them. Here + that costs the "Space" and "Agent" labels, which the values say plainly + enough on their own (~owner/space, and an agent identity). + + The digest below is deliberately NOT this partial; inboxData's doc + comment says what it needs that a flat listing cannot express. */}} + {{template "srht-repo-table" .Data.Open}}

Recently auto-merged diff --git a/web/templates/layout.html b/web/templates/layout.html index 4f4f7ce8b4f4f78cd7e07ba1505f1cdb7bde468e..b0b8b1ac6abd721dc0cfbf162dd89e57290f8bc5 100644 --- a/web/templates/layout.html +++ b/web/templates/layout.html @@ -17,11 +17,17 @@ {{.Title}} - - {{/* Empty when this binary was built without `make css`. The link is - guarded rather than emitted empty: re-requests the page - it is on, which is a page load per page load. /}} - {{if .StyleHref}}{{end}} + {{/ The stylesheet and the favicon, both from sr-ht-ecore's own partial and + both guarded there rather than emitted empty: re-requests + the page it is on, which is a page load per page load, and the + stylesheet is empty in a binary built without `make css`. + + The icon used to be the literal /static/logo.svg written here. It is + still that file, but the href is now resolved at startup through + assets.Resolve: a path a template asserts is one that 404s on every page + load if the file is ever renamed or hashed, where a resolved one is + absent once, at startup, in front of whoever can fix it. /}} + {{template "srht-head-links" .}} {{block "head" .}}{{end}} diff --git a/web/web_test.go b/web/web_test.go index 803aeab527971e025b045bc9a16acfb97e7966c4..7143ff5110d31ea9d1389cb8f8a4835768317aad 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -769,6 +769,19 @@ // and is tested there. What is left here is the seam: that this service hands // ecore the identity its own authn resolved, and that the page around the // chrome shows the right thing to that identity. +// The links come from ecore's own partial, and the icon is this +// service's: chrome ships a built-in data: URI as its default and New overrides +// it with the resolved href of the embedded logo. What this pins is the seam — +// that the override reached the page — and not the partial, which is ecore's. +func TestPagesLinkThisServicesIcon(t testing.T) { + h, _ := testServer(t) + body := get(t, h, "/", "bigbes").Body.String() + assert.Contains(t, body, ``) + // This build has no stylesheet, and the partial guards the link rather than + // emitting an empty href, which would re-request the page it sits on. + assert.NotContains(t, body, ``) +} + // The owner's cookie must reach the chrome as an identity: ecore renders the // login block from the username it is given, so a greeting by name is the proof // that view() passed one.

File	Status	+	−
`web/inbox.go`	M	+34	-8
`web/inbox_test.go`	M	+20
`web/server.go`	M	+14
`web/templates/inbox.html`	M	+15	-19
`web/templates/layout.html`	M	+11	-5
`web/web_test.go`	M	+13

~bigbes/sr-ht-spec: 471d9706

6 changed file(s)

~bigbes/sr-ht-spec: `471d9706`