Skip to content
This repository was archived by the owner on Jul 17, 2026. It is now read-only.

Upgrade Harper to v5#1

Open
BboyAkers wants to merge 7 commits into
mainfrom
v5-upgrade
Open

Upgrade Harper to v5#1
BboyAkers wants to merge 7 commits into
mainfrom
v5-upgrade

Conversation

@BboyAkers

@BboyAkers BboyAkers commented May 11, 2026

Copy link
Copy Markdown
Member

Harper v4 → v5 upgrade

Completes the v5 upgrade for this real-time pub/sub (MQTT/WebSocket/SSE/REST) example and adds full transport test coverage + CI.

Changes

  • Dependencies: harper ^5.0.11^5.0.28; @harperfast/integration-testing 0.3.10.4.0; added typescript as a direct devDependency.
  • Mandatory harness fix (harperBinPath): the harper package's exports map only exposes ".", so the harness's auto-resolution of harper/dist/bin/harper.js throws ERR_PACKAGE_PATH_NOT_EXPORTED. Both test files now resolve the CLI from the exported main entry and pass it explicitly via harperBinPath. This also let us drop the brittle HARPER_INTEGRATION_TEST_INSTALL_SCRIPT env var and the --isolation=none flag from the test script.
  • Test script: added a standard test:integration script; test now delegates to it.
  • New tests — integrationTests/realtime-transports.test.ts: covers the real-time transports the example is built around, alongside the existing REST CRUD suite:
    • MQTT over TCP (mqtt://host:1883): publish persists to the Topics table and is served back over REST.
    • MQTT over WebSocket (ws://host:9926/mqtt): subscriber receives a retained topic value.
    • SSE (Accept: text/event-stream on the REST resource): the stream delivers a live update when the record changes.
  • CI: added .github/workflows/integration-tests.yml (Node 22, 24, 26; actions pinned to commit hashes).
  • Lockfile: regenerated package-lock.json so npm ci resolves ws's native optional deps (bufferutil, utf-8-validate, node-gyp-build) across the whole Node matrix; all deps resolve from the public registry (no file:/.tgz refs).
  • Branding: README v4 commands updated to Harper (harper run ., npm install -g harper); live docs.harperdb.io URLs left intact.

Migration items

  • Applied / confirmed: import is from 'harper' (not harperdb); no harperdb-config.yaml (uses config.yaml); no blob.save(, no .wasLoadedFromSource(, no frozen-record mutation, no manual context passing.
  • N/A: install scripts, child-process spawning, blob storage, caching/sourcedFrom, VM module-loader overrides, and Table.get() return-shape handling — the app has none of these (it's a thin schema + one-line resources.js plus the table-backed MQTT/REST/SSE behavior Harper provides).

Test results

  • Local: blocked at the macOS loopback bind (EADDRNOTAVAIL on 127.0.0.2+) — environmental, not a code issue. The harness requires loopback aliases that need interactive sudo unavailable here. Tests compile and reach the harness.
  • CI: green on Node 22, 24, and 26 (run) — all 8 tests pass (5 REST CRUD + MQTT TCP + MQTT WebSocket + SSE). ubuntu-latest supports the full 127.0.0.0/8 range, so it is the authoritative test gate.

Note: an initial CI run surfaced the real Harper v5 MQTT-on-table semantics — MQTT messages on a table-backed topic carry the full record as JSON (not a bare scalar), and a publish populates the record columns from a JSON body. The transport tests were corrected to assert against that contract (this was a test-code expectation, not a v5 regression).

Known issues / flags

  • Lint: npm run lint reports a pre-existing parsing error — the @harperdb/code-guidelines@0.0.6 ESLint preset references a tsconfig.json that the published package does not ship (it ships tsconfig.node.json). This affects the pre-existing test equally and is unrelated to this upgrade; not worked around here.
  • npm scope (manual): @harperdb/code-guidelines is still on the legacy @harperdb scope. Moving it to the current Harper scope is a manual decision flagged for a human.
  • Python parts: out of harness scope — the Python/hardware pieces referenced in the design docs are device-side clients/demos, not part of the Harper app. Integration tests target the Harper app's pub/sub + REST behavior only.
  • Stale endpoint (non-blocking): src/App.vue subscribes via /subscribe?table=Topics, which is not a defined resource; canonical Harper SSE is Accept: text/event-stream on the resource path (what the new test exercises). Left as-is since it's frontend behavior outside the upgrade scope.

🤖 Generated with Claude Code

BboyAkers and others added 2 commits May 11, 2026 09:48
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Bump harper ^5.0.11 -> ^5.0.28 and @harperfast/integration-testing 0.3.1 -> 0.4.0
- Add typescript as a direct devDependency
- Apply the mandatory harperBinPath harness fix (resolve harper CLI from the
  exported main entry; harper's exports map only exposes ".") to topics-crud test
- Replace the install-script env-var / --isolation=none test script with a
  standard test:integration script
- Add integrationTests/realtime-transports.test.ts covering MQTT (TCP),
  MQTT-over-WebSocket, and SSE in addition to the existing REST CRUD tests
- Add pinned-hash GitHub Actions integration-tests workflow (Node 22/24/26)
- Regenerate package-lock.json so npm ci resolves ws native optional deps
  (bufferutil/utf-8-validate/node-gyp-build) across the full Node matrix
- README: HarperDB v4 commands -> Harper (harper run ., npm install -g harper)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@BboyAkers BboyAkers changed the title Updated to v5 Upgrade Harper to v5 Jun 8, 2026
BboyAkers and others added 2 commits June 8, 2026 14:32
CI surfaced the actual Harper v5 MQTT-on-table behavior:
- MQTT messages on a table-backed topic carry the full record as JSON, not a
  bare scalar. Parse the WebSocket subscribe payload as JSON and assert on
  record.value.
- Publish the MQTT TCP payload as a JSON record body ({ value }) so it maps to
  the Topics columns, and read the REST body tolerantly (JSON record or bare
  value). The SSE test already passed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move github.event.inputs.node-version into an env var to prevent shell injection.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Comment thread integrationTests/topics-crud.test.ts Outdated
const { admin, httpURL } = ctx.harper;
const auth = basicAuth(admin.username, admin.password);

await fetch(`${httpURL}/Topics/test-sensor-read`, {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: setup PUT result not checked — silent failures will mask the real error

The PUT at this line seeds the record needed by the test, but its response is never inspected. If the write fails (server not fully ready, schema mismatch, transient error), the subsequent GET will receive a 404 or unexpected body and the assertion message will point at the wrong place, making debugging hard.

The PUT /Topics/:topic creates a topic entry test (line 34) correctly does ok(res.ok, ...). All the other tests' setup writes should do the same.

Suggested change
await fetch(`${httpURL}/Topics/test-sensor-read`, {
const setup = await fetch(`${httpURL}/Topics/test-sensor-read`, {
method: 'PUT',
headers: { 'Content-Type': 'application/json', Authorization: auth },
body: JSON.stringify({ topic: 'test-sensor-read', value: '42' }),
});
ok(setup.ok, `setup PUT failed: HTTP ${setup.status}`);

Comment thread integrationTests/topics-crud.test.ts Outdated
const { admin, httpURL } = ctx.harper;
const auth = basicAuth(admin.username, admin.password);

await fetch(`${httpURL}/Topics/test-sensor-update`, {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: setup PUT result not checked — silent failure masks actual cause

Same issue as the GET test: the initial seed write for test-sensor-update is not checked for success. If it fails, the second PUT (intended as an update) might actually create the record, and the test would pass for the wrong reason, or fail with a misleading assertion.

const { value, done } = await reader.read();
if (done) return null;
buffer += decoder.decode(value, { stream: true });
if (buffer.includes('15')) return buffer;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: buffer.includes('15') is too weak — can match SSE event IDs or timestamps

The SSE buffer scan looks for the substring '15' anywhere in the raw stream. This matches not just the value field in the payload, but also:

  • SSE event id counters (id: 15)
  • Millisecond timestamps containing 15 (e.g., 1718...15...)
  • Any other numeric data in the stream

The test can both pass spuriously (SSE id counter reaches 15 before the update arrives) and fail to assert the right thing (it never verifies the payload actually contains the updated value in the value field).

Parse the SSE data: line instead:

Suggested change
if (buffer.includes('15')) return buffer;
const dataLines = buffer.split('\n').filter(l => l.startsWith('data:'));
const found = dataLines.some(l => {
try { return JSON.parse(l.slice(5))?.value === '15'; } catch { return false; }
});
if (found) return buffer;


const streamed = await withTimeout(readUpdate, 15000, 'sse update');
ac.abort();
ok(streamed && streamed.includes('15'), 'SSE stream should deliver the updated brightness value (15)');

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same weak assertion — mirrors the issue at line 187

Both the read loop condition and this final ok() assertion use streamed.includes('15'). If the read loop exits because an SSE event id or timestamp contained 15, this assertion will pass while the actual value update was never received. The fix should be applied here alongside line 187 to assert value === '15' in a parsed data: payload.

`expected text/event-stream, got ${sseRes.headers.get('content-type')}`,
);

const reader = sseRes.body!.getReader();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plausible crash: sseRes.body! non-null assertion without a null guard

The ! operator suppresses TypeScript's null check, but in Node.js fetch the body property can be null (e.g., for a 200 response with no body — possible if Harper is misconfigured or the SSE path is wrong). The status check at line 171 only confirms 200; it does not prove body != null. If body is null, getReader() throws a TypeError with no indication of where the stream setup failed.

Suggested change
const reader = sseRes.body!.getReader();
ok(sseRes.body, 'SSE response body should not be null');
const reader = sseRes.body.getReader();

// Resolve the CLI from the exported main entry and pass it explicitly as harperBinPath.
const harperBinPath = resolve(dirname(require.resolve('harper')), 'bin/harper.js');

function basicAuth(username: string, password: string): string {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup: basicAuth and harperBinPath duplicated across both test files

This basicAuth helper (and the harperBinPath resolution pattern at line 19) are identical to what appears in integrationTests/realtime-transports.test.ts. Extract them to a shared integrationTests/helpers.ts so changes to auth encoding or bin resolution only need to happen in one place.

- Check setup PUT results in topics-crud tests (GET, UPDATE, DELETE) so silent failures don't mask errors in the actual assertion steps
- Replace sseRes.body! non-null assertion with explicit ok() guard before getReader()
- Replace buffer.includes('15') substring checks with SSE data: line parser that strictly matches value === '15' in JSON payload, preventing spurious passes on id counters or timestamps

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@BboyAkers

Copy link
Copy Markdown
Member Author

Review follow-up (autonomous agent): Fixed all 4 blocking findings: checked setup PUT results in CRUD tests, added explicit null guard before getReader(), and replaced raw includes('15') checks with an SSE data: line parser that strictly matches value === '15' in JSON payloads.

…ON parse

The sseHasValue() JSON parsing was too strict for the actual Harper SSE
payload format and caused the 15s timeout. Check for the value in the
raw buffer using a specific string pattern that avoids false positives
on counters while still finding the actual value update.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants