ROADMAP.md

main
skunky-art/ROADMAP.md rendered · source · history · blame · raw

466 lines · 17288 bytes

  1# Roadmap
  2
  3Findings from a full review on 2026-09-10, ordered for work. Each item is
  4sized to be one gitbay issue and one merge request. Sizes: S is under an
  5hour, M is a few hours, L needs a design pass first.
  6
  7Verified on: local build from `main` at 028903a, live instance
  8art.krz.sh (v1.4.0), devianter v0.3.4. DeviantArt's WAF blocks the
  9reviewing machine's egress IP, so DA-backed pages were checked on the live
 10instance.
 11
 12Existing gitbay issues: #1 cleanup, #2 search filters, #3 description
 13parsing, #4 instance checker, #5 Makefile, #6 emote bug. They are mapped
 14below where they overlap.
 15
 16## Status
 17
 18Everything below except 5.3 shipped in v1.5.0 (2026-09-11), with v1.5.1
 19fixing the release build. Six patch releases followed on 2026-09-11 and
 202026-09-12 from running the public instance; see "After v1.5.0" at the
 21end. Of the original six issues, #1, #3, #4, #5 and #6 are closed by
 22merged work and #2 is closed as not possible for a guest session. The
 23one open issue is #33, the LibRedirect submission, which needs the
 24maintainer's account.
 25
 26## Ordering principle
 27
 28Frontend proxies get blocked upstream. The existing throttle
 29(`app/httpclient.go`) spreads requests out; it does not reduce them. Tier 1
 30reduces them. Tier 0 lands first because everything after it needs CI.
 31Tier 2's escaping fix lands before other template work because it touches
 32every template and builder.
 33
 34## Upstream cost today
 35
 36Nothing from DeviantArt's JSON API is cached. Only wixmp media is cached,
 37and only when `cache.enabled` is on, which defaults to off.
 38
 39| Page | Upstream calls per view |
 40|---|---|
 41| Post | 2 API (deviation, comments) + 1 avatar per commenter |
 42| User about | 2 API (gruser, comments) + avatars |
 43| Gallery, favourites, search, DD | 1 API, plus thumbnails when the media cache is off |
 44| `/api/random` | up to 3 searches |
 45| Atom feed | 1 API per poll, per reader |
 46
 47Avatars come from a.deviantart.net and are fetched fresh on every view.
 48Concurrent requests for the same page each go upstream. No response carries
 49`Cache-Control`. There is no robots.txt and no per-client rate limit.
 50
 51## Tier 0: process
 52
 53### 0.1 CI on merge requests (S, #7)
 54
 55Done in !6.
 56
 57No pipeline runs `go vet`, `go test` or `golangci-lint`. The only workflow
 58is the GitHub release build; gitbay has zero builds. `.golangci.yml` exists
 59but is never run.
 60
 61Fix: add a gitbay build that runs vet, test with `-race`, and golangci-lint
 62on every MR. Check `gitbay build --help` and `ssh git@gitbay.org help build`
 63for the pipeline format. Optionally mirror the same job to
 64`.github/workflows/` so the GitHub mirror shows status too.
 65
 66Verify: an MR with a failing test shows a failed build.
 67
 68## Tier 1: reduce upstream requests
 69
 70### 1.1 Cache DeviantArt API responses (L, #8)
 71
 72Done in !8.
 73
 74Add an in-memory cache in front of every devianter call: DD, search,
 75deviation, gruser, gallery, favourites, comments. Keyed by endpoint plus
 76arguments. Bounded by entry count or bytes. Singleflight so concurrent
 77requests for one key share a single upstream call. Per-endpoint TTLs, in
 78config with defaults on the order of: DD and search a few minutes,
 79deviations longer, comments shorter.
 80
 81This is the item the TODO at `app/cache.go:3` names. It makes feed polling
 82and `/api/random` close to free.
 83
 84Design first: cache interface, key derivation, TTL config keys, eviction,
 85what the hit/miss log looks like. Write the spec to
 86`docs/superpowers/specs/` and review before code.
 87
 88Files: new `app/apicache.go`, call sites in `app/wrapper.go`,
 89`app/api.go`, `app/api_json.go`, config in `app/config.go`, docs in
 90`SETUP.md`.
 91
 92Verify: unit tests for TTL expiry, singleflight, and bounded size. Two
 93sequential requests for one page make one upstream call.
 94
 95### 1.2 Cache avatars and emotes, add Cache-Control (M, #9)
 96
 97Done in !9.
 98
 99`Emojitar` in `app/wrapper.go` fetches from a.deviantart.net or
100e.deviantart.net on every request and never stores the result. Route it
101through the same disk and memory cache path as `DownloadAndSendMedia`.
102
103Add `Cache-Control` headers: long `max-age` with `immutable` on
104`/media/file` (token-signed wixmp URLs do not change), a day on avatars and
105emotes, a day on `/stylesheet` and `/favicon.ico`, a short `max-age` on HTML
106matching the API cache TTL.
107
108Depends on: nothing, but pairs with 1.1.
109
110Verify: second avatar request is served without an upstream fetch; headers
111present in `curl -I` output.
112
113### 1.3 robots.txt and per-client rate limit (S, #10)
114
115Done in !10.
116
117Serve `/robots.txt` disallowing `/search`, `/api`, `/group_user`,
118`/media` and any path with `?p=`. Add a per-client-IP token bucket ahead of
119the upstream throttle so one crawler cannot consume the whole DA budget and
120turn it into latency for everyone else. Honour `X-Forwarded-For` only when
121the request came from a configured trusted proxy.
122
123Files: `app/router.go`, new `app/ratelimit.go`, `app/config.go`,
124`SETUP.md`.
125
126Verify: test that N+1 requests from one address within the window get 429.
127
128### 1.4 Fewer calls per page (M, #11)
129
130Done in !11.
131
132Post view is two API calls because comments are fetched inline. Move
133comments behind a link (`/post/{author}/{name}/comments` or `?comments=1`)
134so the default post view is one call. Same for the user about page.
135
136Rebuild `/api/random` to pick from cached DD or search results (1.1) instead
137of issuing up to three fresh searches per hit.
138
139Depends on: 1.1.
140
141Verify: post view makes exactly one upstream call in a test with a fake
142transport.
143
144### 1.5 Media cache on by default (S, #12)
145
146Done in !12.
147
148A proxying instance with no cache re-fetches every image from wixmp on every
149view. Set `cache.enabled: true` in the built-in defaults in `app/config.go`
150and in `config.example.json`, with a sane `lifetime` and `max-size`. Keep
151`memcache` off. Document the change in `SETUP.md`.
152
153Verify: fresh start with no config writes to the cache directory.
154
155## Tier 2: security and correctness
156
157### 2.1 Escape template output (M, #13)
158
159Done in !7.
160
161`app/util.go` imports `text/template`. Nothing interpolated is escaped: the
162search query in `static/html/search.htm` and `head.htm`, and every DA
163username, title and description written by `DeviationList`,
164`ParseComments`, `BuildUserPlate` and `ParseDescription`. The CSP blocks
165scripts but not markup, inline styles, meta refresh or injected forms; any
166title containing `<` corrupts the page.
167
168Fix: switch to `html/template`; wrap the pre-built HTML fragments in
169`template.HTML`; escape strings in the Go builders with
170`html.EscapeString` and attribute-escape URLs. Add tests that a query and a
171title containing `"><b>` render as text.
172
173Land before other template work.
174
175### 2.2 Restore the user About branch (S, #14)
176
177Done in !13.
178
179`app/wrapper.go:35` has `else if false`, inherited from upstream commit
180048bb47. Registration date, interests, social links and bio never render for
181users. Find out why it was disabled (likely a devianter struct change),
182restore the branch, add a test with a fixture.
183
184### 2.3 Group search pagination (S, #15)
185
186Done in !13.
187
188`app/wrapper.go:274` increments the page and requests offset `10*page`, so
189page two starts at result 20 and results 10 to 19 are never shown. The nav
190bar also shows the incremented number. Use `10*(page-1)` and do not mutate
191`s.Page` before `NavBase`.
192
193### 2.4 Emojitar writes a body after 404 (S, #16)
194
195Done in !9.
196
197`app/wrapper.go:344` lacks a `return` after `ReturnHTTPError(404)`.
198
199### 2.5 Valid Atom feed (S, #17)
200
201Done in !15.
202
203`DeviationList` in `app/parsers.go` emits no feed-level `<id>` or
204`<updated>`, bare integer entry ids, RFC 1123 `<published>` instead of RFC
2053339, and `media:thumbinal`. Verified on the live feed. Fix all five and add
206a test that parses the output with an Atom library or checks the required
207elements.
208
209### 2.6 `-c` bounds check (S, #18)
210
211Done in !13.
212
213`app/cli.go:29` checks `len(a) >= 2` instead of `n+1 < len(a)`;
214`skunkyart -x -c` panics.
215
216### 2.7 Sanitize the 502 page (S, #19)
217
218Done in !7.
219
220`Error` in `app/util.go` writes the upstream error, including the full
221CloudFront block page, into an `<h3>` unescaped. Truncate to one line and
222escape. Folds into 2.1 if done together.
223
224### 2.8 Parse templates once (S, #20)
225
226Done in !13.
227
228`ExecuteTemplate` calls `ParseFS` on every request. Parse at startup;
229supply the per-request `T` function through the data struct or a per-request
230`Funcs` clone. Template errors then fail at boot instead of as 500s.
231
232## Tier 3: config, docs, i18n
233
234### 3.1 Config-less start and default alignment (S, #21)
235
236Done in !16.
237
238`ExecuteConfig` exits if `config.json` is missing even though defaults
239exist. Start with defaults when no `-c` is given and the default file is
240absent. Align the built-in `nsfw: true` with the example's `false`, or
241document why they differ.
242
243### 3.2 Cache documentation (S, #22)
244
245Done in !16.
246
247`SETUP.md`: `update-interval` is in seconds (the example scans every 5s);
248the `d` unit works but is unlisted; `y` is 360 days; exceeding `max-size`
249deletes the whole cache directory; `lifetime: null` in the example.
250
251### 3.3 API and search type docs (S, #23)
252
253Done in !16.
254
255`API.md` says `t` is text search; devianter defines it as tag. The
256"Folders" option in `static/html/gruser.htm` maps to `f`, which is
257favourites. Fix the doc and rename or remove the option.
258
259### 3.4 i18n coverage (M, #24)
260
261Done in !17.
262
263Go-built HTML hardcodes English: comment headers, "In reply to",
264pagination, folder and content headings, "No results", "[ TEXT ]".
265`gruser.htm` section headings and the index blurb are untranslated. Every
266template declares `lang="en"`. `Languages()` in `app/i18n.go` is unused.
267Move the strings into the catalogues, set `lang` from the resolved
268language, and either use or remove `Languages()`.
269
270### 3.5 systemd unit (S, #25)
271
272Done in !18.
273
274`services/skunkyart.example.service` uses `Directory=` (not a valid key),
275placeholder paths, and says it was never tested. Write a working unit with
276`WorkingDirectory`, `User`, `DynamicUser` or a dedicated user,
277`NoNewPrivileges`, and `Restart=on-failure`. Test it once on a Linux host.
278
279### 3.6 SETUP.md structure (S, #26)
280
281Done in !16.
282
283The nginx section sits between config keys; `theme` and `language` come
284after it. Reorder: config keys, units, reverse proxy.
285
286### 3.7 README (S, #27)
287
288Done in !18.
289
290Add: endpoints and what they do, running the binary without Docker with
291the service files, what `REDIRECTS.md` is for (redirector rules), and a
292screenshot.
293
294## Tier 4: UI
295
296### 4.1 Viewport and mobile CSS (S, #28)
297
298Done in !19.
299
300`static/html/head.htm` and `index.htm` use `initial-scale=0.4` and
301`height=device-height`; `skunky.css` then compensates with
302`* { font-size: 120% }` in portrait. Use `width=device-width,
303initial-scale=1` and adjust the portrait rules to match. Check on a phone
304width before and after.
305
306### 4.2 Accessibility (S, #29)
307
308Done in !19.
309
310Listing and avatar images in `DeviationList`, `ParseComments` and
311`BuildUserPlate` have no `alt`. The post page has no heading element for
312the title. Add both.
313
314### 4.3 Index stylesheet (S, #30)
315
316Done in !19.
317
318`static/html/index.htm` carries an inline stylesheet duplicating layout
319rules. Move it into `skunky.css`.
320
321## Tier 5: identity and reach
322
323### 5.1 One canonical forge (S, #31)
324
325Done in !20.
326
327Origin and issues are on gitbay; releases, the image, Dependabot, the
328instances.json fetch at `app/util.go:64`, the About page "Report an issue"
329link, the index source link, and the `--add-instance` exit message all
330point at GitHub. Decide which is canonical. If gitbay: fetch
331`instances.json` from gitbay, point the links there, keep the GitHub mirror
332for the image build only. If GitHub stays the public face: say so in the
333README and leave the links.
334
335### 5.2 Instance checker (M, issue #4, #32)
336
337Done in !21.
338
339A scheduled job that fetches each instance's `/api/instance` and marks dead
340ones in `INSTANCES.md`, or a CI job that fails when one is down.
341
342### 5.3 LibRedirect listing (S, #33)
343
344Open. The two upstream pull requests are written up on #33.
345
346`REDIRECTS.md` already describes the URL mapping. Check whether LibRedirect
347lists SkunkyArt with the dead upstream instances and submit the fork and
348art.krz.sh. This is the cheapest way to get users.
349
350### 5.4 Makefile and binary releases (S, issue #5, #34)
351
352Done in !22.
353
354Targets for build with the embed tag and version stamp, test, lint.
355Publish binaries alongside the image on release tags.
356
357### 5.5 Existing issues
358
359- #1 cleanup: the TODOs at `app/parsers.go:222` and `app/cache.go:3`; the
360  second is 1.1. `sendMedia` in `app/api.go` duplicates `ParseMedia`'s
361  magic string offsets (`[21:]`, `dot+11`); share one function.
362- #2 search filters: blocked on what devianter exposes; scope after 1.1.
363- #3 description parsing: `ParseDescription` drops `header-two`, ordered
364  lists and nested styles. Needs fixtures from real descriptions.
365- #6 emote bug: the `a.Val[8:9] == "e"` and `[37:len-4]` offsets in the
366  HTML branch of `ParseDescription`. Parse the URL instead of slicing.
367
368## Merge record
369
370Merged into main in this order on 2026-09-11, each stacked on the one
371before: !6 (0.1), !7 (2.1, 2.7), !8 (1.1), !9 (1.2, 2.4), !10 (1.3),
372!11 (1.4), !12 (1.5), !13 (2.2, 2.3, 2.6, 2.8), !15 (2.5), !16 (3.1,
3733.2, 3.3, 3.6), !17 (3.4), !18 (3.5, 3.7), !19 (4.1, 4.2, 4.3), !20
374(5.1), !21 (5.2), !22 (5.4). Then !23 (release string), !24 (Go 1.26 in
375the image and binaries builds) and !25 (no VCS stamping) for the
376release itself.
377
378Two things the stack taught: lint on macOS never compiles the Linux-only
379files, so run `GOOS=linux golangci-lint run` before pushing; and `go get`
380can raise the go directive in go.mod, so check the Dockerfile and
381workflow images still match it.
382
383## After v1.5.0
384
385Everything here came out of deploying v1.5 to art.krz.sh and watching it.
386
387### v1.5.1 (!24, !25)
388
389go.mod had moved to Go 1.26 when x/sync came in while the Dockerfile and
390the binaries job still used 1.25, so the v1.5.0 tag built no image. Both
391now match go.mod. The tarball job also failed on VCS stamping inside the
392build container, fixed with `-buildvcs=false`.
393
394### v1.5.2 (!27, !28)
395
396The instance's VPN exit was banned by DeviantArt's WAF and every
397DeviantArt-backed page 502d until someone restarted the stack.
398
399- API cache entries past their TTL are kept for `api-cache.stale`
400  (default 1h) and served when upstream fails or answers 403 or 429. A
401  block starts a one minute backoff during which the instance stops
402  asking.
403- `/api/random` answered 401 with proxying on: the media signing token
404  was inside the path. Fixed, and `Download` sits behind a seam.
405- The session bootstrap runs before the listener opens, so the first
406  seconds after a restart no longer 502.
407
408Operationally: the instance moved off the VPN to its own address, which
409was clean at the time, and the container got a real log driver (it had
410`none`, which hid every error line).
411
412### v1.5.3 (!29)
413
414The direct address was banned within an hour. `upstream.min-interval-ms`
415and `upstream.max-concurrent` replace the source constants; the
416instance runs at 1000 ms and one in flight. The ban lifted after about
417seventy minutes. The instance also raised `api-cache.ttl` to 30
418minutes and `stale` to a day, and a Cloudflare managed-challenge rule
419now covers non-browser clients on search, post and profile paths.
420
421### v1.5.4 (!30, !31)
422
423Cache rotation trims the oldest files down to `max-size` instead of
424emptying the directory, and never touches the directory itself, which
425on a bind mount logged `unlinkat` and `mkdir` errors every pass (#36).
426The per-platform stat files went with it. x/net bumped (#35).
427
428### v1.5.5 (!32)
429
430A crawler walking post pages at 70 a minute produced 733 upstream
431timeouts in ten minutes while the address stayed unbanned: each queued
432request still took its interval turn after its client had timed out.
433The throttle now honours the request context, and sheds a request that
434would queue longer than 20 seconds with a 503 and `Retry-After`. The
435instance's `rate-limit` dropped to 20 per minute, burst 10.
436
437### v1.5.6 (!33, !34)
438
439The real cause of #3: DeviantArt's editor stores descriptions and
440comments as a document tree (`{"version":1,"document":...}`), not
441Draft.js blocks, so every current description rendered empty. A new
442renderer covers the node set seen in 92 live payloads: paragraphs,
443headings, lists, quotes, code, breaks, rules, text marks, emotes (#6),
444embedded artworks, GIF embeds and mentions. Media and post URLs are
445parsed instead of sliced by offset (#1), and old HTML emotes map by
446image name.
447
448### Closed without code
449
450#2 search filters: DeviantArt's guest search ignores every `order`
451value the site itself uses, and the deviations endpoint redirects
452guests. Nothing to expose. #4 and #5 were covered by the instance
453checker and the Makefile.
454
455### Lessons
456
457- DeviantArt bans an address on volume, not on whether it is a VPN.
458  Pacing, caching and shedding at the instance are what keep it clean;
459  rotating exits only buys an hour.
460- A restart empties the in-memory API cache, so a ban right after a
461  deploy has nothing stale to serve. Deploy when the instance is quiet.
462- Log driver `none` is a trap. Every incident here was diagnosed from
463  lines that driver would have dropped.
464- Fetch real payloads from a host DeviantArt accepts before touching a
465  parser; the format had changed under the old one.
466