From dde010a2fd0e0abc61046ac3b32ae7f1d3d87441 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 20:53:51 +0900 Subject: [PATCH 01/21] docs: design for document download feature --- .../2026-06-26-document-download-design.md | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 docs/specs/2026-06-26-document-download-design.md diff --git a/docs/specs/2026-06-26-document-download-design.md b/docs/specs/2026-06-26-document-download-design.md new file mode 100644 index 0000000..80859a4 --- /dev/null +++ b/docs/specs/2026-06-26-document-download-design.md @@ -0,0 +1,127 @@ +# Document Download — Design + +**Date:** 2026-06-26 +**Status:** Approved (brainstorming) — ready for implementation plan + +## Summary + +Let users download the **currently-open document** — the exact, original bytes +on disk — via a button in the tab bar. The button acts on the active document +and works for every document type reefdoc can open: markdown, allium, code/text, +and binary office formats (PDF, DOCX, XLSX, PPTX). + +The browser performs the save natively. The existing `/api/file` endpoint gains +an opt-in `?download=1` mode that sets a `Content-Disposition: attachment` +header; the frontend adds a right-aligned download button in the tab-bar row +that navigates to that URL for the active document. + +## Goals + +- A single, clear download control tied to the **active** document. +- Works for **all** open document types, with no per-type special-casing. +- The downloaded file is the **original bytes on disk** with its **original + filename** — not a re-rendered or converted form. +- Native browser download (correct filename, no client-side memory overhead for + large files), without leaving the current page. +- Zero behavioral change to the existing inline `/api/file` behavior when + `?download` is absent. + +## Non-Goals (YAGNI) + +- Downloading files from the tree without first opening them (active-document + only). +- Exporting rendered/converted output (e.g. PDF-from-markdown, HTML export). +- Per-tab download icons. +- Bulk / multi-file download. +- Streaming optimization for large files (the current `os.ReadFile` path is + reused unchanged). + +## Server Change + +In `handleFile` (`internal/server/server.go`): + +- When the request carries `?download=1`, add the header: + `Content-Disposition: attachment; filename=""`. +- `` is the base name of the requested path (e.g. `report.pdf` from + `docs/report.pdf`). +- The filename is sanitized for use as a header value: the basename is quoted, + and any `"` characters or control characters are stripped/escaped so the + header cannot be broken or injected. +- All existing behavior is preserved: path safety via `SafeJoin`, symlink + resolution back inside the root, `Content-Type` selection via `contentType`, + and the `os.ReadFile` + write body. `Content-Disposition: attachment` only + instructs the browser to save rather than display; the correct `Content-Type` + (e.g. `application/pdf`) is still sent. +- When `?download` is absent, no `Content-Disposition` header is sent and + behavior is byte-for-byte identical to today (inline rendering preserved). + +Path traversal is already prevented by the existing guards; the sanitization +here is purely about producing a valid, safe `Content-Disposition` value. + +## Frontend Change + +### HTML (`web/index.html`) + +Wrap the tab bar and a new download button in a header row so the button is +cleanly right-aligned without disturbing tab layout: + +```html +
+
+ +
+``` + +The button is `hidden` by default and only shown when a document is active. + +### Behavior (`web/app.js`) + +- A `refreshDownloadButton()` helper toggles the button's visibility based on + whether `store.active` is set. It is called wherever the active document + changes: `open()`, `activate()`, `doClose()`, and session restore. +- On click, the button triggers a native download of the active document by + navigating to: + `/api/file?path=&download=1` + Because the server responds with `Content-Disposition: attachment`, the + browser saves the file without navigating away from the page. Implementation + uses a hidden `` element (or `window.location.assign`) pointed at + that URL. +- The button operates only on `store.active`. When no document is open it is + hidden, so there is no ambiguous state. + +### Styling (`web/app.css`) + +- Style `#tabbar-row` as a flex row: `#tabbar` takes the available width, + `#download-btn` sits at the right edge. +- Style `#download-btn` consistently with existing controls (e.g. + `#theme-toggle`, `.close`): hover state and theme-aware colors. + +## Edge Cases + +- **No document open:** button is `hidden`; nothing to download. +- **Active document deleted on disk:** the download request hits `/api/file`, + which returns 404. No special handling beyond the existing 404 — the user + already sees the "file no longer exists" message in the content pane. This is + an acceptable edge race. +- **Filenames with special characters:** handled by sanitizing/quoting the + `Content-Disposition` filename value (see Server Change). +- **Large files:** reuse the existing `os.ReadFile` + write path; + `Content-Disposition` adds no memory overhead. Optimizing the in-memory read + is out of scope. + +## Testing + +### Go (`internal/server/server_test.go`) + +- `?download=1` produces `Content-Disposition: attachment; filename="..."` with + the correct basename. +- The response body bytes are unchanged versus the inline response. +- A filename containing a `"` is safely escaped/stripped in the header. +- Without `?download`, no `Content-Disposition` header is sent (inline behavior + preserved). + +### JS (`web/app.test.js`) + +- `refreshDownloadButton()` toggles the button's visibility correctly based on + active state. +- Clicking the button builds the correct `&download=1` URL for the active path. From d4cd8a23000a5db4dfb55a8db71f6e46c3622cd1 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 20:58:30 +0900 Subject: [PATCH 02/21] docs: implementation plan for document download --- docs/plans/2026-06-26-document-download.md | 412 +++++++++++++++++++++ 1 file changed, 412 insertions(+) create mode 100644 docs/plans/2026-06-26-document-download.md diff --git a/docs/plans/2026-06-26-document-download.md b/docs/plans/2026-06-26-document-download.md new file mode 100644 index 0000000..29bb399 --- /dev/null +++ b/docs/plans/2026-06-26-document-download.md @@ -0,0 +1,412 @@ +# Document Download Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a tab-bar button that downloads the currently-open document (original bytes, original filename) for every document type reefdoc can open. + +**Architecture:** The Go `/api/file` handler gains an opt-in `?download=1` mode that sets a sanitized `Content-Disposition: attachment` header; all existing path-safety, symlink, content-type, and byte-reading behavior is unchanged. The vanilla-JS frontend adds a right-aligned download button in a new tab-bar row that navigates to that URL for the active document, letting the browser save natively. + +**Tech Stack:** Go (`net/http`, `httptest`), vanilla JS (ES modules), `node:test` for JS unit tests. + +**Spec:** `docs/specs/2026-06-26-document-download-design.md` + +--- + +## File Structure + +- `internal/server/server.go` — add `?download=1` handling + a `dispositionFilename` sanitizer helper in `handleFile`. +- `internal/server/server_test.go` — Go tests for the header, body integrity, sanitization, and inline-preservation. +- `web/index.html` — wrap `#tabbar` in `#tabbar-row` and add `#download-btn`. +- `web/app.js` — add a pure `downloadUrl(path)` builder, a `refreshDownloadButton()` visibility helper, the click handler, and calls into the active-document lifecycle. +- `web/app.test.js` — JS unit tests for `downloadUrl` (duplicated pure function, matching the `resolvePath` pattern). +- `web/app.css` — style `#tabbar-row` and `#download-btn`. + +--- + +## Task 1: Server — `?download=1` sets a sanitized `Content-Disposition` header + +**Files:** +- Modify: `internal/server/server.go` (the `handleFile` function, lines 57-78, and add a helper near `contentType` at the file end) +- Test: `internal/server/server_test.go` + +- [ ] **Step 1: Write the failing tests** + +Add these tests to the end of `internal/server/server_test.go`: + +```go +func TestHandleFile_DownloadSetsContentDisposition(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "report.pdf"), []byte("PDFDATA"), 0o644); err != nil { + t.Fatal(err) + } + s := New(root, NewBroker(), fstest.MapFS{}, nil) + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/file?path=report.pdf&download=1", nil)) + if rec.Code != 200 { + t.Fatalf("status %d", rec.Code) + } + got := rec.Header().Get("Content-Disposition") + want := `attachment; filename="report.pdf"` + if got != want { + t.Fatalf("Content-Disposition %q, want %q", got, want) + } + if ct := rec.Header().Get("Content-Type"); ct != "application/pdf" { + t.Fatalf("Content-Type %q, want application/pdf", ct) + } + if rec.Body.String() != "PDFDATA" { + t.Fatalf("body %q", rec.Body.String()) + } +} + +func TestHandleFile_DownloadUsesBasename(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "docs"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "docs", "intro.md"), []byte("# hi"), 0o644); err != nil { + t.Fatal(err) + } + s := New(root, NewBroker(), fstest.MapFS{}, nil) + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/file?path=docs/intro.md&download=1", nil)) + if got := rec.Header().Get("Content-Disposition"); got != `attachment; filename="intro.md"` { + t.Fatalf("Content-Disposition %q, want basename intro.md", got) + } +} + +func TestHandleFile_DownloadSanitizesFilename(t *testing.T) { + root := t.TempDir() + // A filename containing a double-quote and a control character. If the OS + // allows the file to be created, the header must not contain a raw " or + // control byte that could break or inject the header. + name := "a\"b\x01.md" + if err := os.WriteFile(filepath.Join(root, name), []byte("x"), 0o644); err != nil { + t.Skipf("OS rejected test filename: %v", err) + } + s := New(root, NewBroker(), fstest.MapFS{}, nil) + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/file?path="+name+"&download=1", nil)) + cd := rec.Header().Get("Content-Disposition") + if strings.ContainsAny(cd[len("attachment; filename=\""):], "\x01") { + t.Fatalf("control char leaked into header: %q", cd) + } + // The quoted filename value must not contain a raw double-quote. + inner := strings.TrimSuffix(strings.TrimPrefix(cd, `attachment; filename="`), `"`) + if strings.Contains(inner, `"`) { + t.Fatalf("raw double-quote leaked into filename value: %q", cd) + } +} + +func TestHandleFile_NoDownloadParamOmitsContentDisposition(t *testing.T) { + s, _ := newTestServer(t) + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/file?path=a.md", nil)) + if cd := rec.Header().Get("Content-Disposition"); cd != "" { + t.Fatalf("Content-Disposition should be absent for inline requests, got %q", cd) + } +} +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `go test ./internal/server/ -run TestHandleFile_Download -v` +Expected: FAIL — `Content-Disposition` header is empty (feature not implemented yet). + +- [ ] **Step 3: Implement the `?download=1` handling and sanitizer** + +In `internal/server/server.go`, modify `handleFile` so that after the existing `w.Header().Set("Content-Type", contentType(abs))` line (line 76) and before writing the body, it conditionally sets the disposition header. The full updated tail of `handleFile`: + +```go + w.Header().Set("Content-Type", contentType(abs)) + if r.URL.Query().Get("download") == "1" { + name := dispositionFilename(filepath.Base(abs)) + w.Header().Set("Content-Disposition", `attachment; filename="`+name+`"`) + } + _, _ = w.Write(data) +} +``` + +Then add this helper at the end of `internal/server/server.go` (next to `contentType`): + +```go +// dispositionFilename returns a value safe to embed in a quoted +// Content-Disposition filename. It drops control characters and escapes +// backslashes and double-quotes so the header cannot be broken or injected. +// Path safety is enforced elsewhere; this only produces a valid header value. +func dispositionFilename(base string) string { + var b strings.Builder + for _, r := range base { + switch { + case r < 0x20 || r == 0x7f: + // drop control characters + case r == '"' || r == '\\': + b.WriteByte('\\') + b.WriteRune(r) + default: + b.WriteRune(r) + } + } + return b.String() +} +``` + +`filepath` and `strings` are already imported in this file (lines 9-10), so no import changes are needed. + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `go test ./internal/server/ -run TestHandleFile -v` +Expected: PASS for all `TestHandleFile_*` tests (new and existing). + +- [ ] **Step 5: Run the full Go suite to confirm no regressions** + +Run: `go test ./...` +Expected: PASS (all packages). + +- [ ] **Step 6: Commit** + +```bash +git add internal/server/server.go internal/server/server_test.go +git commit -m "feat(server): add ?download=1 mode to /api/file" +``` + +--- + +## Task 2: Frontend — pure `downloadUrl` builder + unit tests + +**Files:** +- Modify: `web/app.js` (add the `downloadUrl` function near `resolvePath`, ~line 556) +- Test: `web/app.test.js` + +The download URL builder is a pure function. Because `app.js` has DOM side-effects at module scope and cannot be imported headlessly, the test duplicates the pure function exactly as `app.test.js` already does for `resolvePath`. + +- [ ] **Step 1: Write the failing tests** + +Add to the end of `web/app.test.js`: + +```js +// downloadUrl lives in app.js which has DOM side-effects at module scope and +// cannot be imported headlessly. Duplicate the pure function here (matches the +// resolvePath pattern above). +function downloadUrl(path) { + return '/api/file?path=' + encodeURIComponent(path) + '&download=1'; +} + +test('downloadUrl: builds the download URL for a simple path', () => { + assert.equal(downloadUrl('a.md'), '/api/file?path=a.md&download=1'); +}); + +test('downloadUrl: encodes spaces and slashes in nested paths', () => { + assert.equal( + downloadUrl('my docs/report.pdf'), + '/api/file?path=my%20docs%2Freport.pdf&download=1' + ); +}); +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `npm test` +Expected: FAIL — the new `downloadUrl` tests are present but, if you have not yet added the duplicated function, the test file errors. (If you added the duplicated helper in the test file in Step 1, the tests will PASS immediately — that is acceptable for a pure-duplication test; the real verification is that `app.js` ships the same function in Step 3.) + +- [ ] **Step 3: Add the real `downloadUrl` to `web/app.js`** + +In `web/app.js`, add this function immediately above `resolvePath` (currently at line 556): + +```js +// Build the URL that downloads a document's original bytes. The server responds +// with Content-Disposition: attachment, so the browser saves rather than renders. +function downloadUrl(path) { + return '/api/file?path=' + encodeURIComponent(path) + '&download=1'; +} +``` + +- [ ] **Step 4: Run the JS tests to confirm they pass** + +Run: `npm test` +Expected: PASS (all tests, including the two new `downloadUrl` cases). + +- [ ] **Step 5: Commit** + +```bash +git add web/app.js web/app.test.js +git commit -m "feat(app): add downloadUrl builder" +``` + +--- + +## Task 3: Frontend — download button markup and styling + +**Files:** +- Modify: `web/index.html` (lines 48-51, the `#main` block) +- Modify: `web/app.css` (append new rules) + +- [ ] **Step 1: Wrap the tab bar and add the button in `web/index.html`** + +Replace the current `#main` block (lines 48-51): + +```html +
+
+

Select a file from the tree.

+
+``` + +with: + +```html +
+
+
+ +
+

Select a file from the tree.

+
+``` + +- [ ] **Step 2: Add styling to `web/app.css`** + +Append to `web/app.css`. These rules reuse the project's existing theme variables (`--bg`, `--fg`, `--border`, defined in `:root` and `body[data-theme="dark"]` at the top of `web/app.css`) and mirror the existing `#theme-toggle` control. Note the existing `#tabbar` rule (line 23) already sets `display:flex; flex:0 0 auto; ...`; wrapping it in `#tabbar-row` does not require changing that rule — the row is a new flex parent and `#tabbar` becomes a flex child within it. + +```css +#tabbar-row { display:flex; align-items:stretch; border-bottom:1px solid var(--border); } +#tabbar-row #tabbar { flex:1 1 auto; min-width:0; border-bottom:none; } +#download-btn { flex:0 0 auto; align-self:center; margin:0 8px; padding:4px 8px; + font-size:15px; line-height:1; cursor:pointer; background:var(--bg); + color:var(--fg); border:1px solid var(--border); border-radius:4px; } +#download-btn:hover { background:var(--border); } +``` + +Note: the existing `#tabbar` rule sets its own `border-bottom`; moving that border onto `#tabbar-row` (and overriding `#tabbar` with `border-bottom:none`) keeps a single continuous bottom border across the full row including the button area. + +- [ ] **Step 3: Verify the page renders with the button hidden** + +Run: `go build -o reefdoc . && ./reefdoc ./docs` +Then open `http://127.0.0.1:8080` in a browser. Expected: the layout is unchanged and no download button is visible (it is `hidden` because no document is open yet). Stop the server (Ctrl-C) when done. + +- [ ] **Step 4: Commit** + +```bash +git add web/index.html web/app.css +git commit -m "feat(app): add download button markup and styling" +``` + +--- + +## Task 4: Frontend — wire the button into the active-document lifecycle + +**Files:** +- Modify: `web/app.js` + +- [ ] **Step 1: Add the button element reference** + +In `web/app.js`, alongside the existing element references (after line 157 where `tocEl` is defined), add: + +```js +const downloadBtn = el('download-btn'); +``` + +- [ ] **Step 2: Add the `refreshDownloadButton` helper and click handler** + +In `web/app.js`, immediately after the `downloadUrl` function added in Task 2, add: + +```js +// Show the download button only when a document is active; clicking it triggers +// a native browser download of the active document's original bytes. +function refreshDownloadButton() { + downloadBtn.hidden = !store.active; +} + +downloadBtn.addEventListener('click', () => { + if (!store.active) return; + const a = document.createElement('a'); + a.href = downloadUrl(store.active); + a.download = ''; // hint the browser to save; server filename still wins + document.body.appendChild(a); + a.click(); + a.remove(); +}); +``` + +- [ ] **Step 3: Call `refreshDownloadButton` wherever the active document changes** + +Add a `refreshDownloadButton();` call in each of these four places in `web/app.js`: + +In `open()` (after `renderTabs();`, around line 416): +```js + renderTabs(); + refreshDownloadButton(); +``` + +In `activate()` (after `renderTabs();`, around line 427): +```js + renderTabs(); + refreshDownloadButton(); +``` + +In `doClose()` — at the end of the function (after the `if (store.active) { ... } else { ... }` block, around line 445), add: +```js + refreshDownloadButton(); +``` + +In the boot/session-restore block, after the session's `renderTabs();` call (around line 729) and also in the `else if (bootPath)` path the button is handled by `open()`. To cover the no-session/no-bootPath case explicitly, add a single call after the entire boot restore block (after line 735): +```js +refreshDownloadButton(); +``` + +- [ ] **Step 4: Manually verify the button behavior** + +Run: `go build -o reefdoc . && ./reefdoc ./docs` +Open `http://127.0.0.1:8080`. Expected behavior: +1. With no document open, the button is hidden. +2. Open a markdown file from the tree — the download button appears in the tab-bar row. +3. Click it — the browser downloads the original `.md` file with its correct name. +4. Open a PDF (if one exists under `./docs`, or create one) — clicking downloads the original PDF. +5. Close all tabs — the button disappears. +Stop the server (Ctrl-C) when done. + +- [ ] **Step 5: Run all tests to confirm no regressions** + +Run: `go test ./... && npm test` +Expected: PASS for both suites. + +- [ ] **Step 6: Commit** + +```bash +git add web/app.js +git commit -m "feat(app): wire download button to active document lifecycle" +``` + +--- + +## Task 5: Documentation + +**Files:** +- Modify: `README.md` (Features list, lines 36-47) +- Modify: `CHANGELOG.md` + +- [ ] **Step 1: Add the feature to the README features list** + +In `README.md`, add a bullet to the Features list (after line 46, the "Hyperlinks between documents" bullet or in a sensible position): + +```markdown +- Download the open document (original file) with one click +``` + +- [ ] **Step 2: Add a CHANGELOG entry** + +Open `CHANGELOG.md`, read its existing format, and add an entry matching that format describing: "Download the currently-open document via a tab-bar button (works for all document types; serves the original bytes via `/api/file?download=1`)." Follow the existing heading/version convention exactly — do not invent a new format. + +- [ ] **Step 3: Commit** + +```bash +git add README.md CHANGELOG.md +git commit -m "docs: announce document download feature" +``` + +--- + +## Self-Review Notes + +- **Spec coverage:** Server `?download=1` + sanitized `Content-Disposition` (Task 1) covers the Server Change section. `downloadUrl` (Task 2), button markup (Task 3), and lifecycle wiring (Task 4) cover the Frontend Change section. Edge cases (no doc open → hidden button; deleted file → existing 404; special filenames → `dispositionFilename`; large files → unchanged read path) are all addressed. Testing section maps to Task 1 (Go) and Task 2 (JS). +- **Placeholder scan:** No TBDs. The only deferral is the CHANGELOG format, which is intentionally "match the existing convention" since the format is project-defined. +- **Type/name consistency:** `downloadUrl`, `refreshDownloadButton`, `#download-btn`, `#tabbar-row`, and `dispositionFilename` are used consistently across all tasks. From f3d5e94931055621cbc9530063d12a0eabfa3a1f Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 21:05:38 +0900 Subject: [PATCH 03/21] feat(server): add ?download=1 mode to /api/file --- internal/server/server.go | 24 ++++++++++++ internal/server/server_test.go | 71 ++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/internal/server/server.go b/internal/server/server.go index 9327314..cedebe1 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -74,6 +74,10 @@ func (s *Server) handleFile(w http.ResponseWriter, r *http.Request) { return } w.Header().Set("Content-Type", contentType(abs)) + if r.URL.Query().Get("download") == "1" { + name := dispositionFilename(filepath.Base(abs)) + w.Header().Set("Content-Disposition", `attachment; filename="`+name+`"`) + } _, _ = w.Write(data) } @@ -142,3 +146,23 @@ func contentType(path string) string { return "text/plain; charset=utf-8" } } + +// dispositionFilename returns a value safe to embed in a quoted +// Content-Disposition filename. It drops control characters and escapes +// backslashes and double-quotes so the header cannot be broken or injected. +// Path safety is enforced elsewhere; this only produces a valid header value. +func dispositionFilename(base string) string { + var b strings.Builder + for _, r := range base { + switch { + case r < 0x20 || r == 0x7f: + // drop control characters + case r == '"' || r == '\\': + b.WriteByte('\\') + b.WriteRune(r) + default: + b.WriteRune(r) + } + } + return b.String() +} diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 81d8f18..d3d2d8d 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "strings" @@ -254,3 +255,73 @@ func TestHandleFile_BinaryBytesIntact(t *testing.T) { t.Fatalf("body bytes %v, want %v", rec.Body.Bytes(), raw) } } + +func TestHandleFile_DownloadSetsContentDisposition(t *testing.T) { + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "report.pdf"), []byte("PDFDATA"), 0o644); err != nil { + t.Fatal(err) + } + s := New(root, NewBroker(), fstest.MapFS{}, nil) + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/file?path=report.pdf&download=1", nil)) + if rec.Code != 200 { + t.Fatalf("status %d", rec.Code) + } + got := rec.Header().Get("Content-Disposition") + want := `attachment; filename="report.pdf"` + if got != want { + t.Fatalf("Content-Disposition %q, want %q", got, want) + } + if ct := rec.Header().Get("Content-Type"); ct != "application/pdf" { + t.Fatalf("Content-Type %q, want application/pdf", ct) + } + if rec.Body.String() != "PDFDATA" { + t.Fatalf("body %q", rec.Body.String()) + } +} + +func TestHandleFile_DownloadUsesBasename(t *testing.T) { + root := t.TempDir() + if err := os.MkdirAll(filepath.Join(root, "docs"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "docs", "intro.md"), []byte("# hi"), 0o644); err != nil { + t.Fatal(err) + } + s := New(root, NewBroker(), fstest.MapFS{}, nil) + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/file?path=docs/intro.md&download=1", nil)) + if got := rec.Header().Get("Content-Disposition"); got != `attachment; filename="intro.md"` { + t.Fatalf("Content-Disposition %q, want basename intro.md", got) + } +} + +func TestHandleFile_DownloadSanitizesFilename(t *testing.T) { + root := t.TempDir() + name := "a\"b\x01.md" + if err := os.WriteFile(filepath.Join(root, name), []byte("x"), 0o644); err != nil { + t.Skipf("OS rejected test filename: %v", err) + } + s := New(root, NewBroker(), fstest.MapFS{}, nil) + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, + "/api/file?path="+url.PathEscape(name)+"&download=1", nil)) + cd := rec.Header().Get("Content-Disposition") + if strings.ContainsAny(cd[len("attachment; filename=\""):], "\x01") { + t.Fatalf("control char leaked into header: %q", cd) + } + inner := strings.TrimSuffix(strings.TrimPrefix(cd, `attachment; filename="`), `"`) + // A raw, unescaped double-quote would break the header. An escaped \" is fine. + if strings.Contains(strings.ReplaceAll(inner, `\"`, ""), `"`) { + t.Fatalf("raw double-quote leaked into filename value: %q", cd) + } +} + +func TestHandleFile_NoDownloadParamOmitsContentDisposition(t *testing.T) { + s, _ := newTestServer(t) + rec := httptest.NewRecorder() + s.Handler().ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/file?path=a.md", nil)) + if cd := rec.Header().Get("Content-Disposition"); cd != "" { + t.Fatalf("Content-Disposition should be absent for inline requests, got %q", cd) + } +} From 08a5ac4921031a712dbd0ac334d6b6032944a2f5 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 21:10:54 +0900 Subject: [PATCH 04/21] feat(app): add downloadUrl builder --- web/app.js | 6 ++++++ web/app.test.js | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/web/app.js b/web/app.js index 1edf348..dfeb838 100644 --- a/web/app.js +++ b/web/app.js @@ -553,6 +553,12 @@ function restoreScroll(tab) { contentEl.scrollTop = (tab.scrollRatio || 0) * contentEl.scrollHeight; } +// Build the URL that downloads a document's original bytes. The server responds +// with Content-Disposition: attachment, so the browser saves rather than renders. +function downloadUrl(path) { + return '/api/file?path=' + encodeURIComponent(path) + '&download=1'; +} + function resolvePath(base, href) { const dir = base.slice(0, base.lastIndexOf('/') + 1); return decodeURIComponent(new URL(href, 'file:///' + dir).pathname.slice(1)); diff --git a/web/app.test.js b/web/app.test.js index 69057af..ef731d5 100644 --- a/web/app.test.js +++ b/web/app.test.js @@ -35,3 +35,21 @@ test('resolvePath: decodes percent-encoded characters in path', () => { test('resolvePath: bare relative href without leading ./', () => { assert.equal(resolvePath('docs/guide/intro.md', 'sibling.md'), 'docs/guide/sibling.md'); }); + +// downloadUrl lives in app.js which has DOM side-effects at module scope and +// cannot be imported headlessly. Duplicate the pure function here (matches the +// resolvePath pattern above). +function downloadUrl(path) { + return '/api/file?path=' + encodeURIComponent(path) + '&download=1'; +} + +test('downloadUrl: builds the download URL for a simple path', () => { + assert.equal(downloadUrl('a.md'), '/api/file?path=a.md&download=1'); +}); + +test('downloadUrl: encodes spaces and slashes in nested paths', () => { + assert.equal( + downloadUrl('my docs/report.pdf'), + '/api/file?path=my%20docs%2Freport.pdf&download=1' + ); +}); From 1ab792e394d403c38ee304ef0ac37511dd067996 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 21:13:45 +0900 Subject: [PATCH 05/21] test(app): pin downloadUrl encoding of query-delimiter chars --- web/app.test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/web/app.test.js b/web/app.test.js index ef731d5..d0ff165 100644 --- a/web/app.test.js +++ b/web/app.test.js @@ -53,3 +53,10 @@ test('downloadUrl: encodes spaces and slashes in nested paths', () => { '/api/file?path=my%20docs%2Freport.pdf&download=1' ); }); + +test('downloadUrl: encodes query-delimiter characters in the path', () => { + assert.equal( + downloadUrl('a&b=c.md'), + '/api/file?path=a%26b%3Dc.md&download=1' + ); +}); From ac68962f81af3761a6f9567f70c45f81305f91fe Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 21:15:23 +0900 Subject: [PATCH 06/21] feat(app): add download button markup and styling --- web/app.css | 7 +++++++ web/index.html | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/web/app.css b/web/app.css index 6df170e..40a682c 100644 --- a/web/app.css +++ b/web/app.css @@ -123,3 +123,10 @@ body[data-theme="dark"] .allium-block--contract { border-left-color:#a97ee8; } border: 1px solid var(--border); padding: 2px 6px; } + +#tabbar-row { display:flex; align-items:stretch; border-bottom:1px solid var(--border); } +#tabbar-row #tabbar { flex:1 1 auto; min-width:0; border-bottom:none; } +#download-btn { flex:0 0 auto; align-self:center; margin:0 8px; padding:4px 8px; + font-size:15px; line-height:1; cursor:pointer; background:var(--bg); + color:var(--fg); border:1px solid var(--border); border-radius:4px; } +#download-btn:hover { background:var(--border); } diff --git a/web/index.html b/web/index.html index 2c7cad5..2ef2e5e 100644 --- a/web/index.html +++ b/web/index.html @@ -46,7 +46,10 @@
-
+
+
+ +

Select a file from the tree.

From 9e4315ef3676237b510d010b4504de5396128b01 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 21:19:13 +0900 Subject: [PATCH 07/21] a11y(app): add aria-label to download button --- web/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/index.html b/web/index.html index 2ef2e5e..3228989 100644 --- a/web/index.html +++ b/web/index.html @@ -48,7 +48,7 @@
- +

Select a file from the tree.

From 4e8b0f6eb5cce4cbb5956ac50b65779c8b147b06 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 21:27:38 +0900 Subject: [PATCH 08/21] feat(app): wire download button to active document lifecycle --- web/app.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/web/app.js b/web/app.js index dfeb838..d75cbe0 100644 --- a/web/app.js +++ b/web/app.js @@ -155,6 +155,7 @@ const treeEl = el('tree'); const tabbarEl = el('tabbar'); const contentEl = el('content'); const tocEl = el('toc'); +const downloadBtn = el('download-btn'); function currentTheme() { return document.body.getAttribute('data-theme'); @@ -414,6 +415,7 @@ async function open(path, title) { openTab(store, path, title); // idempotent: adds if new, always activates syncUrl(path, { push: true }); renderTabs(); + refreshDownloadButton(); syncWatches(); saveSession(); await show(path); @@ -425,6 +427,7 @@ function activate(path) { if (tab) tab.updated = false; syncUrl(path, { push: true }); renderTabs(); + refreshDownloadButton(); saveSession(); show(path); } @@ -442,6 +445,7 @@ function doClose(path) { contentEl.innerHTML = '

Select a file from the tree.

'; tocEl.innerHTML = ''; } + refreshDownloadButton(); } // ---- Render a document ---- @@ -559,6 +563,22 @@ function downloadUrl(path) { return '/api/file?path=' + encodeURIComponent(path) + '&download=1'; } +// Show the download button only when a document is active; clicking it triggers +// a native browser download of the active document's original bytes. +function refreshDownloadButton() { + downloadBtn.hidden = !store.active; +} + +downloadBtn.addEventListener('click', () => { + if (!store.active) return; + const a = document.createElement('a'); + a.href = downloadUrl(store.active); + a.download = ''; // hint the browser to save; server filename still wins + document.body.appendChild(a); + a.click(); + a.remove(); +}); + function resolvePath(base, href) { const dir = base.slice(0, base.lastIndexOf('/') + 1); return decodeURIComponent(new URL(href, 'file:///' + dir).pathname.slice(1)); @@ -739,6 +759,7 @@ if (session) { } else if (bootPath) { open(bootPath, titleFor(bootPath)); } +refreshDownloadButton(); // Back/forward navigation: open whatever target the URL now points at. window.addEventListener('hashchange', () => { @@ -750,6 +771,7 @@ window.addEventListener('hashchange', () => { // Navigated back to a bare URL: clear the view. store.active = null; renderTabs(); + refreshDownloadButton(); saveSession(); contentEl.innerHTML = '

Select a file from the tree.

'; tocEl.innerHTML = ''; From d00786970e0a65d94a73997098b285cc14e86477 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 21:40:51 +0900 Subject: [PATCH 09/21] docs: announce document download feature --- CHANGELOG.md | 8 ++++++++ README.md | 1 + 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 471e127..c618aab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to reefdoc are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.10.0] - 2026-06-26 + +### Added +- Download the currently-open document with one click via a button in the tab + bar. Works for every document type reefdoc opens (markdown, Allium, code, + PDF, DOCX, XLSX, PPTX); the server streams the original file unchanged via a + download mode on the file API. + ## [0.9.0] - 2026-06-25 ### Added diff --git a/README.md b/README.md index 2b5097e..28f3c57 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,7 @@ go build -o reefdoc . && ./reefdoc ./docs - GitHub-flavored markdown, code syntax highlighting, and mermaid diagrams - [Allium](https://allium.dev) spec files (`.allium`) rendered as formatted cards - Preview PDF, DOCX, XLSX, and PPTX files in the browser (rendered client-side) +- Download the open document (its original file) with one click - Auto table of contents from document headings - Dark / light theme (mermaid follows the theme) - Live reload: edit a file in any editor and the open tab updates From 7000a18ec49f5e7e926145cd35bd44677d0c6543 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 21:43:44 +0900 Subject: [PATCH 10/21] docs: tighten changelog wording for document download --- CHANGELOG.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c618aab..4ec0de6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,8 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Download the currently-open document with one click via a button in the tab bar. Works for every document type reefdoc opens (markdown, Allium, code, - PDF, DOCX, XLSX, PPTX); the server streams the original file unchanged via a - download mode on the file API. + PDF, DOCX, XLSX, PPTX), and you get the original file back unchanged. ## [0.9.0] - 2026-06-25 From 1501943f581fc23ba986ca77db0356a95152d7b7 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 21:47:27 +0900 Subject: [PATCH 11/21] docs: note JS test scope and filename* simplification --- docs/specs/2026-06-26-document-download-design.md | 13 ++++++++++--- internal/server/server.go | 5 +++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/docs/specs/2026-06-26-document-download-design.md b/docs/specs/2026-06-26-document-download-design.md index 80859a4..0ded907 100644 --- a/docs/specs/2026-06-26-document-download-design.md +++ b/docs/specs/2026-06-26-document-download-design.md @@ -122,6 +122,13 @@ The button is `hidden` by default and only shown when a document is active. ### JS (`web/app.test.js`) -- `refreshDownloadButton()` toggles the button's visibility correctly based on - active state. -- Clicking the button builds the correct `&download=1` URL for the active path. +- `downloadUrl(path)` builds the correct `&download=1` URL for the active path, + including correct percent-encoding of spaces, slashes, and query-delimiter + characters. + +Note: `refreshDownloadButton()` and the click handler live in `web/app.js`, +which has module-scope DOM side effects and cannot be imported headlessly. Per +the project's established convention (see the `resolvePath` tests), only pure +functions are unit-tested; the trivial one-line visibility toggle +(`downloadBtn.hidden = !store.active`) is verified by inspection rather than a +headless test. diff --git a/internal/server/server.go b/internal/server/server.go index cedebe1..c65aeb8 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -151,6 +151,11 @@ func contentType(path string) string { // Content-Disposition filename. It drops control characters and escapes // backslashes and double-quotes so the header cannot be broken or injected. // Path safety is enforced elsewhere; this only produces a valid header value. +// +// Non-ASCII filenames are passed through as raw UTF-8 in the quoted filename +// parameter rather than using the RFC 6266 extended (filename*) form. All +// current browsers accept and decode raw UTF-8 here, so this is a deliberate +// simplification, not an oversight. func dispositionFilename(base string) string { var b strings.Builder for _, r := range base { From 6f9fc712cf4df6165881b6348ce8d3b7fd219fe7 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Fri, 26 Jun 2026 22:12:08 +0900 Subject: [PATCH 12/21] test(server): cover download mode in E2E HTTP test --- internal/server/e2e_test.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/internal/server/e2e_test.go b/internal/server/e2e_test.go index 2509da6..3a805db 100644 --- a/internal/server/e2e_test.go +++ b/internal/server/e2e_test.go @@ -21,15 +21,19 @@ func TestE2E_EndpointsOverHTTP(t *testing.T) { defer srv.Close() cases := []struct { - path string - wantStatus int - wantSub string + path string + wantStatus int + wantSub string + wantDisposition string // expected Content-Disposition; "" means must be absent }{ - {"/", 200, "app"}, - {"/api/tree", 200, "doc.md"}, - {"/api/file?path=doc.md", 200, "# Title"}, - {"/api/file?path=missing.md", 404, ""}, - {"/api/file?path=../escape", 400, ""}, + {"/", 200, "app", ""}, + {"/api/tree", 200, "doc.md", ""}, + {"/api/file?path=doc.md", 200, "# Title", ""}, + {"/api/file?path=missing.md", 404, "", ""}, + {"/api/file?path=../escape", 400, "", ""}, + // Download mode: the same file API, over real HTTP, must add the + // attachment header so the browser saves rather than renders. + {"/api/file?path=doc.md&download=1", 200, "# Title", `attachment; filename="doc.md"`}, } for _, c := range cases { resp, err := http.Get(srv.URL + c.path) @@ -44,5 +48,8 @@ func TestE2E_EndpointsOverHTTP(t *testing.T) { if c.wantSub != "" && !strings.Contains(string(body), c.wantSub) { t.Errorf("%s: body %q missing %q", c.path, body, c.wantSub) } + if got := resp.Header.Get("Content-Disposition"); got != c.wantDisposition { + t.Errorf("%s: Content-Disposition %q want %q", c.path, got, c.wantDisposition) + } } } From c316643af15d6af0299b78be9f6833d8ad7c7a21 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Sat, 27 Jun 2026 00:05:01 +0900 Subject: [PATCH 13/21] docs: design for browser E2E test of document download --- docs/specs/2026-06-26-download-e2e-design.md | 162 +++++++++++++++++++ 1 file changed, 162 insertions(+) create mode 100644 docs/specs/2026-06-26-download-e2e-design.md diff --git a/docs/specs/2026-06-26-download-e2e-design.md b/docs/specs/2026-06-26-download-e2e-design.md new file mode 100644 index 0000000..1e58922 --- /dev/null +++ b/docs/specs/2026-06-26-download-e2e-design.md @@ -0,0 +1,162 @@ +# Browser E2E Test for Document Download — Design + +**Date:** 2026-06-26 +**Status:** Approved (brainstorming) — ready for implementation plan + +## Summary + +Add a Playwright browser end-to-end test that drives the **real `reefdoc` +binary** and verifies the document-download feature exactly as a user +experiences it: the download button's visibility lifecycle, the click → native +browser download, the correct suggested filename, and byte-for-byte fidelity of +the saved file (text and binary). + +This closes the one remaining gap in the download feature's test coverage. The +Go handler tests (`internal/server/server_test.go`) and the HTTP-level E2E +(`internal/server/e2e_test.go`) verify the server's `?download=1` behavior, but +the browser side — the `#download-btn` show/hide wiring and the temporary-anchor +click → download — is currently only verified by inspection. This test exercises +that path through a real browser against the real binary. + +## Goals + +- Verify, through a real browser against the real binary, that: + - the download button is hidden when no document is open; + - it appears when a document is opened; + - clicking it downloads the open document with the correct filename; + - the downloaded bytes equal the original file on disk (text and binary); + - the button hides again after the last tab is closed. +- Run in CI automatically as an isolated job, without slowing the existing fast + `go` and `web` jobs. +- Keep the shipped binary and its runtime zero-added-dependency: Playwright is a + dev/test dependency only. + +## Non-Goals (YAGNI) + +- Multi-browser matrix — Chromium only. +- Visual / screenshot regression. +- Making the `e2e` CI job a required status check by default (a separate repo + settings decision). +- Replacing the existing Go handler tests or HTTP-level E2E — this complements + them. +- A stubbed/standalone asset server — the test drives the real binary for full + fidelity. + +## Test Harness + +A new test file `web/e2e/download.e2e.js`, run by Playwright (not `node --test`, +which lacks browser fixtures). + +- **Boot the real binary.** In `beforeAll`: run `go build -o reefdoc .`, then + spawn `./reefdoc -addr 127.0.0.1: ` as a child process, where + `` is a fixture directory seeded with known files. Playwright connects + to `http://127.0.0.1:`. This serves the real embedded assets and the + real `/api/file?download=1` path. +- **Free-port selection.** Pick an ephemeral free port at runtime (bind a + throwaway listener to `:0`, read the assigned port, release it) to avoid the + stale-server port collisions seen during development. +- **Server-ready wait.** Poll `GET /` until it returns 200 (with a timeout) + before any Playwright navigation, so the test never races server startup. +- **Cleanup.** `afterAll` always kills the child process and removes the temp + dir, even on test failure, so CI does not leak a server. +- **Fixture content.** The temp dir is seeded at runtime (keeping the repo + clean) with a text file `doc.md` (known contents) and a small binary file + `sample.pdf` (known bytes). + +## Test Cases + +A single spec file covering the full lifecycle: + +1. **Button hidden when no document is open.** Load `/` fresh with `localStorage` + cleared (so reefdoc's session-restore does not auto-open a tab). Assert + `#download-btn` is hidden. +2. **Button appears after opening a document.** Click `doc.md` in the file tree. + Assert `#download-btn` becomes visible. +3. **Download — filename + contents (text).** Set up + `page.waitForEvent('download')` before clicking `#download-btn`. Assert + `download.suggestedFilename() === 'doc.md'`; `download.saveAs()`; assert + the saved bytes equal the original `doc.md` on disk. +4. **Download fidelity (binary).** Open `sample.pdf`, click `#download-btn`, + assert filename `sample.pdf`, and that the saved bytes exactly equal the + original PDF bytes. +5. **Button hides after closing the last tab.** Close the open tab(s) via the + tab `×` control. Assert `#download-btn` is hidden again. + +**Selectors** use the stable markup already present: `#download-btn`, +`#tree .tree-item`, `.tab .close`. Assertions target user-visible outcomes +(visibility, the actual downloaded file), not internals. + +## Dependencies, Config & Layout + +**npm (`package.json`):** +- Add `@playwright/test` to `devDependencies`. +- Add script `"e2e": "playwright test"`. +- Playwright's Chromium browser is installed in CI via + `npx playwright install --with-deps chromium`. + +**`playwright.config.js`** (repo root): +- `testDir` → `web/e2e`. +- Chromium project only. +- Sensible timeout; artifacts/output dir configured. +- No `webServer` block — the test manages the Go binary lifecycle itself, since + it must `go build` first. + +**Keeping the `web` job clean:** the existing `web` job runs `node --test`, which +globs `*.test.js`. The E2E file is `*.e2e.js` under `web/e2e/`, so `node --test` +does not pick it up. (Verify the glob exclusion during implementation.) + +**New CI job `e2e` (`.github/workflows/ci.yml`):** +```yaml +e2e: + name: E2E (Playwright + real binary) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: { go-version: '1.23', cache: true } + - uses: actions/setup-node@v6 + with: { node-version: '22' } + - run: npm install + - run: npx playwright install --with-deps chromium + - run: npm run e2e +``` +Runs in parallel with `go` and `web`. The test builds reefdoc itself, so no +separate Go build step is needed beyond the toolchain. + +**Layout:** +- `web/e2e/download.e2e.js` — the test +- `playwright.config.js` — config +- `package.json` — devDep + `e2e` script +- `.github/workflows/ci.yml` — new `e2e` job +- `.gitignore` — Playwright artifacts (`test-results/`, `playwright-report/`) + and the `/reefdoc` test binary if not already ignored + +## Edge Cases & Reliability + +- **Free-port selection** avoids stale-server collisions. +- **Server-ready polling** before navigation avoids startup races. +- **`localStorage` cleared on load** so case 1 is deterministic and unaffected by + session/tab persistence. +- **`waitForEvent('download')` set up before the click** to avoid a download + race. +- **`afterAll` always kills the child binary**, even on failure, so CI does not + leak a server. + +## Risks & Mitigations + +- **CI cost/slowness:** the browser install adds time; isolated in its own + parallel job, single browser (Chromium only). +- **Dependency weight vs. project ethos:** Playwright is a heavy dev dependency + in an otherwise minimal repo. Accepted deliberately for true download-event + fidelity. It is a dev/test dependency only — the shipped binary and runtime + stay zero-added-dependency, so the "self-contained binary" promise is intact. +- **`go build` from the JS test:** the test shells out to the Go toolchain. The + `e2e` CI job installs Go; locally it requires Go on PATH (noted in the test). + +## Testing + +This *is* a test. Its own verification: the `e2e` job passing in CI, and a local +`npm run e2e` passing (requires Go + Playwright's Chromium installed). A +deliberate manual sanity step during implementation: temporarily break the +feature (e.g. remove the `refreshDownloadButton()` call) and confirm the E2E +fails, proving the test actually guards the behavior. From 85203498e2b5bb1c66aa007ea39b556123cf0abd Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Sat, 27 Jun 2026 07:01:04 +0900 Subject: [PATCH 14/21] docs: implementation plan for download browser E2E test --- docs/plans/2026-06-26-download-e2e.md | 427 ++++++++++++++++++++++++++ 1 file changed, 427 insertions(+) create mode 100644 docs/plans/2026-06-26-download-e2e.md diff --git a/docs/plans/2026-06-26-download-e2e.md b/docs/plans/2026-06-26-download-e2e.md new file mode 100644 index 0000000..394f3ce --- /dev/null +++ b/docs/plans/2026-06-26-download-e2e.md @@ -0,0 +1,427 @@ +# Browser E2E Test for Document Download — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a Playwright browser E2E test that drives the real `reefdoc` binary and verifies the document-download feature end-to-end (button visibility lifecycle, click → download, filename, and saved-file content fidelity for text and binary). + +**Architecture:** A self-contained Playwright spec (`web/e2e/download.e2e.js`) builds `reefdoc` with `go build`, spawns it on a runtime-selected free port against a temp fixture dir, drives a real Chromium via Playwright, and asserts user-visible outcomes including the actual downloaded bytes. A new isolated CI job `e2e` runs it; the existing fast `go`/`web` jobs are untouched. + +**Tech Stack:** Playwright (`@playwright/test`, Chromium only), Node 22, Go 1.23. + +**Spec:** `docs/specs/2026-06-26-download-e2e-design.md` + +--- + +## File Structure + +- `playwright.config.js` (repo root) — Playwright config: `testDir: web/e2e`, Chromium-only project, timeout, artifacts dir. +- `web/e2e/download.e2e.js` — the E2E spec: harness (build + spawn binary, free port, ready-poll, cleanup) + 5 test cases. +- `package.json` — add `@playwright/test` devDependency + `"e2e"` script. +- `.github/workflows/ci.yml` — new `e2e` job. +- `.gitignore` — Playwright artifacts (`/test-results`, `/playwright-report`). + +Note on `package-lock.json`: it is gitignored in this repo, and the existing `web` CI job runs `npm install` (not `npm ci`). The new `e2e` job follows the same `npm install` pattern — do not add `npm ci` or commit a lockfile. + +--- + +## Task 1: Playwright dependency, config, and gitignore + +**Files:** +- Modify: `package.json` +- Create: `playwright.config.js` +- Modify: `.gitignore` + +- [ ] **Step 1: Add the Playwright devDependency and e2e script to `package.json`** + +The current file is: +```json +{ + "name": "reefdoc-web-tests", + "private": true, + "type": "module", + "scripts": { "test": "node --test" }, + "devDependencies": { + "highlight.js": "11.9.0", + "markdown-it": "14.1.0", + "markdown-it-task-lists": "2.1.1" + } +} +``` +Change it to (adds the `e2e` script and the `@playwright/test` devDependency; keep `test` exactly as-is so the unit job is unchanged): +```json +{ + "name": "reefdoc-web-tests", + "private": true, + "type": "module", + "scripts": { + "test": "node --test", + "e2e": "playwright test" + }, + "devDependencies": { + "@playwright/test": "1.48.2", + "highlight.js": "11.9.0", + "markdown-it": "14.1.0", + "markdown-it-task-lists": "2.1.1" + } +} +``` + +- [ ] **Step 2: Install the dependency and the Chromium browser** + +Run: +```bash +npm install +npx playwright install chromium +``` +Expected: `@playwright/test` appears under `node_modules/`, and `npx playwright --version` prints a version. (CI uses `--with-deps`; locally `chromium` alone is fine.) + +- [ ] **Step 3: Create `playwright.config.js` at the repo root** + +```js +import { defineConfig, devices } from '@playwright/test'; + +export default defineConfig({ + testDir: './web/e2e', + testMatch: '**/*.e2e.js', + fullyParallel: false, + forbidOnly: !!process.env.CI, + retries: process.env.CI ? 1 : 0, + workers: 1, + timeout: 30_000, + reporter: process.env.CI ? 'list' : 'line', + outputDir: './test-results', + projects: [ + { name: 'chromium', use: { ...devices['Desktop Chrome'] } }, + ], +}); +``` + +- [ ] **Step 4: Add Playwright artifacts to `.gitignore`** + +Append to `.gitignore`: +``` +# Playwright E2E artifacts +/test-results +/playwright-report +``` + +- [ ] **Step 5: Verify Playwright is wired up (no tests yet)** + +Run: `npx playwright test --list` +Expected: exits cleanly reporting "Total: 0 tests in 0 files" (no spec exists yet) — confirms the config loads without error. + +- [ ] **Step 6: Commit** + +```bash +git add package.json playwright.config.js .gitignore +git commit -m "build(e2e): add Playwright (chromium) config and scripts" +``` + +--- + +## Task 2: E2E harness — build, spawn, free port, ready-poll, cleanup + +**Files:** +- Create: `web/e2e/download.e2e.js` + +This task builds only the harness (`beforeAll`/`afterAll` + a trivial smoke test that the server is reachable). Test cases come in Task 3. Writing the harness first lets us prove the build/spawn/teardown plumbing in isolation. + +- [ ] **Step 1: Write the harness with one smoke test** + +Create `web/e2e/download.e2e.js`: +```js +import { test, expect } from '@playwright/test'; +import { spawn, execFileSync } from 'node:child_process'; +import { createServer } from 'node:net'; +import { mkdtempSync, writeFileSync, rmSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// Repo root is two levels up from web/e2e/. +const repoRoot = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); + +// Known fixture content, asserted byte-for-byte after download. +const DOC_MD = '# Title\n\nHello from reefdoc E2E.\n'; +// Minimal valid-enough PDF bytes (content fidelity is what we assert, not +// renderability). Includes a non-ASCII/control-ish byte to prove binary safety. +const PDF_BYTES = Buffer.from([0x25, 0x50, 0x44, 0x46, 0x2d, 0x31, 0x2e, 0x34, + 0x0a, 0x00, 0xff, 0xfe, 0x80, 0x0a, 0x25, 0x25, 0x45, 0x4f, 0x46, 0x0a]); + +// Pick a free TCP port by binding :0, reading the assigned port, releasing it. +function freePort() { + return new Promise((resolve, reject) => { + const srv = createServer(); + srv.unref(); + srv.on('error', reject); + srv.listen(0, '127.0.0.1', () => { + const { port } = srv.address(); + srv.close(() => resolve(port)); + }); + }); +} + +// Poll GET / until 200 or timeout. +async function waitForServer(base, timeoutMs = 10_000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + try { + const res = await fetch(base + '/'); + if (res.ok) return; + } catch { + // not up yet + } + await new Promise((r) => setTimeout(r, 100)); + } + throw new Error('reefdoc server did not become ready at ' + base); +} + +let proc; +let tmp; +let base; + +test.beforeAll(async () => { + // Build the real binary (exercises go:embed assets and the real server). + execFileSync('go', ['build', '-o', 'reefdoc', '.'], { + cwd: repoRoot, + stdio: 'inherit', + }); + + // Seed a temp fixture dir with known files. + tmp = mkdtempSync(join(tmpdir(), 'reefdoc-e2e-')); + writeFileSync(join(tmp, 'doc.md'), DOC_MD); + writeFileSync(join(tmp, 'sample.pdf'), PDF_BYTES); + + const port = await freePort(); + base = `http://127.0.0.1:${port}`; + proc = spawn(join(repoRoot, 'reefdoc'), ['-addr', `127.0.0.1:${port}`, tmp], { + cwd: repoRoot, + stdio: 'inherit', + }); + await waitForServer(base); +}); + +test.afterAll(async () => { + if (proc) proc.kill('SIGKILL'); + if (tmp) rmSync(tmp, { recursive: true, force: true }); +}); + +test('server is reachable and serves the app shell', async ({ page }) => { + const res = await page.goto(base + '/'); + expect(res.ok()).toBeTruthy(); + // The file tree should list our seeded doc. + await expect(page.locator('#tree')).toContainText('doc.md'); +}); +``` + +- [ ] **Step 2: Run the harness smoke test** + +Run: `npm run e2e` +Expected: PASS (1 test). This proves: `go build` works, the binary spawns on a free port, the ready-poll works, Playwright connects, and the tree renders the fixture. If `go` is not on PATH, the build step throws a clear error — install Go and retry. + +- [ ] **Step 3: Commit** + +```bash +git add web/e2e/download.e2e.js +git commit -m "test(e2e): harness that builds and serves the real reefdoc binary" +``` + +--- + +## Task 3: The five download E2E test cases + +**Files:** +- Modify: `web/e2e/download.e2e.js` + +Add the real coverage. Each case uses stable selectors already in the markup: +- file tree row: `#tree .tree-item[data-path=""]` +- download button: `#download-btn` +- tab close control: `.tab .close` + +- [ ] **Step 1: Replace the smoke test with the five lifecycle/download tests** + +In `web/e2e/download.e2e.js`, remove the `test('server is reachable ...')` smoke test and add the following. Note: `page.goto` with `localStorage` cleared on first load makes the "no document open" assertion deterministic (reefdoc persists tab sessions in `localStorage`). + +```js +// Open the app with a clean slate: clear persisted session/tab state so no +// document auto-restores. Must run before app.js reads localStorage, so we +// navigate, clear, then reload. +async function freshPage(page) { + await page.goto(base + '/'); + await page.evaluate(() => localStorage.clear()); + await page.reload(); + await page.waitForSelector('#tree'); +} + +// Click a file row in the tree to open it as a tab. +async function openDoc(page, path) { + await page.locator(`#tree .tree-item[data-path="${path}"]`).click(); +} + +test('download button is hidden when no document is open', async ({ page }) => { + await freshPage(page); + await expect(page.locator('#download-btn')).toBeHidden(); +}); + +test('download button appears after opening a document', async ({ page }) => { + await freshPage(page); + await openDoc(page, 'doc.md'); + await expect(page.locator('#download-btn')).toBeVisible(); +}); + +test('downloads a text document with correct name and contents', async ({ page }) => { + await freshPage(page); + await openDoc(page, 'doc.md'); + await expect(page.locator('#download-btn')).toBeVisible(); + + const [download] = await Promise.all([ + page.waitForEvent('download'), + page.locator('#download-btn').click(), + ]); + expect(download.suggestedFilename()).toBe('doc.md'); + + const savePath = join(tmp, 'downloaded-doc.md'); + await download.saveAs(savePath); + const got = readFileSync(savePath); + const want = readFileSync(join(tmp, 'doc.md')); + expect(Buffer.compare(got, want)).toBe(0); +}); + +test('downloads a binary document with byte-for-byte fidelity', async ({ page }) => { + await freshPage(page); + await openDoc(page, 'sample.pdf'); + await expect(page.locator('#download-btn')).toBeVisible(); + + const [download] = await Promise.all([ + page.waitForEvent('download'), + page.locator('#download-btn').click(), + ]); + expect(download.suggestedFilename()).toBe('sample.pdf'); + + const savePath = join(tmp, 'downloaded-sample.pdf'); + await download.saveAs(savePath); + const got = readFileSync(savePath); + const want = readFileSync(join(tmp, 'sample.pdf')); + expect(Buffer.compare(got, want)).toBe(0); +}); + +test('download button hides again after closing the last tab', async ({ page }) => { + await freshPage(page); + await openDoc(page, 'doc.md'); + await expect(page.locator('#download-btn')).toBeVisible(); + + // Close every open tab via its × control. + const closers = page.locator('.tab .close'); + for (let n = await closers.count(); n > 0; n = await closers.count()) { + await closers.first().click(); + } + await expect(page.locator('#download-btn')).toBeHidden(); +}); +``` + +- [ ] **Step 2: Run the full E2E suite** + +Run: `npm run e2e` +Expected: PASS (5 tests). All five assert real user-visible behavior against the real binary. + +- [ ] **Step 3: Sanity-check that the test actually guards the behavior** + +Temporarily break the wiring to prove the test fails when the feature is broken: +1. In `web/app.js`, comment out the body of `refreshDownloadButton` so it becomes a no-op (the button never shows/hides). +2. Run `go build -o reefdoc .` is NOT needed manually — the E2E `beforeAll` rebuilds. Just run `npm run e2e`. +3. Expected: the "appears after opening" and "hides after closing" tests FAIL (button stays hidden/visible). This confirms the test has teeth. +4. Revert the change to `web/app.js` (restore the one-line body `downloadBtn.hidden = !store.active;`), run `go build` is again handled by `beforeAll`; run `npm run e2e` and confirm all 5 pass again. + +Do not commit the temporary break. Verify `git diff web/app.js` is empty after reverting. + +- [ ] **Step 4: Commit** + +```bash +git add web/e2e/download.e2e.js +git commit -m "test(e2e): cover download button lifecycle and file fidelity" +``` + +--- + +## Task 4: CI job + +**Files:** +- Modify: `.github/workflows/ci.yml` + +- [ ] **Step 1: Add the `e2e` job** + +The current file has two jobs (`go`, `web`). Add a third job `e2e` at the same indentation level under `jobs:` (after the `web` job). Insert: + +```yaml + e2e: + name: E2E (Playwright + real binary) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: + go-version: '1.23' + cache: true + - uses: actions/setup-node@v6 + with: + node-version: '22' + - run: npm install + - run: npx playwright install --with-deps chromium + - run: npm run e2e +``` + +- [ ] **Step 2: Validate the workflow YAML locally** + +Run: +```bash +python3 -c "import yaml,sys; yaml.safe_load(open('.github/workflows/ci.yml')); print('YAML OK')" +``` +Expected: `YAML OK`. (Confirms the new job didn't break the file's structure/indentation.) + +- [ ] **Step 3: Confirm the unit `web` job still excludes the E2E file** + +Run: `npm test` +Expected: the existing 60 unit tests pass and the run does NOT execute `download.e2e.js` (node --test globs `*.test.js`, not `*.e2e.js`). Confirm the test count is unchanged at 60. + +- [ ] **Step 4: Commit** + +```bash +git add .github/workflows/ci.yml +git commit -m "ci: add e2e job running Playwright against the real binary" +``` + +--- + +## Task 5: Documentation + +**Files:** +- Modify: `README.md` + +- [ ] **Step 1: Document how to run the E2E suite** + +In `README.md`, find the testing line in the Status section (currently: "Run the tests with `go test ./...` and `npm test`.") and expand it to mention the E2E suite. Replace that sentence with: + +```markdown +Run the unit tests with `go test ./...` and `npm test`. The browser end-to-end +test (Playwright, drives the real binary) runs with `npm run e2e` — it needs Go +on your PATH and Playwright's Chromium installed (`npx playwright install chromium`). +``` + +- [ ] **Step 2: Verify the README still reads cleanly** + +Re-read the Status section to confirm the sentence flows and no version/blockquote was touched. + +- [ ] **Step 3: Commit** + +```bash +git add README.md +git commit -m "docs: document the npm run e2e browser test" +``` + +--- + +## Self-Review Notes + +- **Spec coverage:** Harness (build/spawn/free-port/ready-poll/cleanup) → Task 2. The 5 test cases (hidden / appears / text download+contents / binary download+contents / hides after close) → Task 3. Playwright dep + Chromium-only config → Task 1. Isolated `e2e` CI job → Task 4. `web` job stays clean via `*.e2e.js` naming → verified in Task 4 Step 3. Reliability mitigations (free port, ready-poll, localStorage clear, waitForEvent-before-click, afterAll kill) → all present in Tasks 2-3. The spec's "break the feature to prove the test has teeth" sanity step → Task 3 Step 3. +- **Placeholder scan:** No TBDs. Playwright version pinned (`1.48.2`); if `npm install` resolves a different current patch, that's acceptable — the API used (`waitForEvent('download')`, `download.saveAs`, `download.suggestedFilename`) is stable across 1.4x. +- **Naming/selector consistency:** Selectors (`#download-btn`, `#tree .tree-item[data-path]`, `.tab .close`) verified against `web/app.js` and `web/index.html`. Helper names (`freshPage`, `openDoc`, `freePort`, `waitForServer`) are consistent across Tasks 2-3. `tmp`/`base`/`proc` module-level vars are shared between `beforeAll` and the tests. From 969427537eab3f904ab473563386ac6ed58c73bb Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Sat, 27 Jun 2026 07:15:47 +0900 Subject: [PATCH 15/21] docs: pin Playwright 1.55.0 (Node 24 compat) in E2E plan --- docs/plans/2026-06-26-download-e2e.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plans/2026-06-26-download-e2e.md b/docs/plans/2026-06-26-download-e2e.md index 394f3ce..0e9da8d 100644 --- a/docs/plans/2026-06-26-download-e2e.md +++ b/docs/plans/2026-06-26-download-e2e.md @@ -58,7 +58,7 @@ Change it to (adds the `e2e` script and the `@playwright/test` devDependency; ke "e2e": "playwright test" }, "devDependencies": { - "@playwright/test": "1.48.2", + "@playwright/test": "1.55.0", "highlight.js": "11.9.0", "markdown-it": "14.1.0", "markdown-it-task-lists": "2.1.1" @@ -423,5 +423,5 @@ git commit -m "docs: document the npm run e2e browser test" ## Self-Review Notes - **Spec coverage:** Harness (build/spawn/free-port/ready-poll/cleanup) → Task 2. The 5 test cases (hidden / appears / text download+contents / binary download+contents / hides after close) → Task 3. Playwright dep + Chromium-only config → Task 1. Isolated `e2e` CI job → Task 4. `web` job stays clean via `*.e2e.js` naming → verified in Task 4 Step 3. Reliability mitigations (free port, ready-poll, localStorage clear, waitForEvent-before-click, afterAll kill) → all present in Tasks 2-3. The spec's "break the feature to prove the test has teeth" sanity step → Task 3 Step 3. -- **Placeholder scan:** No TBDs. Playwright version pinned (`1.48.2`); if `npm install` resolves a different current patch, that's acceptable — the API used (`waitForEvent('download')`, `download.saveAs`, `download.suggestedFilename`) is stable across 1.4x. +- **Placeholder scan:** No TBDs. Playwright pinned at `1.55.0` (Node 24 compatible; `1.48.2` deadlocks its TS-ESM loader on Node 24). The download API used (`waitForEvent('download')`, `download.saveAs`, `download.suggestedFilename`) is stable across 1.4x–1.5x. - **Naming/selector consistency:** Selectors (`#download-btn`, `#tree .tree-item[data-path]`, `.tab .close`) verified against `web/app.js` and `web/index.html`. Helper names (`freshPage`, `openDoc`, `freePort`, `waitForServer`) are consistent across Tasks 2-3. `tmp`/`base`/`proc` module-level vars are shared between `beforeAll` and the tests. From e4abca939a069286b24e372fd30422805cd040d8 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Sat, 27 Jun 2026 07:17:01 +0900 Subject: [PATCH 16/21] build(e2e): add Playwright (chromium) config and scripts --- .gitignore | 4 ++++ package.json | 6 +++++- playwright.config.js | 16 ++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 playwright.config.js diff --git a/.gitignore b/.gitignore index ae42fcf..444f01e 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,7 @@ # Git worktrees /.worktrees + +# Playwright E2E artifacts +/test-results +/playwright-report diff --git a/package.json b/package.json index 5155527..73e1ecc 100644 --- a/package.json +++ b/package.json @@ -2,8 +2,12 @@ "name": "reefdoc-web-tests", "private": true, "type": "module", - "scripts": { "test": "node --test" }, + "scripts": { + "test": "node --test", + "e2e": "playwright test" + }, "devDependencies": { + "@playwright/test": "1.55.0", "highlight.js": "11.9.0", "markdown-it": "14.1.0", "markdown-it-task-lists": "2.1.1" diff --git a/playwright.config.js b/playwright.config.js new file mode 100644 index 0000000..acb853d --- /dev/null +++ b/playwright.config.js @@ -0,0 +1,16 @@ +import { defineConfig, devices } from '@playwright/test'; + +export default defineConfig({ + testDir: './web/e2e', + testMatch: '**/*.e2e.js', + fullyParallel: false, + forbidOnly: !!process.env.CI, + retries: process.env.CI ? 1 : 0, + workers: 1, + timeout: 30_000, + reporter: process.env.CI ? 'list' : 'line', + outputDir: './test-results', + projects: [ + { name: 'chromium', use: { ...devices['Desktop Chrome'] } }, + ], +}); From 25e57e4a626aea8ee22f0710bddf6828a235c5c1 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Sat, 27 Jun 2026 07:21:01 +0900 Subject: [PATCH 17/21] test(e2e): harness that builds and serves the real reefdoc binary --- web/e2e/download.e2e.js | 82 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 web/e2e/download.e2e.js diff --git a/web/e2e/download.e2e.js b/web/e2e/download.e2e.js new file mode 100644 index 0000000..93ba646 --- /dev/null +++ b/web/e2e/download.e2e.js @@ -0,0 +1,82 @@ +import { test, expect } from '@playwright/test'; +import { spawn, execFileSync } from 'node:child_process'; +import { createServer } from 'node:net'; +import { mkdtempSync, writeFileSync, rmSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +// Repo root is two levels up from web/e2e/. +const repoRoot = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); + +// Known fixture content, asserted byte-for-byte after download. +const DOC_MD = '# Title\n\nHello from reefdoc E2E.\n'; +// Minimal valid-enough PDF bytes (content fidelity is what we assert, not +// renderability). Includes a non-ASCII/control-ish byte to prove binary safety. +const PDF_BYTES = Buffer.from([0x25, 0x50, 0x44, 0x46, 0x2d, 0x31, 0x2e, 0x34, + 0x0a, 0x00, 0xff, 0xfe, 0x80, 0x0a, 0x25, 0x25, 0x45, 0x4f, 0x46, 0x0a]); + +// Pick a free TCP port by binding :0, reading the assigned port, releasing it. +function freePort() { + return new Promise((resolve, reject) => { + const srv = createServer(); + srv.unref(); + srv.on('error', reject); + srv.listen(0, '127.0.0.1', () => { + const { port } = srv.address(); + srv.close(() => resolve(port)); + }); + }); +} + +// Poll GET / until 200 or timeout. +async function waitForServer(base, timeoutMs = 10_000) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + try { + const res = await fetch(base + '/'); + if (res.ok) return; + } catch { + // not up yet + } + await new Promise((r) => setTimeout(r, 100)); + } + throw new Error('reefdoc server did not become ready at ' + base); +} + +let proc; +let tmp; +let base; + +test.beforeAll(async () => { + // Build the real binary (exercises go:embed assets and the real server). + execFileSync('go', ['build', '-o', 'reefdoc', '.'], { + cwd: repoRoot, + stdio: 'inherit', + }); + + // Seed a temp fixture dir with known files. + tmp = mkdtempSync(join(tmpdir(), 'reefdoc-e2e-')); + writeFileSync(join(tmp, 'doc.md'), DOC_MD); + writeFileSync(join(tmp, 'sample.pdf'), PDF_BYTES); + + const port = await freePort(); + base = `http://127.0.0.1:${port}`; + proc = spawn(join(repoRoot, 'reefdoc'), ['-addr', `127.0.0.1:${port}`, tmp], { + cwd: repoRoot, + stdio: 'inherit', + }); + await waitForServer(base); +}); + +test.afterAll(async () => { + if (proc) proc.kill('SIGKILL'); + if (tmp) rmSync(tmp, { recursive: true, force: true }); +}); + +test('server is reachable and serves the app shell', async ({ page }) => { + const res = await page.goto(base + '/'); + expect(res.ok()).toBeTruthy(); + // The file tree should list our seeded doc. + await expect(page.locator('#tree')).toContainText('doc.md'); +}); From 825bfbd7670824c755e3afedf20444280d16f804 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Sat, 27 Jun 2026 07:25:22 +0900 Subject: [PATCH 18/21] test(e2e): fail fast if reefdoc exits before ready --- web/e2e/download.e2e.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/web/e2e/download.e2e.js b/web/e2e/download.e2e.js index 93ba646..0a5c5d2 100644 --- a/web/e2e/download.e2e.js +++ b/web/e2e/download.e2e.js @@ -29,10 +29,18 @@ function freePort() { }); } -// Poll GET / until 200 or timeout. -async function waitForServer(base, timeoutMs = 10_000) { +// Poll GET / until 200 or timeout. Bails fast (with the real reason) if the +// spawned binary exits before becoming ready, instead of waiting the full +// timeout and throwing a vague message. +async function waitForServer(base, proc, timeoutMs = 10_000) { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { + if (proc?.exitInfo) { + throw new Error( + `reefdoc exited before becoming ready (code=${proc.exitInfo.code}, ` + + `signal=${proc.exitInfo.signal}); see stderr above` + ); + } try { const res = await fetch(base + '/'); if (res.ok) return; @@ -66,7 +74,8 @@ test.beforeAll(async () => { cwd: repoRoot, stdio: 'inherit', }); - await waitForServer(base); + proc.on('exit', (code, signal) => { proc.exitInfo = { code, signal }; }); + await waitForServer(base, proc); }); test.afterAll(async () => { From 53af95c1be8b974290bf3dd22cae188a028035ae Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Sat, 27 Jun 2026 07:27:58 +0900 Subject: [PATCH 19/21] test(e2e): cover download button lifecycle and file fidelity --- web/e2e/download.e2e.js | 78 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 73 insertions(+), 5 deletions(-) diff --git a/web/e2e/download.e2e.js b/web/e2e/download.e2e.js index 0a5c5d2..f4bc275 100644 --- a/web/e2e/download.e2e.js +++ b/web/e2e/download.e2e.js @@ -83,9 +83,77 @@ test.afterAll(async () => { if (tmp) rmSync(tmp, { recursive: true, force: true }); }); -test('server is reachable and serves the app shell', async ({ page }) => { - const res = await page.goto(base + '/'); - expect(res.ok()).toBeTruthy(); - // The file tree should list our seeded doc. - await expect(page.locator('#tree')).toContainText('doc.md'); +// Open the app with a clean slate: clear persisted session/tab state so no +// document auto-restores. Must run before app.js reads localStorage, so we +// navigate, clear, then reload. +async function freshPage(page) { + await page.goto(base + '/'); + await page.evaluate(() => localStorage.clear()); + await page.reload(); + await page.waitForSelector('#tree'); +} + +// Click a file row in the tree to open it as a tab. +async function openDoc(page, path) { + await page.locator(`#tree .tree-item[data-path="${path}"]`).click(); +} + +test('download button is hidden when no document is open', async ({ page }) => { + await freshPage(page); + await expect(page.locator('#download-btn')).toBeHidden(); +}); + +test('download button appears after opening a document', async ({ page }) => { + await freshPage(page); + await openDoc(page, 'doc.md'); + await expect(page.locator('#download-btn')).toBeVisible(); +}); + +test('downloads a text document with correct name and contents', async ({ page }) => { + await freshPage(page); + await openDoc(page, 'doc.md'); + await expect(page.locator('#download-btn')).toBeVisible(); + + const [download] = await Promise.all([ + page.waitForEvent('download'), + page.locator('#download-btn').click(), + ]); + expect(download.suggestedFilename()).toBe('doc.md'); + + const savePath = join(tmp, 'downloaded-doc.md'); + await download.saveAs(savePath); + const got = readFileSync(savePath); + const want = readFileSync(join(tmp, 'doc.md')); + expect(Buffer.compare(got, want)).toBe(0); +}); + +test('downloads a binary document with byte-for-byte fidelity', async ({ page }) => { + await freshPage(page); + await openDoc(page, 'sample.pdf'); + await expect(page.locator('#download-btn')).toBeVisible(); + + const [download] = await Promise.all([ + page.waitForEvent('download'), + page.locator('#download-btn').click(), + ]); + expect(download.suggestedFilename()).toBe('sample.pdf'); + + const savePath = join(tmp, 'downloaded-sample.pdf'); + await download.saveAs(savePath); + const got = readFileSync(savePath); + const want = readFileSync(join(tmp, 'sample.pdf')); + expect(Buffer.compare(got, want)).toBe(0); +}); + +test('download button hides again after closing the last tab', async ({ page }) => { + await freshPage(page); + await openDoc(page, 'doc.md'); + await expect(page.locator('#download-btn')).toBeVisible(); + + // Close every open tab via its × control. + const closers = page.locator('.tab .close'); + for (let n = await closers.count(); n > 0; n = await closers.count()) { + await closers.first().click(); + } + await expect(page.locator('#download-btn')).toBeHidden(); }); From 39a13c3fe6313422887ed82e651aa6b1031efd30 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Sat, 27 Jun 2026 07:32:46 +0900 Subject: [PATCH 20/21] ci: add e2e job running Playwright against the real binary --- .github/workflows/ci.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 303656e..78c4be9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,3 +38,19 @@ jobs: node-version: '22' - run: npm install - run: npm test + + e2e: + name: E2E (Playwright + real binary) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: + go-version: '1.23' + cache: true + - uses: actions/setup-node@v6 + with: + node-version: '22' + - run: npm install + - run: npx playwright install --with-deps chromium + - run: npm run e2e From cf74c3f73306b1ac4aa90b86a7928880f3f183b7 Mon Sep 17 00:00:00 2001 From: Yuki Hayashi Date: Sat, 27 Jun 2026 07:33:32 +0900 Subject: [PATCH 21/21] docs: document the npm run e2e browser test --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 28f3c57..4e72b5e 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,9 @@ plus a vanilla-JS frontend (tree, tabs, markdown/mermaid/highlighting, TOC, themes, live reload). See [`docs/specs`](docs/specs) for the design and [`docs/plans`](docs/plans) for the implementation plan. -Run the tests with `go test ./...` and `npm test`. +Run the unit tests with `go test ./...` and `npm test`. The browser end-to-end +test (Playwright, drives the real binary) runs with `npm run e2e` — it needs Go +on your PATH and Playwright's Chromium installed (`npx playwright install chromium`). ## Architecture