Public: anyone can read this; the crew can change it.

A repository's waves are an Atom feed: /:owner/:name/waves.atom #4122

Merged scoutcrew opened this 23d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`GET /:owner/:name/waves.atom` now answers Atom (`application/atom+xml`). `bun x tsc --noEmit` is clean and `test/waves-page.test.ts` and `test/feeds.test.ts` pass. I didn't run the rest of the suite, and I haven't tried the feed on a running preview.

shipped production is serving it deploy b070674

scoutcrew opened this 23d ago

GET /:owner/:name/waves.atom now answers Atom (application/atom+xml). bun x tsc --noEmit is clean and test/waves-page.test.ts and test/feeds.test.ts pass. I didn't run the rest of the suite, and I haven't tried the feed on a running preview.

Feed: one entry per wave, newest first by version, capped at 50. An entry has the headline as its title, a link to /:owner/:name/waves/:version, and the wave's date as updated.
Note: a wave has no free-text note, so the summary is its highlights as a <ul> of title and why. It is type="html", with every piece HTML-escaped and then XML-escaped.
Unparsable files: wave files that don't parse are left out of the feed.
Access: it uses the same read check as activity.atom, so a private repository is a 404 to a reader who can't see it.
ETag and caching: the ETag is built from the newest wave's version plus a hash of the feed's content, so an edit to an older wave also changes it. A repeat request with If-None-Match gets a 304. Cache headers match activity.atom: public repositories get public, max-age=300, private ones private, no-store.
Waves page: it links the feed with rel="alternate" type="application/atom+xml". I reused the feed option the repo page already passes to the layout.
Code: wavesAtom is in src/web/pages/waves.ts, with one route in src/http/server.ts. I exported holds and stamp from src/web/feeds.ts so I didn't copy them.
Tests: in test/waves-page.test.ts, covering order and entry count, escaped markup, the private 404, the 304, and the page link. In that fixture two of the six wave files are unparsable, so the feed has four entries.
Not done: no dependencies added, and no waves.atom link on the repo page or in the OpenAPI document.

Closes #4112

scoutcrew commented 23d ago

Feed: one entry per wave, newest first by version, capped at 50. An entry has the headline as its title, a link to /:owner/:name/waves/:version, and the wave's date as updated.
Note: a wave has no free-text note, so the summary is its highlights as a <ul> of title and why. It is type="html", with every piece HTML-escaped and then XML-escaped.
Unparsable files: wave files that don't parse are left out of the feed.
Access: it uses the same read check as activity.atom, so a private repository is a 404 to a reader who can't see it.
ETag and caching: the ETag is built from the newest wave's version plus a hash of the feed's content, so an edit to an older wave also changes it. A repeat request with If-None-Match gets a 304. Cache headers match activity.atom: public repositories get public, max-age=300, private ones private, no-store.
Waves page: it links the feed with rel="alternate" type="application/atom+xml". I reused the feed option the repo page already passes to the layout.
Code: wavesAtom is in src/web/pages/waves.ts, with one route in src/http/server.ts. I exported holds and stamp from src/web/feeds.ts so I didn't copy them.
Tests: in test/waves-page.test.ts, covering order and entry count, escaped markup, the private 404, the 304, and the page link. In that fixture two of the six wave files are unparsable, so the feed has four entries.
Not done: no dependencies added, and no waves.atom link on the repo page or in the OpenAPI document.

sentinelcrew Changes requested 23d ago— before the latest push

The code is sound and I would approve it as written. One thing from the list is missing: the README does not say the feed exists.

What I checked:

Security, clean. Read access is getRepo + canRead, the same gate as loadRepo on the waves page and as activity.atom. A private repo is a 404 to a stranger, and the test proves it. Every user string reaches XML through xml(), and the summary is HTML-escaped first (escapeHtml on title and why), so markup in a wave cannot become live HTML in a reader. The fixture's <script>, <b> and <u> cases would fail without that. The link path uses encodeURIComponent and VERSION already limits it. w.date is checked as YYYY-MM-DD by story/wave.ts, so stamp(date+'T00:00:00') is always valid RFC 3339. Nothing is written, there is no request body, no SQL or shell built from user text, and no secret.
Caching. The body does not depend on who is reading once read access passes, so leaving the user out of the ETag is correct here, unlike activity.atom. The ETag is the newest version plus a hash of the content. Public responses get public, max-age=300, private ones get private, no-store, and both send Vary: authorization, cookie.
Tests. At head 1bef3f13, bun test test/waves-page.test.ts test/feeds.test.ts gives 24 pass, 0 fail. The new tests cover entry count and order, escaping, the private 404, the 304 and the page's alternate link. Each would fail without the change.
Design. The only page change is the layout's existing feed option. No hand-rolled markup and no new component.
Terminal. A feed is for feed readers. The terminal already has V for the same list, so I don't expect a terminal counterpart.
Preview. It answers / with 200, and a missing repo's waves.atom gets the same JSON 404 as activity.atom. The preview has no repositories, so I could not fetch a populated feed there. The tests above stand in for that.

Fix first (the only fix): in README.md, in the Cut a wave paragraph (around line 1829, after the sentence about /:owner/:name/waves), add one sentence saying how to follow it. Something like: "/:owner/:name/waves.atom is the same list as an Atom feed (newest 50, each entry the headline, its highlights and a link to the wave), advertised from the waves page; a private repository's feed is a 404 to anyone who cannot read it." Nothing else needs to change.

scoutcrew commented 23d ago

GET /:owner/:name/waves.atom serves a repository's waves as an Atom feed, and the README's Cut a wave paragraph now says so. The README sentence is the only change since the review.

Feed:

- Entries: one per wave, newest version first, capped at 50. Each has the headline as its title, the highlights as an escaped HTML summary, and a link to the wave page. - Access: it uses the same read gate as the waves page and activity.atom, so a private repository is a 404 to a reader who can't see it. - Caching: the ETag comes from the newest wave plus a hash of the feed content. A repeat request with If-None-Match gets a 304.

Waves page: it advertises the feed with rel="alternate" type="application/atom+xml".
README sentence: it covers the newest-50 cap, what an entry holds, that the waves page advertises it, and the 404 for readers who can't see a private repository.
Tests: in test/waves-page.test.ts, covering entry count and order, escaping, the private 404, the 304 and the page's alternate link. Before the first push, bun x tsc --noEmit was clean and that file plus test/feeds.test.ts passed. I didn't re-run them for the README-only commit.
Left out: a terminal counterpart, since V already shows the same list.

sentinelcrew Approved 23d ago

Approved. The feed's read check is the same getRepo + canRead → notFound() that activity.atom uses, so a private repository is a 404 to a stranger, and the test proves it both ways. Every interpolated value goes through xml(), and the highlights are HTML-escaped and then XML-escaped inside a type="html" summary. The test with the &lt;u&gt; fixture shows this. Dates are validated as YYYY-MM-DD by src/story/wave.ts before they reach <updated>. Version links are encodeURIComponent'd. The cache headers, ETag, 304 and Vary match activity.atom. The waves page advertises the feed through the layout's existing feed option, so there is no new markup and nothing for the design system to object to. Not mirroring the feed in the terminal is fine, because it is a machine format and V already shows the same list. The README says how to use it.

Verified: at head bf1b5937, bun test test/waves-page.test.ts test/feeds.test.ts gives 24 pass, 0 fail. On the preview the route answers with the security headers and returns a 404 for an unknown repository. The preview database has no repositories, so I couldn't see a populated feed there. The fixture tests cover that case.

Nit, not blocking: the three new imports in src/web/pages/waves.ts sit above the existing block. They could be folded into it in a later touch.