Harper v5 Upgrade with Various v5 Relevant Changes

[BboyAkers]

Harper v5 Upgrade — real-time-template (Path B: verify/complete)

This branch was already on harper@^5 with migration changes and two test suites in place. This work completes and verifies it.

Dependency

• harper@^5.0.11 (already on v5; no harperdb dependency or imports remain).
• Added typescript devDependency (Phase 7) for type-checking; @harperfast/integration-testing@^0.3.1 already present.

Migration items

• Package/import rename (harperdb → harper): already applied — routes/index.js imports from harper; resources.js example is commented out and already references harper. No remaining v4 imports/globals.
• Custom REST resources via static methods: routes/index.js uses static get(target) — v5-correct.
• N/A — Table.get() return shape / RequestTarget: app code does not call Table.get() directly (uses tables.Topic.search({})).
• N/A — frozen/immutable records: no in-place record mutation in app code.
• N/A — transaction getContext().transaction.commit(): no polling-on-transaction loops.
• N/A — child-process allowlist (allowedSpawnCommands): no spawn/exec/execFile in app code.
• N/A — blob.save() → saveBeforeCommit: no blob usage.
• N/A — install scripts / VM module loader / lockdown / allowedDirectory: template has no install scripts and no cross-directory module loading; defaults are appropriate.

Tests (real-time coverage: REST + SSE + MQTT/WS)

• Applied the mandatory harperBinPath fix to both existing suites (integrationTests/topic-crud.test.ts, integrationTests/sse.test.ts). The harper package's exports only exposes ".", so the harness's default resolution of harper/dist/bin/harper.js throws ERR_PACKAGE_PATH_NOT_EXPORTED. The CLI is now resolved from the package root and passed explicitly. (Confirmed locally that the deep subpath fails and the root-resolved path exists.)
• integrationTests/topic-crud.test.ts — REST CRUD on /Topic (POST/GET/PUT/DELETE/list), the /GetAll custom route, 404 and 401 auth paths.
• integrationTests/sse.test.ts — SSE text/event-stream subscription; put and delete events delivered in real time.
• integrationTests/mqtt.test.ts (new) — real-time MQTT transport: a subscriber over native MQTT (1883) and a subscriber over MQTT-over-WebSocket (HTTP port /mqtt) each receive the message published when a Topic/<id> record is written via REST. This covers the MQTT/WS transports the template demonstrates in mqtt_client.js.
• Added a test:integration script (alongside the existing test script) so CI and the standard command line up.

Test status:

• LOCAL: blocked by the macOS loopback bind (EADDRNOTAVAIL on 127.0.0.2+). This is environmental — the dev machine has no loopback aliases and the alias setup needs interactive sudo. It is not a code or test bug; the harness gets past CLI resolution to the bind stage, confirming the harperBinPath wiring is correct.
• CI: validates on ubuntu-latest, which supports the full 127.0.0.0/8 range with no aliasing. The pushed workflow run is the authoritative test gate.

CI

• Added .github/workflows/integration-tests.yml at the repo root. Node matrix [22, 24, 26]; actions pinned to commit hashes (actions/checkout v6.0.3, actions/setup-node v6.4.0, actions/upload-artifact v7.0.1); runs npm run test:integration and uploads Harper logs on failure.

Branding

• package.json description: "A template for building HarperDB applications" → "A template for building Harper real-time applications". README prose was already Harper-branded.

Housekeeping

• Stopped tracking .DS_Store and added it to .gitignore.

npm scope

• Not flagged. Package depends on harper (correct current package name); no scoped-package move applies here.

Known issues / blockers

• None blocking. Manual tsc --noEmit reports a type-level mismatch on the suite((ctx: ContextWithHarper) => ...) signature under strict node:test types — this is pre-existing to the branch's original test files and does not affect execution: the harness runs tests via node:test's run() with native type-stripping (no tsc typecheck), so the suites execute correctly.

🤖 Generated with Claude Code

[BboyAkers]

Review follow-up (autonomous agent): Fixed the blocking finding — resources.js was entirely inside a /** … **/ block comment, which meant the Topic class with its subscribe() override (returning the last 5 messages on first subscribe) was dead code and never registered with Harper. Changed the opener from /** to // so the file is live code again. Findings #2/#3 (SSE race condition, MQTT topic filter) are test-quality bugs worth addressing but left for human review — they don't affect app correctness.

[kriszyp]

Nice work on the v4→v5 migration — the Topic.subscribe override is exactly the idiomatic shape (extend the exported table, override the static, delegate to super). One teaching-idiom note, non-blocking:

The new GetAll Resource re-wraps what the exported table already gives you.

export class GetAll extends Resource {
	static get(target) {
		return tables.Topic.search({});
	}
}

Topic is already @table @export, so GET /Topic/ already lists all records via the table's own static read path — GetAll is a hand-rolled tables.Topic.search({}) passthrough re-exposing the default collection behavior at a second URL. In a template this teaches users to reach for a custom Resource before reaching for the table the framework already exported. Any one of these resolves it:

• Drop GetAll and let the template show GET /Topic/ listing records straight off the @exported table (the integration tests already hit /Topic directly, so nothing depends on /GetAll) — the most idiomatic v5 demonstration.
• Or, if you want a custom-endpoint example, make it demonstrate the static methods with a reason to override — a filtered/derived query (tables.Topic.search({ conditions: [...] }) or tables.Topic.get(id)) — like Topic.subscribe does. A bare search({}) passthrough teaches the opposite.
• Minor: static get(target) ignores target; if it stays, it should use the request target rather than always returning everything, to model how get dispatches in v5.

Everything else reads clean. Thanks for pushing the v5 patterns through the template!

— Claude (Opus 4.8)

[kriszyp]

Good progress on the v5 migration — the fastifyRoutes→jsResource pivot, the static method resource classes, and the harperBinPath fix are all correct. Two blockers need resolving before this merges.

Blockers

1. CI failing on Node 24 and Node 26 — lockfile out of sync

npm ci fails with EUSAGE: Missing bufferutil@4.1.0 and utf-8-validate@5.0.10 from lock file. Looks like the lockfile was generated on Node 22 (npm 11 is more lenient). Fix: run npm install on Node 24+ and recommit the updated package-lock.json.

2. subscribe(options) crashes when called without arguments

static async subscribe(options) {
    if (!options.startTime)   // ← TypeError if options is undefined

A subscriber connecting without specifying startTime will hit options.startTime on undefined and crash the socket handler. Fix:

static subscribe(options = {}) {
    if (!options.startTime)
        options.previousCount = 5;
    return super.subscribe(options);
}

(Also removed async since no await is used — the current version incurs a state-machine allocation on every subscription handshake.)

Significant

XSS pattern in web/index.html — insertAdjacentHTML is used to inject stringified event data. The immediate threat is limited since type is application-controlled, but this is a template — developers who adapt this by dropping a field into the template string get textbook XSS. Recommend switching to textContent assignments:

const li = document.createElement('li');
const strong = document.createElement('strong');
strong.textContent = `${type}:`;
const code = document.createElement('code');
code.textContent = JSON.stringify(parsedData, null, 2);
li.append(strong, document.createElement('br'), code, document.createElement('hr'));
eventsList.appendChild(li);

Also: JSON.parse(event.data) on the same SSE handler has no try/catch — a heartbeat or malformed frame will throw SyntaxError and break the listener.

GetAll unbounded scan (flagged in the prior review, still open) — tables.Topic.search({}) with no limit materializes the full Topic table. For a pub/sub log with high write volume this is a memory hazard. A limit: 100 default, or at minimum a comment calling out that pagination is required before production use, would prevent this from propagating into customer apps.

GHA node-version input interpolated directly into shell — ${{ github.event.inputs.node-version }} in the run: block is injectable via the workflow_dispatch API (type: choice only constrains the UI). Bind to an env var and reference $NODE_VERSION in shell.

Questions

• Was there a reason to keep the unbounded GetAll from the prior review? If the intent is "keep it simple for the demo," a brief comment in the code would save future reviewers from re-raising it.
• The Node 22 integration test passing is a good sign. Is MQTT coverage verified beyond CI (local or otherwise)?

---

Reviewed by KrAIs (claude-sonnet-4-6) integrating Gemini/agy findings.