~bigbes/sr-ht-compare · issues

hteik5ojpg0e77fhp6f1pul8ohgg7dqj · 9 rows

idcontent_hashtitledescriptiondesignacceptance_criterianotesstatuspriorityissue_typeassigneeestimated_minutescreated_atcreated_byownerupdated_atclosed_atclosed_by_sessionexternal_refspec_idcompaction_levelcompacted_atcompacted_at_commitoriginal_sizesenderephemeralwisp_typepinnedis_templatemol_typework_typesource_systemmetadatasource_repoclose_reasonevent_kindactortargetpayloadawait_typeawait_idtimeout_nswaitershook_beadrole_beadagent_statelast_activityrole_typerigdue_atdefer_untilno_historystarted_atis_blocked
sr-ht-compare-11dd93cb90c612f995ac19f371322ae99fc738ba4348621e8b6f91e81348d776ffaGive compare's CSRF guard a rendered deny handler instead of nilWhat is wrong: sourcehut-compare/web/router.go installs 'r.Use(csrf.Require(s.chromeSvc.SelfOrigin(), nil))'. Passing nil selects csrf.denyPlain, a bare text/plain http.Error. Every sibling — sourcehut-{tokens,bench,coverage,curator,specs,artifacts,dolt}/web/router.go — passes a renderer, e.g. 'func(w, r) { s.renderError(w, r, http.StatusForbidden, csrf.Message) }'. Status and message already agree with the siblings; only the presentation differs. Why it matters: compare has no POST routes today, so nothing is refused yet — but the first form added gets a naked text/plain 403 on a surface whose every other refusal is a rendered page with nav. The router's own comment explains the guard exists 'because the day somebody adds the first POST is exactly the day nobody remembers to add the check'; the same argument applies to the renderer. csrf.Require's doc calls the nil default 'a usable default rather than an invitation to skip the middleware', and compare already has an error renderer (s.renderError) it simply isn't wiring in. Correct variant: the seven siblings that pass a renderer, e.g. repo/path sourcehut-tokens/web/router.go::csrf.Require call site. Fix: sourcehut-compare/web/router.go — replace the nil argument with 'func(w, r) { s.renderError(w, r, http.StatusForbidden, csrf.Message) }'. Note: the register rates this consistency after two source reports disagreed (one called it cosmetic, one correctness-risk); both agree there are currently no POST routes so nothing is refused today. Audit: D34 (cross-repo audit, 2026-08-17)open3choreNULLNULL2026-08-17T21:37:43Zbigbesbigbes@gmail.com2026-08-17T21:37:43ZNULLNULL0NULLNULLNULL000�{}0NULLNULLNULL0NULL0
sr-ht-compare-4yv3a6f90aef7850f81dba8c5e2c73048f090739410fbe0de7bed713ed762731d1dMake sourcehut-compare/Makefile's css recipe portable off GNU sha256sumWhat is wrong: the css recipe in sourcehut-compare/Makefile ends with 'mv web/static/main.min.css web/static/main.min.$$(sha256sum ... | cut -c1-8).css'. On a machine without GNU coreutils, sha256sum does not exist, the command substitution yields empty, and mv produces web/static/main.min..css. This affects six Makefiles: sourcehut-{artifacts,compare,coverage,dolt,specs,tokens}/Makefile. Why it matters: that filename does not match main.min.*.css, so assets.Resolve answers "" and every page renders unstyled, with no error anywhere (set -e does not catch a failure inside $$( )). Compounded by D09 (compare tracks the old build artefact) and the compare/dolt/specs/artifacts gap in check-version/check-embedded-css (D12). Correct variant: repo/path sourcehut-bench/Makefile and sourcehut-curator/Makefile, which define 'SHA256SUM ?= sha256sum' and use $(SHA256SUM), with a comment naming 'make SHA256SUM="shasum -a 256" css' as the macOS invocation. Fix: add 'SHA256SUM ?= sha256sum' to sourcehut-compare/Makefile and substitute $(SHA256SUM) for the raw sha256sum call in the css recipe. Audit: D08 (cross-repo audit, 2026-08-17)open1bugNULLNULL2026-08-17T21:36:10Zbigbesbigbes@gmail.com2026-08-17T21:36:10ZNULLNULL0NULLNULLNULL000�{}0NULLNULLNULL0NULL0
sr-ht-compare-65gc3a07761c446057e82e20332a5c957ced1c742de31ad5dba2c95a43597585d38Move compare's inline layout.html CSS into scss/main.scssWhat is wrong: sourcehut-compare/web/templates/layout.html has a 28-line inline <style> block (the .diff-status* rules). sourcehut-compare's scss/main.scss is about 200 lines and contains no .diff-status rule at all — the two are unrelated to each other. (Companion finding: sourcehut-dolt has ~467 more lines of inline CSS across four templates, tracked separately in dolt's tracker.) Why it matters: inline CSS is re-sent on every page load, sits outside the hashed-asset immutable cache, cannot use the shared SCSS variables ($font-family-monospace, the dark palette), and — because it lives in layout.html specifically — is paid for on every single page compare serves, including the error page. compare already has a working 'make css' pipeline that the six other siblings use for exactly this content. Correct variant: the six services that keep everything in scss/main.scss below '@import "base"', e.g. repo/path sourcehut-tokens/scss/main.scss. Fix: move the .diff-status* rules from sourcehut-compare/web/templates/layout.html into sourcehut-compare/scss/main.scss, then delete the <style> block from layout.html. Audit: D37 (cross-repo audit, 2026-08-17)open3choreNULLNULL2026-08-17T21:37:44Zbigbesbigbes@gmail.com2026-08-17T21:37:44ZNULLNULL0NULLNULLNULL000�{}0NULLNULLNULL0NULL0
sr-ht-compare-cz1d4ed69b8f83cc9d16ac512119174c4e0e047f5c5a5a0d6f22e4e3b9ff078f32cAdd check-version and check-embedded-css targets to sourcehut-compare/MakefileWhat is wrong: sourcehut-compare/Makefile defines no check-version: target (nor check-embedded-css); its APKBUILD build() runs only 'make check-css' and package() re-verifies nothing. This gap exists in four repos: sourcehut-{artifacts,compare,dolt,specs}/Makefile. Why it matters: check-version inspects the compiled binary (go version -m $(CHECK_BIN)) for vcs.revision/vcs.modified and fails the build if the tree was dirty; check-embedded-css greps the binary for the embedded stylesheet name and diffs it against what is on disk, catching 'compiled before make css ran'. compare's own D09 (tracked build artefact that make css deletes) makes compare hit exactly the dirty-tree failure mode this gate exists to catch, on every local make css, and nothing enforces it. Correct variant: repo/path sourcehut-tokens/Makefile — bench, coverage, curator, tokens (both targets) and federation (check-version only) all call these from both build() and package(), so the gate runs against the artefact that actually ships. Fix: port check-version/check-embedded-css plus their CHECK_BIN/CHECK_BINS staged-artefact invocations from sourcehut-tokens/Makefile into sourcehut-compare/Makefile and its APKBUILD build()/package(). Audit: D12 (cross-repo audit, 2026-08-17)open2bugNULLNULL2026-08-17T21:36:11Zbigbesbigbes@gmail.com2026-08-17T21:36:11ZNULLNULL0NULLNULLNULL000�{}0NULLNULLNULL0NULL0
sr-ht-compare-d4m9ad11b77c76bb64020f443627f8a476d44a5bcd875cdf25af89c4d87b7fd0c84Switch compare's template embed to the all:templates prefixWhat is wrong: sourcehut-compare embeds its template tree with a bare glob, 'templates/*.html' (in the //go:embed directive), instead of the 'all:templates' directory-walk prefix that artifacts/bench/coverage/curator/tokens use (each with a comment noting the all: prefix is load-bearing). specs uses the same bare-glob form as compare; dolt uses a two-pattern glob ('templates/*.html templates/icons/*.svg'). Why it matters: nothing is broken today (a glob still picks up _-prefixed partials, unlike a directory walk), but the glob form is fragile going forward: a partial placed in a subdirectory, or any non-.html fragment, is silently NOT embedded and fails at request time as 'no such template' — dolt already had to add a second glob pattern for its icons/ subdirectory to work around exactly this. Correct variant: 'all:templates', e.g. repo/path sourcehut-tokens's //go:embed directive over its templates tree. Fix: change sourcehut-compare's //go:embed directive from 'templates/*.html' to 'all:templates'. Audit: D39 (cross-repo audit, 2026-08-17)open3choreNULLNULL2026-08-17T21:37:46Zbigbesbigbes@gmail.com2026-08-17T21:37:46ZNULLNULL0NULLNULLNULL000�{}0NULLNULLNULL0NULL0
sr-ht-compare-f6ec7291faccf05d3d977171de95de133c614db74303c64d102c81d5f59a4d1e764Pass *http.Request into compare's template render error logWhat is wrong: sourcehut-compare/web/templates.go::(*Server).render logs 'slog.Error("web: render", scribe.Err(err), "page", page, "status", status)' with no *http.Request, so there is no ErrorContext, no method, no path, and no request id. Every other service in the family logs 'slog.ErrorContext(r.Context(), ..., "method", r.Method, "path", r.URL.Path, scribe.Err(err))', the shape pages.Render's own doc-comment shows. Why it matters: chimw.RequestLogger's request id lives in the request context and is dropped here, so a render failure in compare cannot be correlated with the request line that produced it — the one piece of observability every sibling gets for free. Correct variant: the seven siblings, e.g. repo/path sourcehut-tokens/web/templates.go::render (or pages.Render's own doc-comment) for the ErrorContext shape. Fix: thread an *http.Request into sourcehut-compare/web/templates.go::(*Server).render and switch the log call to slog.ErrorContext(r.Context(), ...) including method and path. Audit: D35 (cross-repo audit, 2026-08-17)open3choreNULLNULL2026-08-17T21:37:44Zbigbesbigbes@gmail.com2026-08-17T21:37:44ZNULLNULL0NULLNULLNULL000�{}0NULLNULLNULL0NULL0
sr-ht-compare-g779de449836d2118c925ba7fecbe77c2b5528f3b24fcc0d83cbc879606e52a930eBump gqlparser/gqlgen off the GO-2024-2920 floor in compare/go.modWhat is wrong: sourcehut-compare/go.mod pins github.com/vektah/gqlparser/v2 v2.5.8 and github.com/99designs/gqlgen v0.17.36. gqlparser v2.5.8 is below the fix for GO-2024-2920 (a DoS in parseDirectives), which landed in v2.5.14. Five modules share this stale pin: sourcehut-specs/go.mod, sourcehut-compare/go.mod, sourcehut-tokens/go.mod, sr-ht-core/go.mod, sr-ht-ecore/go.mod. Why it matters: sr-ht-core and sr-ht-ecore are libraries that compare depends on, so their floor becomes compare's floor regardless of whether compare's own /query path requires a credential first. Correct variant: gqlparser/v2 v2.5.36 + gqlgen v0.17.94, already in use by sourcehut-{artifacts,bench,coverage,curator,dolt}, thistle, sr-ht-api and sourcehut-federation (repo/path: sourcehut-bench/go.mod as a reference). Fix: bump sr-ht-core and sr-ht-ecore to gqlparser/v2@v2.5.36 + gqlgen@v0.17.94 first, then run 'go get github.com/vektah/gqlparser/v2@v2.5.36 github.com/99designs/gqlgen@v0.17.94' in sourcehut-compare/go.mod and re-run go mod tidy. Audit: D06 (cross-repo audit, 2026-08-17)open0bugNULLNULL2026-08-17T21:36:09Zbigbesbigbes@gmail.com2026-08-17T21:36:09ZNULLNULL0NULLNULLNULL000�{}0NULLNULLNULL0NULL0
sr-ht-compare-jdqcea9b707c3ee54120545e344272afc44380b8c68685835f8c7aba5ec03e59276Untrack the compiled CSS/JS bundle that make css deletes in sourcehut-compareWhat is wrong: sourcehut-compare/web/static/main.min.79713f25.css (142 KB) and web/static/bundle.a62c2e63.js (10 MB) are both tracked in git, while sourcehut-compare/.gitignore already has the patterns /web/static/main.css and /web/static/main.min.*.css (with a NOTE that removal from the index 'waits on the first CI run'). A tracked file is unaffected by a later-added ignore pattern, and 'make css' opens with 'rm -f web/static/main.css $(CSS)' — it deletes a tracked file. Why it matters: every developer who runs 'make css' gets a dirty working tree, and Go stamps every binary built afterwards vcs.modified=true, which is exactly what check-version exists to catch — and compare is one of the four repos that has no check-version target (see D12/sr-ht-compare issue for that). Separately, install-files copies web/static/*, so a stale tracked CSS would be staged if make css is skipped. Correct variant: the seven sibling services, all of which gitignore the compiled stylesheet and cannot hit this class of bug. Fix: git rm --cached web/static/main.min.79713f25.css (the ignore entry already exists). Separately decide whether the 10 MB esbuild bundle from frontend/ (web/static/bundle.a62c2e63.js) should be built in CI like the CSS rather than committed, and gitignore it too if so. Audit: D09 (cross-repo audit, 2026-08-17)open1bugNULLNULL2026-08-17T21:36:10Zbigbesbigbes@gmail.com2026-08-17T21:36:10ZNULLNULL0NULLNULLNULL000�{}0NULLNULLNULL0NULL0
sr-ht-compare-vos6708ad7c99bfd4b1bd2621479d3f656f6fdd32d05874a9fccbd554b3d53ace1cMount compare's /static/ with chimw.GetHead instead of r.HandleWhat is wrong: sourcehut-compare/web/router.go mounts static assets with 'r.Handle(assets.DefaultPrefix+"*", s.static)', which registers every HTTP method. Two other idioms exist in the family: chimw.GetHead(...) in artifacts/bench/curator/tokens, and r.Mount(...) in specs. compare and dolt both use r.Handle. Why it matters: with r.Handle or r.Mount, a same-origin POST to e.g. /static/main.min.<sha>.css is served the file with a 200 instead of compare's rendered 405 — harmless today, but the routing tree no longer describes what the service actually serves, which is exactly the argument chimw.GetHead's own doc makes for registering the GET/HEAD pair explicitly instead of accepting every method. Correct variant: chimw.GetHead, e.g. repo/path sourcehut-tokens/web/router.go static mount. Fix: sourcehut-compare/web/router.go — replace 'r.Handle(assets.DefaultPrefix+"*", s.static)' with 'chimw.GetHead(r, assets.DefaultPrefix+"*", s.static)' (or the service's existing GetHead wrapper). Audit: D38 (cross-repo audit, 2026-08-17)open3choreNULLNULL2026-08-17T21:37:45Zbigbesbigbes@gmail.com2026-08-17T21:37:45ZNULLNULL0NULLNULLNULL000�{}0NULLNULLNULL0NULL0