Upgrading Repo to Harper v5#2
Conversation
- UncachedBlog.get(), PageBuilder.get(), CachedBlog.get() → static - this → target (record properties and renderPost argument) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
target is a RequestTarget (URLSearchParams), not the record. Fetch post/cached via tables.Post.get(target) and tables.BlogCache.get(target) before passing to renderPost or accessing .content. Remove unused context parameter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add @harperfast/integration-testing + typescript dev deps and a test:integration script - Add integrationTests/app.test.ts covering the SSR render path (UncachedBlog + CachedBlog), the Post REST table, and Harper's multi-tier cache behavior (304 cache hit, source-change invalidation, re-cache). The suite builds the Vite bundles before installing the fixture so the SSR component can load dist/. - Apply the harperBinPath harness fix (harper's exports map only exposes ".", so the harness auto-resolution throws ERR_PACKAGE_PATH_NOT_EXPORTED) - Add .github/workflows/integration-tests.yml (Node 22/24/26, actions pinned by commit hash, with a build step for the SSR bundles) - Add tsconfig.json - Branding: HarperDB -> Harper in README prose Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI surfaced two issues:
- CachedBlog returned a full { status, headers, body } response. Harper's
REST layer treats any response carrying a `headers` property as a
complete Response and skips its conditional-request handling, so no
ETag/Last-Modified was emitted and 304 cache hits never occurred.
Revert to the { contentType, data } shape (the v4 behavior, still valid
in v5) so Harper applies caching from the BlogCache record. UncachedBlog
keeps the full-response shape (it is intentionally uncached).
- Relax the SSR Content-Type assertions to startsWith('text/html').
- Regenerate package-lock.json from a clean install so the optional
native deps of `ws` (bufferutil, utf-8-validate, node-gyp-build) are
present; the prior lockfile omitted them and broke `npm ci` on Linux CI.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
After the source Post is updated, the BlogCache re-populates and its Last-Modified can keep advancing for a moment while it settles, so validators captured from one response can already be stale by the next request (200 instead of 304). Fetch fresh validators immediately before the conditional re-request and retry briefly until a stable 304 cache hit, rather than reusing a single snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In Harper v5 a sourcedFrom cache source is resolved per-id by instantiating
the source and calling its instance get(), so PageBuilder's static get() was
never invoked and BlogCache stored raw Post records (cached.content
undefined) — /CachedBlog served an empty body. Convert PageBuilder,
UncachedBlog, and CachedBlog from static get(target) to instance get(query)
calling await super.get(query), matching harper's SimpleCacheSource and the
react-ssr-example reference. Keep the { contentType, data } response shape.
Strengthen the integration test: /CachedBlog/0 must return 200 with a
non-empty text/html body containing __INITIAL_POST_DATA__ and the post title,
which fails on the pre-fix static-get bug.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
| } | ||
|
|
||
| export class UncachedBlog extends tables.Post { | ||
| async get() { |
There was a problem hiding this comment.
Bug: XSS via unescaped JSON.stringify inside <script> tag
JSON.stringify(post) is injected directly into a <script> block without escaping HTML-special characters. A post whose title, body, or comments contain </script> terminates the script tag early, breaking the page and enabling stored XSS.
Failure scenario: title: 'x</script><script>alert(document.cookie)</script>' — the generated HTML becomes <script>window.__INITIAL_POST_DATA__ = {"title":"x</script><script>alert(...); the first </script> closes the tag and the injected script executes.
Fix: escape < and > in the serialized JSON:
.replace(`<!--app-data-->`, `<script>window.__INITIAL_POST_DATA__ = ${JSON.stringify(post).replace(/</g, '\\u003c').replace(/>/g, '\\u003e')};</script>`)| async get() { | ||
| async get(query) { | ||
| const post = await super.get(query); | ||
| return { |
There was a problem hiding this comment.
Bug: null deref crash when record is missing in UncachedBlog.get()
super.get(query) returns null when no record exists for the given id. The result is passed directly to renderPost(post), which calls serverEntry.render({ initialPostData: null }). Inside the Vue app, ref(null).value.title throws TypeError: Cannot read properties of null (reading 'title') during SSR.
Failure scenario: GET /UncachedBlog/nonexistent — Harper routes the request to this handler; super.get('nonexistent') returns null; renderPost(null) crashes; the unhandled error surfaces as a 500 instead of a clean 404.
Fix: guard against a missing record:
async get(query) {
const post = await super.get(query);
if (!post) return { status: 404 };
return { status: 200, headers: { 'Content-Type': 'text/html' }, body: await renderPost(post) };
}|
|
||
| // Caching source for BlogCache. In v5 a caching source resolves per-id through | ||
| // an instance `get`, so the cache instantiates this resource for the requested | ||
| // id and calls `get()`; `super.get()` returns the underlying Post record. |
There was a problem hiding this comment.
Bug: null deref crash when record is missing in PageBuilder.get()
Same issue as UncachedBlog.get(): if super.get(query) returns null (no record for the requested id), renderPost(null) crashes with a TypeError inside Vue's SSR renderer. Since PageBuilder is the caching source for BlogCache, this will also surface as an unhandled error when the cache tries to populate an entry that doesn't exist in the Post table.
Failure scenario: GET /CachedBlog/nonexistent triggers the cache source; PageBuilder.get('nonexistent') calls super.get('nonexistent') which returns null; renderPost(null) throws.
Fix:
async get(query) {
const post = await super.get(query);
if (!post) return null; // propagate miss to the cache layer
return { content: await renderPost(post) };
}| async get(query) { | ||
| const post = await super.get(query); | ||
| return { | ||
| content: await renderPost(this), |
There was a problem hiding this comment.
Bug: null deref crash in CachedBlog.get() when super.get() returns null
If the BlogCache table has no entry for the requested id (cache miss with no source-populated record yet), super.get(query) returns null. The next line cached.content then throws TypeError: Cannot read properties of null (reading 'content').
Failure scenario: First request to GET /CachedBlog/\<id> where the Post record was deleted after the cache entry was evicted — super.get(query) returns null; null.content throws a 500 instead of a clean 404.
Fix:
async get(query) {
const cached = await super.get(query);
if (!cached) return { status: 404 };
return { contentType: 'text/html', data: cached.content };
}| // The `harper` package's `exports` map only exposes ".", so the harness's | ||
| // auto-resolution of 'harper/dist/bin/harper.js' fails with ERR_PACKAGE_PATH_NOT_EXPORTED. | ||
| // Resolve the CLI from the (exported) main entry and pass it explicitly. | ||
| const require = createRequire(import.meta.url); |
There was a problem hiding this comment.
Cleanup: build guard checks only entry-server.js, misses dist/client/index.html
resources.js reads both dist/server/entry-server.js (imported dynamically) and dist/client/index.html (read synchronously at startup). The guard here only checks for the server bundle; if the client build is absent or partial (e.g. a leftover server build from a previous run where the client build failed), Harper will start but crash immediately when resources.js tries to readFileSync the missing HTML template.
Failure scenario: developer runs npm run build:server without build:client, or a prior CI run failed mid-build leaving only the server bundle — test proceeds past the guard, Harper starts, resources.js throws ENOENT: no such file or directory, open '.../dist/client/index.html', all tests fail with an unhelpful startup error.
Fix: check both outputs:
if (
!existsSync(resolve(FIXTURE_PATH, 'dist/server/entry-server.js')) ||
!existsSync(resolve(FIXTURE_PATH, 'dist/client/index.html'))
) {
execFileSync('npm', ['run', 'build'], { cwd: FIXTURE_PATH, stdio: 'inherit' });
}| body: JSON.stringify({ comments: current.comments.concat(`Test comment ${Math.random()}`) }), | ||
| }); | ||
| ok(patch.ok, `expected PATCH to succeed, got ${patch.status}`); | ||
|
|
There was a problem hiding this comment.
Cleanup: strictEqual(fresh.status, 200) inside the retry loop produces a misleading failure
The unconditional GET /CachedBlog/0 on every loop iteration is expected to always return 200 — if it doesn't (e.g. a transient 500 after cache re-population), the strictEqual inside the loop fires and the test fails with Expected: 200, Actual: 500 at an iteration-scoped line, obscuring the actual problem (which is that the cache didn't settle to a stable 304).
Cost: when this assertion fires it masks the real failure — the outer strictEqual(reCachedStatus, 304, '...') with its descriptive message is never reached, making CI failures harder to diagnose.
Suggestion: move the guard outside the loop or use ok(fresh.status === 200, ...) with a message that names the attempt number, so a real cache re-population error is distinguishable from a 304-settle timeout.
- Escape < and > in JSON.stringify(post) to prevent stored XSS via </script> injection - Return 404 in UncachedBlog.get() when super.get() returns null (missing record) - Return null in PageBuilder.get() when super.get() returns null to propagate cache miss - Return 404 in CachedBlog.get() when super.get() returns null (cache miss / evicted) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Review follow-up (autonomous agent): Fixed all 4 blocking findings: XSS via unescaped JSON in |
Harper v4 → v5 upgrade
Completes the v5 upgrade for the Vue SSR + multi-tier caching example. Builds on the existing migration commits on this branch (dependency swap + Resource method conversion), fixes a caching regression those commits introduced, and adds integration tests + CI.
Dependency
harper@^5.0.11(was theharperdbv4 runtime global). Allharperdbimport/global usage removed.@harperfast/integration-testing@^0.4.0,typescript.Migration items applied
resources.jsimports{ tables } from 'harper'.UncachedBlog,PageBuilder(theBlogCachesource), andCachedBlognow use instanceasync get(query)callingawait super.get(query), matching Harper's ownSimpleCacheSourceand thereact-ssr-examplereference. An earlier migration commit had mechanically converted these tostatic async get(target). The canonical v5 caching-source contract is the instance form:tables.BlogCache.sourcedFrom(PageBuilder)resolves a record by delegating through the source's base staticget(which isreliesOnPrototype), instantiating the source and invoking the instanceget. Aligning to the instance form keeps the example idiomatic and future-proof, and avoids depending on the static handler being called directly. (Note: in the currently-resolved runtime,harper@5.0.28, the sourcegetdispatch —resources/Table.jsthrottledCallToSource— accepts both a user staticgetand the inherited base staticgetbacked by an instance method, so the static form did not actually produce an empty body in CI; see Tests below. The change is a correctness/idiomatic alignment rather than a fix for an observed CI failure.){ contentType, data }shape (with its explanatory comment). Harper's REST layer treats any response carrying aheadersproperty as a complete Response and skips its conditional-request handling, so a full{ status, headers, body }response suppressesETag/Last-Modifiedand breaks 304 cache hits. The{ contentType, data }shape sets Content-Type while leaving caching to theBlogCacherecord.UncachedBlogkeeps the full-response shape since it is intentionally uncached.Migration items N/A (and why)
Table.get()shape /wasLoadedFromSource()— not used; invalidation is driven bysourcedFrom()+ tableexpiration.PATCHonPost.getContext, explicit commit) — no manual transaction management.Tests
Added/strengthened
integrationTests/app.test.ts(real Harper via@harperfast/integration-testing):GET /UncachedBlog/0returnstext/htmlwith the SSR-injected__INITIAL_POST_DATA__hydration script.GET /CachedBlog/0must return200with a non-emptytext/htmlbody that contains the full HTML document, the__INITIAL_POST_DATA__hydration marker, and the seeded post title — asserting the cache stores a real rendered page, not a raw/empty record.Post/0is auto-seeded on startup and readable./CachedBlogendpoint) — conditional re-request yields304(cache hit);PATCHto the sourcePostinvalidates the cache (200on the next conditional request); the re-populated cache yields304again.The suite runs
npm run buildbefore installing the fixture so the SSR component can importdist/server/entry-server.jsand servedist/client/index.html.Applies the mandatory harness fix:
harper'sexportsmap only exposes".", so the harness's auto-resolution ofharper/dist/bin/harper.jsthrowsERR_PACKAGE_PATH_NOT_EXPORTED. The test resolves the CLI from the exported main entry and passes it asharperBinPath. (Upstream: harper should export its bin path, or the harness should resolve via the package root.)Test status:
EADDRNOTAVAILon 127.0.0.2+) — environmental (loopback aliases not configured / require interactive sudo), not a code bug.Lockfile
package-lock.jsonincludes the optional native deps ofws(bufferutil,utf-8-validate,node-gyp-build) sonpm cisucceeds across the[22, 24, 26]matrix; nofile:/.tgzpins.CI
.github/workflows/integration-tests.ymlat repo root: Node matrix[22, 24, 26], actions pinned by commit hash, anpm run buildstep (SSR bundles) beforetest:integration, and Harper log upload on failure.Branding
HarperDB→HarperinREADME.mdprose (incl.harperdb run .→harper run .). Live URLs unchanged.npm scope
Not flagged — package is
privateand unpublished; no scope move applicable.Blockers
None.
🤖 Generated with Claude Code