Skip to content

Report the Harper version in the startup options banner - #2263

Draft
DavidCockerill wants to merge 1 commit into
mainfrom
david/startup-log-version
Draft

Report the Harper version in the startup options banner#2263
DavidCockerill wants to merge 1 commit into
mainfrom
david/startup-log-version

Conversation

@DavidCockerill

@DavidCockerill DavidCockerill commented Aug 21, 2026

Copy link
Copy Markdown
Member

Adds a Version: field to the startup options banner, so the block an operator reads — or pastes into a support thread — identifies the build it came from. Sourced from the same packageJson.version the Harper <version> successfully started marker already uses, so the two cannot disagree.

Version:            5.2.4
Hostname:           stage-1.studio.harperfabric.com
Worker Threads:     4
Root Path:          /home/harperdb/harper

Until now the version reached stdout only via that marker, which sits above the ASCII logo and a variable number of boot lines, so any excerpt starting at the banner carried no build identity. Under IS_SCRIPTED_SERVICE the marker isn't printed at all (bin/run.ts:227), which makes the banner the only version on stdout for those installs.

For the human reviewer

  1. An integration boot to cover a one-line log field. integrationTests/server/startup-banner.test.ts boots Harper and reads its stdout; the alternative was a unit test over the exported startupLog with a stubbed portResolutions and a captured console.log, or no test at all. The unit version wouldn't prove the line actually reaches stdout from worker 0, which is the only property worth guarding. Cost is one boot per run — 3.9s observed, and the suite already boots Harper in most files. Reversible by deleting the file.

  2. The test's oracle is self-referential, deliberately. It compares the banner's version against the readiness marker's version. Both read the same in-process packageJson.version binding, so it guards presence, format and plausible shape, never the value — it cannot catch a wrong or stale version. The alternative, comparing against the resolved distribution's package.json, does prove the value but couples the assertion to whichever build the harness resolved (HARPER_INTEGRATION_TEST_INSTALL_SCRIPT, or a harper package in node_modules, can point elsewhere), so "wrong binary under test" would read as a banner regression. One line either way — say the word if you'd rather have the value check and accept that failure mode.

  3. Field position. Version: goes after the conditional Mode: READ-ONLY line and before Hostname:, so the yellow read-only alert stays the first thing you see. The alternative is making Version: the literal first line. One-line move, but this banner's field order is the de facto operator contract, so it's worth your opinion rather than my silent choice.

  4. Console-only, not hdb.log. The banner is a bare console.log, so with logging.stdStreams: false the new field — like every other banner field — never lands in hdb.log; I verified this on a local boot. A support engineer holding only a customer's hdb.log from such an install still can't read the version. Emitting through hdbLogger would fix that, but makes the version a log-format commitment the moment anything parses it, and duplicates it wherever stdStreams is on. I chose not to. Relatedly, the version now prints twice on stdout — banner plus marker — which is deliberate: the banner is the block people paste, the marker scrolls away.

Where to look hardest: readStartupStdout. startupOutput.stdout stops accumulating at the readiness marker, and the banner is printed from worker 0 — reaching the parent's stdout over the worker forwarding channel, which is not ordered against the main thread's own marker write. Either line can therefore be absent from that snapshot, which is why the helper polls stdout.log (always available: @harperfast/integration-testing/dist/run.js:80-83 defaults HARPER_INTEGRATION_TEST_LOG_DIR to a fresh temp dir when unset) and throws with the path and a tail rather than degrading silently.

Three review findings were raised and not acted on, each refuted against source rather than deferred: an ANSI-anchoring risk on the readiness regex (the harness ANSI-strips before both its snapshot and its disk write, harperLifecycle.js:304,320, so FORCE_COLOR never reaches what the test reads); a claim that npm run test:integration leaves HARPER_INTEGRATION_TEST_LOG_DIR unset (the runner defaults it, cited above); and a node:assert/strict conformance nit (.oxlintrc.json and AGENTS.md:196 forbid that import). That last one was a real contradiction in the repo, so it is fixed here: integrationTests/README.md no longer tells you to write an import the linter rejects, in both the requirements list and the template. No existing integration test imported /strict, so nothing else changes.

Verification

End-to-end route: new integration test, plus a live smoke.

  • npm run test:integration -- "integrationTests/server/startup-banner.test.ts" → 1 passing.
  • Fails-on-base: with the bin/run.ts line removed and dist rebuilt, the same test fails on the poll timeout, naming the log path. Restored and re-verified green.
  • Live smoke: booted dist/bin/harper.js run from this worktree against a scratch root; banner printed Version: 5.2.4, matching the Harper 5.2.4 successfully started line below it and package.json.
  • Confirmed the banner is absent from that boot's hdb.log under logging.stdStreams: false (decision 4) — console-only, unchanged from before.
  • prettier --check and oxlint --format stylish --quiet clean on both files.

Not run to completion locally: npm run test:unit:main. It hangs indefinitely on macOS in unitTests/components/applicationSpawn.test.js ("terminates a detached process tree when its owning worker is force-terminated") under the suite's --timeout 0. Reproduced identically on this branch and on a main checkout, so it is pre-existing and unrelated — filed separately. CI's unit matrix is the gate, and it is green on all three Node versions.

Complexity: medium

Review-Coverage: authored=claude; ran=gemini; adjudicated=domain; blocked=cursor-composer(not-installed),codex(exit-1); declined=cursor-grok; rounds=10 @ e86cf0e

Human-Review-Need: 3 (decisions: test-layer-choice, logdir-absent-policy, banner-field-order, version-surface, duplicate-version-emission, version-source) @ e86cf0e

@claude

This comment has been minimized.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the startup log in bin/run.ts to include the application version from packageJson.version. There are no review comments, and I have no feedback to provide.

@DavidCockerill
DavidCockerill force-pushed the david/startup-log-version branch from fa87964 to 96500b6 Compare August 21, 2026 20:12
The banner is the block operators paste into support threads and investigation
notes; the version only appeared in the separate "successfully started" line,
which is frequently outside the excerpt. Sourced from the same
packageJson.version that line uses so the two cannot disagree.
@DavidCockerill
DavidCockerill force-pushed the david/startup-log-version branch from 96500b6 to e86cf0e Compare August 21, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant