Commit 23159cdf3a
Verified · cmc
Layout: unified · split
ROADMAP.md added +323
| @@ -0,0 +1,323 @@ | |||
| 1 | # Roadmap | ||
| 2 | |||
| 3 | Findings from a full review on 2026-09-10, ordered for work. Each item is | ||
| 4 | sized to be one gitbay issue and one merge request. Sizes: S is under an | ||
| 5 | hour, M is a few hours, L needs a design pass first. | ||
| 6 | |||
| 7 | Verified on: local build from `main` at 028903a, live instance | ||
| 8 | art.krz.sh (v1.4.0), devianter v0.3.4. DeviantArt's WAF blocks the | ||
| 9 | reviewing machine's egress IP, so DA-backed pages were checked on the live | ||
| 10 | instance. | ||
| 11 | |||
| 12 | Existing gitbay issues: #1 cleanup, #2 search filters, #3 description | ||
| 13 | parsing, #4 instance checker, #5 Makefile, #6 emote bug. They are mapped | ||
| 14 | below where they overlap. | ||
| 15 | |||
| 16 | ## Ordering principle | ||
| 17 | |||
| 18 | Frontend proxies get blocked upstream. The existing throttle | ||
| 19 | (`app/httpclient.go`) spreads requests out; it does not reduce them. Tier 1 | ||
| 20 | reduces them. Tier 0 lands first because everything after it needs CI. | ||
| 21 | Tier 2's escaping fix lands before other template work because it touches | ||
| 22 | every template and builder. | ||
| 23 | |||
| 24 | ## Upstream cost today | ||
| 25 | |||
| 26 | Nothing from DeviantArt's JSON API is cached. Only wixmp media is cached, | ||
| 27 | and only when `cache.enabled` is on, which defaults to off. | ||
| 28 | |||
| 29 | | Page | Upstream calls per view | | ||
| 30 | |---|---| | ||
| 31 | | Post | 2 API (deviation, comments) + 1 avatar per commenter | | ||
| 32 | | User about | 2 API (gruser, comments) + avatars | | ||
| 33 | | Gallery, favourites, search, DD | 1 API, plus thumbnails when the media cache is off | | ||
| 34 | | `/api/random` | up to 3 searches | | ||
| 35 | | Atom feed | 1 API per poll, per reader | | ||
| 36 | |||
| 37 | Avatars come from a.deviantart.net and are fetched fresh on every view. | ||
| 38 | Concurrent requests for the same page each go upstream. No response carries | ||
| 39 | `Cache-Control`. There is no robots.txt and no per-client rate limit. | ||
| 40 | |||
| 41 | ## Tier 0: process | ||
| 42 | |||
| 43 | ### 0.1 CI on merge requests (S, #7) | ||
| 44 | |||
| 45 | No pipeline runs `go vet`, `go test` or `golangci-lint`. The only workflow | ||
| 46 | is the GitHub release build; gitbay has zero builds. `.golangci.yml` exists | ||
| 47 | but is never run. | ||
| 48 | |||
| 49 | Fix: add a gitbay build that runs vet, test with `-race`, and golangci-lint | ||
| 50 | on every MR. Check `gitbay build --help` and `ssh git@gitbay.org help build` | ||
| 51 | for the pipeline format. Optionally mirror the same job to | ||
| 52 | `.github/workflows/` so the GitHub mirror shows status too. | ||
| 53 | |||
| 54 | Verify: an MR with a failing test shows a failed build. | ||
| 55 | |||
| 56 | ## Tier 1: reduce upstream requests | ||
| 57 | |||
| 58 | ### 1.1 Cache DeviantArt API responses (L, #8) | ||
| 59 | |||
| 60 | Add an in-memory cache in front of every devianter call: DD, search, | ||
| 61 | deviation, gruser, gallery, favourites, comments. Keyed by endpoint plus | ||
| 62 | arguments. Bounded by entry count or bytes. Singleflight so concurrent | ||
| 63 | requests for one key share a single upstream call. Per-endpoint TTLs, in | ||
| 64 | config with defaults on the order of: DD and search a few minutes, | ||
| 65 | deviations longer, comments shorter. | ||
| 66 | |||
| 67 | This is the item the TODO at `app/cache.go:3` names. It makes feed polling | ||
| 68 | and `/api/random` close to free. | ||
| 69 | |||
| 70 | Design first: cache interface, key derivation, TTL config keys, eviction, | ||
| 71 | what the hit/miss log looks like. Write the spec to | ||
| 72 | `docs/superpowers/specs/` and review before code. | ||
| 73 | |||
| 74 | Files: new `app/apicache.go`, call sites in `app/wrapper.go`, | ||
| 75 | `app/api.go`, `app/api_json.go`, config in `app/config.go`, docs in | ||
| 76 | `SETUP.md`. | ||
| 77 | |||
| 78 | Verify: unit tests for TTL expiry, singleflight, and bounded size. Two | ||
| 79 | sequential requests for one page make one upstream call. | ||
| 80 | |||
| 81 | ### 1.2 Cache avatars and emotes, add Cache-Control (M, #9) | ||
| 82 | |||
| 83 | `Emojitar` in `app/wrapper.go` fetches from a.deviantart.net or | ||
| 84 | e.deviantart.net on every request and never stores the result. Route it | ||
| 85 | through the same disk and memory cache path as `DownloadAndSendMedia`. | ||
| 86 | |||
| 87 | Add `Cache-Control` headers: long `max-age` with `immutable` on | ||
| 88 | `/media/file` (token-signed wixmp URLs do not change), a day on avatars and | ||
| 89 | emotes, a day on `/stylesheet` and `/favicon.ico`, a short `max-age` on HTML | ||
| 90 | matching the API cache TTL. | ||
| 91 | |||
| 92 | Depends on: nothing, but pairs with 1.1. | ||
| 93 | |||
| 94 | Verify: second avatar request is served without an upstream fetch; headers | ||
| 95 | present in `curl -I` output. | ||
| 96 | |||
| 97 | ### 1.3 robots.txt and per-client rate limit (S, #10) | ||
| 98 | |||
| 99 | Serve `/robots.txt` disallowing `/search`, `/api`, `/group_user`, | ||
| 100 | `/media` and any path with `?p=`. Add a per-client-IP token bucket ahead of | ||
| 101 | the upstream throttle so one crawler cannot consume the whole DA budget and | ||
| 102 | turn it into latency for everyone else. Honour `X-Forwarded-For` only when | ||
| 103 | the request came from a configured trusted proxy. | ||
| 104 | |||
| 105 | Files: `app/router.go`, new `app/ratelimit.go`, `app/config.go`, | ||
| 106 | `SETUP.md`. | ||
| 107 | |||
| 108 | Verify: test that N+1 requests from one address within the window get 429. | ||
| 109 | |||
| 110 | ### 1.4 Fewer calls per page (M, #11) | ||
| 111 | |||
| 112 | Post view is two API calls because comments are fetched inline. Move | ||
| 113 | comments behind a link (`/post/{author}/{name}/comments` or `?comments=1`) | ||
| 114 | so the default post view is one call. Same for the user about page. | ||
| 115 | |||
| 116 | Rebuild `/api/random` to pick from cached DD or search results (1.1) instead | ||
| 117 | of issuing up to three fresh searches per hit. | ||
| 118 | |||
| 119 | Depends on: 1.1. | ||
| 120 | |||
| 121 | Verify: post view makes exactly one upstream call in a test with a fake | ||
| 122 | transport. | ||
| 123 | |||
| 124 | ### 1.5 Media cache on by default (S, #12) | ||
| 125 | |||
| 126 | A proxying instance with no cache re-fetches every image from wixmp on every | ||
| 127 | view. Set `cache.enabled: true` in the built-in defaults in `app/config.go` | ||
| 128 | and in `config.example.json`, with a sane `lifetime` and `max-size`. Keep | ||
| 129 | `memcache` off. Document the change in `SETUP.md`. | ||
| 130 | |||
| 131 | Verify: fresh start with no config writes to the cache directory. | ||
| 132 | |||
| 133 | ## Tier 2: security and correctness | ||
| 134 | |||
| 135 | ### 2.1 Escape template output (M, #13) | ||
| 136 | |||
| 137 | `app/util.go` imports `text/template`. Nothing interpolated is escaped: the | ||
| 138 | search query in `static/html/search.htm` and `head.htm`, and every DA | ||
| 139 | username, title and description written by `DeviationList`, | ||
| 140 | `ParseComments`, `BuildUserPlate` and `ParseDescription`. The CSP blocks | ||
| 141 | scripts but not markup, inline styles, meta refresh or injected forms; any | ||
| 142 | title containing `<` corrupts the page. | ||
| 143 | |||
| 144 | Fix: switch to `html/template`; wrap the pre-built HTML fragments in | ||
| 145 | `template.HTML`; escape strings in the Go builders with | ||
| 146 | `html.EscapeString` and attribute-escape URLs. Add tests that a query and a | ||
| 147 | title containing `"><b>` render as text. | ||
| 148 | |||
| 149 | Land before other template work. | ||
| 150 | |||
| 151 | ### 2.2 Restore the user About branch (S, #14) | ||
| 152 | |||
| 153 | `app/wrapper.go:35` has `else if false`, inherited from upstream commit | ||
| 154 | 048bb47. Registration date, interests, social links and bio never render for | ||
| 155 | users. Find out why it was disabled (likely a devianter struct change), | ||
| 156 | restore the branch, add a test with a fixture. | ||
| 157 | |||
| 158 | ### 2.3 Group search pagination (S, #15) | ||
| 159 | |||
| 160 | `app/wrapper.go:274` increments the page and requests offset `10*page`, so | ||
| 161 | page two starts at result 20 and results 10 to 19 are never shown. The nav | ||
| 162 | bar also shows the incremented number. Use `10*(page-1)` and do not mutate | ||
| 163 | `s.Page` before `NavBase`. | ||
| 164 | |||
| 165 | ### 2.4 Emojitar writes a body after 404 (S, #16) | ||
| 166 | |||
| 167 | `app/wrapper.go:344` lacks a `return` after `ReturnHTTPError(404)`. | ||
| 168 | |||
| 169 | ### 2.5 Valid Atom feed (S, #17) | ||
| 170 | |||
| 171 | `DeviationList` in `app/parsers.go` emits no feed-level `<id>` or | ||
| 172 | `<updated>`, bare integer entry ids, RFC 1123 `<published>` instead of RFC | ||
| 173 | 3339, and `media:thumbinal`. Verified on the live feed. Fix all five and add | ||
| 174 | a test that parses the output with an Atom library or checks the required | ||
| 175 | elements. | ||
| 176 | |||
| 177 | ### 2.6 `-c` bounds check (S, #18) | ||
| 178 | |||
| 179 | `app/cli.go:29` checks `len(a) >= 2` instead of `n+1 < len(a)`; | ||
| 180 | `skunkyart -x -c` panics. | ||
| 181 | |||
| 182 | ### 2.7 Sanitize the 502 page (S, #19) | ||
| 183 | |||
| 184 | `Error` in `app/util.go` writes the upstream error, including the full | ||
| 185 | CloudFront block page, into an `<h3>` unescaped. Truncate to one line and | ||
| 186 | escape. Folds into 2.1 if done together. | ||
| 187 | |||
| 188 | ### 2.8 Parse templates once (S, #20) | ||
| 189 | |||
| 190 | `ExecuteTemplate` calls `ParseFS` on every request. Parse at startup; | ||
| 191 | supply the per-request `T` function through the data struct or a per-request | ||
| 192 | `Funcs` clone. Template errors then fail at boot instead of as 500s. | ||
| 193 | |||
| 194 | ## Tier 3: config, docs, i18n | ||
| 195 | |||
| 196 | ### 3.1 Config-less start and default alignment (S, #21) | ||
| 197 | |||
| 198 | `ExecuteConfig` exits if `config.json` is missing even though defaults | ||
| 199 | exist. Start with defaults when no `-c` is given and the default file is | ||
| 200 | absent. Align the built-in `nsfw: true` with the example's `false`, or | ||
| 201 | document why they differ. | ||
| 202 | |||
| 203 | ### 3.2 Cache documentation (S, #22) | ||
| 204 | |||
| 205 | `SETUP.md`: `update-interval` is in seconds (the example scans every 5s); | ||
| 206 | the `d` unit works but is unlisted; `y` is 360 days; exceeding `max-size` | ||
| 207 | deletes the whole cache directory; `lifetime: null` in the example. | ||
| 208 | |||
| 209 | ### 3.3 API and search type docs (S, #23) | ||
| 210 | |||
| 211 | `API.md` says `t` is text search; devianter defines it as tag. The | ||
| 212 | "Folders" option in `static/html/gruser.htm` maps to `f`, which is | ||
| 213 | favourites. Fix the doc and rename or remove the option. | ||
| 214 | |||
| 215 | ### 3.4 i18n coverage (M, #24) | ||
| 216 | |||
| 217 | Go-built HTML hardcodes English: comment headers, "In reply to", | ||
| 218 | pagination, folder and content headings, "No results", "[ TEXT ]". | ||
| 219 | `gruser.htm` section headings and the index blurb are untranslated. Every | ||
| 220 | template declares `lang="en"`. `Languages()` in `app/i18n.go` is unused. | ||
| 221 | Move the strings into the catalogues, set `lang` from the resolved | ||
| 222 | language, and either use or remove `Languages()`. | ||
| 223 | |||
| 224 | ### 3.5 systemd unit (S, #25) | ||
| 225 | |||
| 226 | `services/skunkyart.example.service` uses `Directory=` (not a valid key), | ||
| 227 | placeholder paths, and says it was never tested. Write a working unit with | ||
| 228 | `WorkingDirectory`, `User`, `DynamicUser` or a dedicated user, | ||
| 229 | `NoNewPrivileges`, and `Restart=on-failure`. Test it once on a Linux host. | ||
| 230 | |||
| 231 | ### 3.6 SETUP.md structure (S, #26) | ||
| 232 | |||
| 233 | The nginx section sits between config keys; `theme` and `language` come | ||
| 234 | after it. Reorder: config keys, units, reverse proxy. | ||
| 235 | |||
| 236 | ### 3.7 README (S, #27) | ||
| 237 | |||
| 238 | Add: endpoints and what they do, running the binary without Docker with | ||
| 239 | the service files, what `REDIRECTS.md` is for (redirector rules), and a | ||
| 240 | screenshot. | ||
| 241 | |||
| 242 | ## Tier 4: UI | ||
| 243 | |||
| 244 | ### 4.1 Viewport and mobile CSS (S, #28) | ||
| 245 | |||
| 246 | `static/html/head.htm` and `index.htm` use `initial-scale=0.4` and | ||
| 247 | `height=device-height`; `skunky.css` then compensates with | ||
| 248 | `* { font-size: 120% }` in portrait. Use `width=device-width, | ||
| 249 | initial-scale=1` and adjust the portrait rules to match. Check on a phone | ||
| 250 | width before and after. | ||
| 251 | |||
| 252 | ### 4.2 Accessibility (S, #29) | ||
| 253 | |||
| 254 | Listing and avatar images in `DeviationList`, `ParseComments` and | ||
| 255 | `BuildUserPlate` have no `alt`. The post page has no heading element for | ||
| 256 | the title. Add both. | ||
| 257 | |||
| 258 | ### 4.3 Index stylesheet (S, #30) | ||
| 259 | |||
| 260 | `static/html/index.htm` carries an inline stylesheet duplicating layout | ||
| 261 | rules. Move it into `skunky.css`. | ||
| 262 | |||
| 263 | ## Tier 5: identity and reach | ||
| 264 | |||
| 265 | ### 5.1 One canonical forge (S, #31) | ||
| 266 | |||
| 267 | Origin and issues are on gitbay; releases, the image, Dependabot, the | ||
| 268 | instances.json fetch at `app/util.go:64`, the About page "Report an issue" | ||
| 269 | link, the index source link, and the `--add-instance` exit message all | ||
| 270 | point at GitHub. Decide which is canonical. If gitbay: fetch | ||
| 271 | `instances.json` from gitbay, point the links there, keep the GitHub mirror | ||
| 272 | for the image build only. If GitHub stays the public face: say so in the | ||
| 273 | README and leave the links. | ||
| 274 | |||
| 275 | ### 5.2 Instance checker (M, issue #4, #32) | ||
| 276 | |||
| 277 | A scheduled job that fetches each instance's `/api/instance` and marks dead | ||
| 278 | ones in `INSTANCES.md`, or a CI job that fails when one is down. | ||
| 279 | |||
| 280 | ### 5.3 LibRedirect listing (S, #33) | ||
| 281 | |||
| 282 | `REDIRECTS.md` already describes the URL mapping. Check whether LibRedirect | ||
| 283 | lists SkunkyArt with the dead upstream instances and submit the fork and | ||
| 284 | art.krz.sh. This is the cheapest way to get users. | ||
| 285 | |||
| 286 | ### 5.4 Makefile and binary releases (S, issue #5, #34) | ||
| 287 | |||
| 288 | Targets for build with the embed tag and version stamp, test, lint. | ||
| 289 | Publish binaries alongside the image on release tags. | ||
| 290 | |||
| 291 | ### 5.5 Existing issues | ||
| 292 | |||
| 293 | - #1 cleanup: the TODOs at `app/parsers.go:222` and `app/cache.go:3`; the | ||
| 294 | second is 1.1. `sendMedia` in `app/api.go` duplicates `ParseMedia`'s | ||
| 295 | magic string offsets (`[21:]`, `dot+11`); share one function. | ||
| 296 | - #2 search filters: blocked on what devianter exposes; scope after 1.1. | ||
| 297 | - #3 description parsing: `ParseDescription` drops `header-two`, ordered | ||
| 298 | lists and nested styles. Needs fixtures from real descriptions. | ||
| 299 | - #6 emote bug: the `a.Val[8:9] == "e"` and `[37:len-4]` offsets in the | ||
| 300 | HTML branch of `ParseDescription`. Parse the URL instead of slicing. | ||
| 301 | |||
| 302 | ## Stacked MR order | ||
| 303 | |||
| 304 | Each MR branches from the previous one's tip and is merged in order. | ||
| 305 | |||
| 306 | 1. `ci/pipeline` (0.1) | ||
| 307 | 2. `fix/escape-templates` (2.1 + 2.7) | ||
| 308 | 3. `feat/api-cache` (1.1, after its spec is approved) | ||
| 309 | 4. `feat/avatar-cache-headers` (1.2) | ||
| 310 | 5. `feat/robots-ratelimit` (1.3) | ||
| 311 | 6. `feat/fewer-calls` (1.4) | ||
| 312 | 7. `chore/cache-default-on` (1.5) | ||
| 313 | 8. `fix/small-bugs` (2.2, 2.3, 2.4, 2.6, 2.8; one MR, one commit each) | ||
| 314 | 9. `fix/atom-feed` (2.5) | ||
| 315 | 10. `docs/config-and-setup` (3.1, 3.2, 3.3, 3.6) | ||
| 316 | 11. `feat/i18n-coverage` (3.4) | ||
| 317 | 12. `chore/services-readme` (3.5, 3.7) | ||
| 318 | 13. `ui/viewport-a11y` (4.1, 4.2, 4.3) | ||
| 319 | 14. `chore/canonical-forge` (5.1) | ||
| 320 | 15. 5.2 through 5.5 as independent MRs off `main` | ||
| 321 | |||
| 322 | Items 8 through 15 do not depend on the cache stack and can be reordered or | ||
| 323 | interleaved when the cache work stalls on design. | ||