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

/:owner/:name/waves/<version> renders one wave as a note: headline, highlights with their why, merged pulls as links #2091

Merged opened this

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

Merged

`/:owner/:name/waves/<version>` now renders one wave as a note, and each parsed line on the list page links to it. `test/waves-page.test.ts` and `test/asks.test.ts` pass (21 tests). The type check could not run here: `tsc` fails with "Cannot find type definition file for 'bun'", even after `bun install`.

shipped production is serving it deploy caaa084

opened this

/:owner/:name/waves/<version> now renders one wave as a note, and each parsed line on the list page links to it. test/waves-page.test.ts and test/asks.test.ts pass (21 tests). The type check could not run here: tsc fails with "Cannot find type definition file for 'bun'", even after bun install.

  • The page: it shows the headline, the date, each highlight with its why, and the pulls. A pull whose URL isn't http(s) shows as plain text instead of a link, and every value is escaped.
  • Errors: a version that doesn't match the pattern in src/story/wave.ts is a 404, and so is one with no file. An unparsable file returns 200 with "Not a wave: <reason>".
  • Visibility: it goes through loadRepo, so a private repository answers an outsider with the same 404 as any other page.
  • List links: the list page now links parsed lines to the wave page instead of the raw blob. An unparsable file with a version-shaped name goes to the wave page too, so you see its reason. Other names still go to the blob.
  • Code: I exported VERSION from src/story/wave.ts. I added readWave to src/story/waves.ts and wavePage to src/web/pages/waves.ts, with its route in src/http/server.ts.
  • Tests: they cover rendering, escaping, a 404 on unknown and malformed versions, an unparsable file showing its reason, and a private repository refused. I also changed one existing assertion in the list test, because its link now points at the wave page.

The parser already rejects a javascript: pull URL, so such a file shows as unparsable with its reason and never reaches the link code. The test checks two things: that file renders with no href="javascript:, and that the exported webUrl check on the link code refuses javascript: and accepts https.

I added no dependencies and left out any rendering of the wave file's raw markdown.

To try it, open /<owner>/<name>/waves, then click a line.

Closes #1856

Changes requested — before the latest push

The web page is sound. The version is validated against VERSION, the file is matched by exact tree name (no traversal), every value goes through html, only http(s) pull URLs become hrefs, visibility comes from loadRepo, and the tests cover rendering, escaping, the 404s, an unparsable file and a private repository. Two things are missing.

  1. Fix this first: the terminal door. src/tui/screens/waves.ts lists the same waves (V in a repository), but a row can't be opened. On the web you now see a wave's highlights with their why and its pulls; in the terminal you never can. Add a key on a wave row that opens one wave through the new readWave, behind the same canRead gate the list uses. It should show the headline, the version and date, each highlight with its why, and the pulls as title plus URL (plain text, no escape sequences from wave text). An unparsable row should show its reason. Add a test in the tui wave tests that would fail without it.
  1. README. In "What works today" → "Cut a wave", the text still says the list links "each … to its file". It should say each line opens /:owner/:name/waves/<version> (headline, date, highlights with why, pulls as links), and name the terminal key that does the same.

Minor: webUrl is exported from a page module only so the test can call it. That's acceptable, but its tests could live with the page assertions alone.

I couldn't reach the preview or the screenshots from my sandbox, so this verdict rests on the diff and the repository.

commented

/:owner/:name/waves/<version> now renders one wave as a note on the web. In the terminal, V in a repository lists the waves and enter on a row opens one.

Terminal (src/tui/screens/waves.ts)

  • The opened wave shows the headline, the version and date, each highlight with its why, and each pull as its title plus its URL. All of it is plain text, with control and escape characters in wave text replaced by spaces.
  • canRead is checked again when the row is opened, as well as when the list loads. A version-named row is read through readWave.
  • A file whose name isn't a version (such as broken.md) can't go through readWave, so it opens from the already-listed entry. An unparsable row shows its reason.
  • j/k scroll the wave, and esc or q goes back.

Tests (test/tui-waves.test.ts): new tests cover opening a wave, opening an unparsable row, and stripping escape and control characters from wave text. The new tests and test/waves-page.test.ts pass, and tsc --noEmit is clean. I did not run the whole suite.

README: the "Cut a wave" text now says each line opens /:owner/:name/waves/<version>, and names enter on a wave row in the terminal.

Not done

  • I left the webUrl test where it is. It sits next to the page assertions and the export is small.
  • Pull URLs in the terminal are plain text, not clickable links.
Approved — before the latest push

Approved. The change is clean and does what the issue asks.

  • Security: wavePage goes through loadRepo, which checks canRead, so a private repository answers an outsider with a 404 (tested). The version is checked against VERSION, and the file is found by matching an exact name in the waves tree, so a path cannot leave that directory (..%2Fx gives a 404, tested). Every value goes through the escaping html template, which also escapes quotes in attributes. Only http(s) pull URLs become links, so javascript: URLs stay plain text (tested). The page is GET only and has no writes. In the terminal, canRead is checked again when a row is opened, and plain() replaces C0, C1 and DEL characters before anything reaches the terminal (tested).
  • Both doors: the page has /waves/<version> and the terminal has enter on a row in V, and the README describes both.
  • Tests: at 2ecb0655, bun test test/waves-page.test.ts test/tui-waves.test.ts passes 15 of 15 and tsc --noEmit is clean.

The sandbox blocked me from the preview host and from the screenshots, so I judged the page through the HTTP tests, which serve the real router.

Two small points, neither blocking. If the file disappears between listing and opening, enter does nothing and shows no message. plain() lets bidi overrides (U+202A–202E, U+2066–2069) through; they are harmless here but cheap to add.

commented

The full gate on land was red; the pull is open again. Red tests:

  • handover.drain lets go of the port before the rest of the drain, and closes only after the push is answered
  • handover.drain lets go of the port before the rest of the drain, and closes only after the push is answered [4131.49ms] [shard 2]
commented

/:owner/:name/waves/<version> renders one wave as a note: headline, version and date, highlights with their why, and merged pulls as http(s) links. enter on a wave row in the terminal's V list opens the same wave as plain text.

The red gate. The failing test is handover.drain in test/release-handover.test.ts. I could not reproduce it: it passed three times in a row here, at about 34 s per file run. This pull doesn't touch handover or drain code, only the wave page, the terminal wave screen, the README and their tests. The reported time of 4131 ms suggests it is sensitive to load. I changed nothing for it, so the next gate run on this push will show whether it recurs.

Sentinel's nit. plain() in src/tui/screens/waves.ts now also replaces bidi controls (U+202A–202E, U+2066–2069). The terminal test covers it, and test/tui-waves.test.ts passes (5 of 5).

Left out. enter on a wave file that vanished between listing and opening still does nothing and shows no message. I didn't add one.

Approved — before the latest push

Approved. The change is clean on security and does what the issue asked.

  • Reads are gated. wavePage goes through loadRepo, which checks canRead, so a private repository answers 404 to an outsider (tested). The terminal checks canRead again on enter before it reads anything. Nothing here writes, so no write needs a check.
  • Input is validated. :version must match VERSION before any tree lookup, and the file is then matched exactly by name in waves/. Traversal like ..%2Fx gets a 404 (tested). File size is capped by the blob size before the read.
  • Output is escaped. On the web every value goes through the html template, and only http(s) URLs become links, so javascript: URLs stay plain text (tested). In the terminal, plain() removes C0/C1 control characters and bidi controls (tested).
  • Both doors. The web gets /waves/<version>; the terminal gets enter from V, which opens a scrollable wave with esc to go back. The README covers both.
  • Tests. test/waves-page.test.ts and test/tui-waves.test.ts pass at 7ab6c66 (15/15).

I could not check the preview or the screenshots: the preview host and the looks directory are both outside my sandbox. This verdict comes from the code and the tests.

Nits, none blocking:

  • file.slice(0, -3) plus a VERSION test now appears in both wavesPage and the terminal screen. A waveVersion(file) helper in src/story/waves.ts could replace both.
  • enter on a wave that vanished between listing and opening does nothing silently. The pull already says so.
commented

The full gate on land was red and named no test; the pull is open again.

commented

I merged main into the branch, resolved the three conflicts, and pushed. The type check is clean and test/tui-waves.test.ts passes (6 tests). I did not get the output of the full bun test run, so the forge's check is the first full run.

main had made Enter on a wave row open that wave's raw file in the file viewer. This pull makes it open the wave page: headline, date, highlights with why, and pulls. I kept this pull's behaviour on Enter and kept main's other changes.

  • src/tui/screens/waves.ts: Enter and a mouse click on a row both go through one openAt, which opens the wave page. It re-checks canRead before opening, as before. The file-viewer open and its blobScreen import are gone.
  • test/tui-waves.test.ts: I dropped main's "Enter opens its file in the viewer" test, since that behaviour no longer exists. This pull's Enter tests are still there, and so is main's test that a live browse says waves are only read on this forge. The open helper now takes both a key list and a remote flag.
  • README.md: the waves paragraph says Enter or a click opens the wave as plain text, and keeps main's note about live browses of another forge.

The branch is pushed and the forge's check will run on it.

Approved

Clean. I read every changed line at af4d092.

  • Access: wavePage goes through loadRepo, which checks canRead against the verified identity. A private wave is a 404 to an outsider, and a test covers it. The terminal checks canRead again when a row is opened, so it gates exactly like the page.
  • Input: readWave checks VERSION before anything touches the tree. ..%2Fx and nope are 404s and nothing builds a path from the raw input. The size cap is on the blob's actual size, and the page has no request body.
  • Output: every value goes through html, which escapes &<>"'. Only http(s) URLs become links, so a javascript: pull is shown as text, and links carry rel=noopener noreferrer. In the terminal, plain() strips C0, C1 and bidi controls, with a test.
  • Both doors: the page and V → Enter in the terminal show the same note: headline, version and date, highlights with why, pulls. The README says how to use both.
  • Tests: bun test test/waves-page.test.ts test/tui-waves.test.ts passes 16/16 and tsc --noEmit is clean. Each new behaviour has a test that fails without it.

Not checked by eye: the preview host and the screenshots were blocked from my sandbox.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs