Skip to content

fix(upgrade): create hdb_deployment on 5.0.x→5.1 upgrades, and run upgrade directives without confirmation - #1126

Merged
sleekmountaincat merged 3 commits into
mainfrom
fix/hdb-deployment-directive-5-1-0
Jun 4, 2026
Merged

fix(upgrade): create hdb_deployment on 5.0.x→5.1 upgrades, and run upgrade directives without confirmation#1126
sleekmountaincat merged 3 commits into
mainfrom
fix/hdb-deployment-directive-5-1-0

Conversation

@sleekmountaincat

@sleekmountaincat sleekmountaincat commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Two related fixes that unblock the 5.0.x → 5.1 upgrade path:

  1. Directive mis-versioned (5.2.05.1.0). The hdb_deployment table-creation directive was tagged 5.2.0, so it never ran on a 5.0.x → 5.1.x upgrade (directives run only when current < directive_version <= upgrade). The table was never created on upgraded installs → replicated deploy_component failed on peers ("Deployment tracking is not initialized on this node"). Fresh installs were unaffected (table comes from systemSchema.json via mount_hdb).

  2. Upgrade directives no longer require interactive confirmation. Because (1) made a directive actually apply, startup began prompting [CONFIRM_UPGRADE] on stdin — which hangs, or with no TTY defaults to "no" and refuses to start, breaking unattended/scripted starts (systemd, containers, CI). upgrade() only runs when a directive applies (verified: getVersionUpdateInfo returns an object solely when hasUpgradesRequired is true; no non-directive path triggers it), so directive upgrades now run automatically with a non-blocking notice (release-notes link retained). Downgrades still confirm (forceDowngradePrompt).

Changes

  • upgrade/directives/5-2-0.ts5-1-0.ts, version 5.2.05.1.0; controller import/registration updated; regression tests in unitTests/upgrade/directivesController.test.js.
  • bin/upgrade.js — removed the blocking upgrade-confirmation prompt; directives run automatically via a non-blocking notice.
  • upgrade/upgradePrompt.ts — removed the now-unused forceUpdatePrompt (forceDowngradePrompt kept).
  • unitTests/bin/upgrade.test.js — removed the obsolete (skipped) upgrade() prompt tests; runUpgrade() tests unchanged.
  • DESIGN.md — documented the directive-versioning rule + comparator caveat.

⚠️ Known: downgrade and start integration subtest is currently red (separate core fix in progress)

After a real 5.x upgrade, the 4.x binary can't read the upgraded system DB and refuses to start. Root-caused to a core storage bug: system.__dbis__ metadata records were saved with the wrong encoder setting, producing msgpackr-2.x records that 4.x's msgpackr 1.x can't decode (the main data flow is unaffected). @kriszyp is fixing this in core; the subtest goes green once that lands. It is not caused by this PR — this PR surfaced it by making the upgrade actually run. Byte-level investigation in the comments.

Beta caveat (directive gate)

utility/common_utils.ts:compareVersions strips trailing .0, sorting 5.1.0-beta.1 above 5.1.0. So 5.0.x → 5.1.x GA and 5.0.x → 5.1.0-beta.1 both fire the directive ✅, but an install already on 5.1.0-beta.x upgrading to GA does not ❌. The GA customer base (the reported impact) is covered.

Possible follow-up (not in this PR)

A cross-model review (Gemini) suggested that if a future directive performs a heavy/irreversible migration, we may want confirmation back for that case only — done safely: add a directive metadata flag (e.g. requiresConfirmation), and for those, prompt if a TTY is present, else fail fast with a clear message (set FORCE_AUTO_UPGRADE=true to proceed) instead of hanging. Not needed for the trivial hdb_deployment directive; noted for when a risky directive is introduced.

Testing

  • unitTests/upgrade/directivesController.test.js → 5 passing; unitTests/bin/** → 75 passing.
  • Docker e2e (4.x→5.1): upgrade and start / upgrade and migrate pass with no prompt (plain env: {}); downgrade and start red pending the core fix above.
  • npm run build / lint (--deny-warnings) / format clean.

🤖 Generated with Claude Code (agent: Claude Opus 4.8). Attention: the downgrade subtest red-state is expected and tracked to @kriszyp's core metadata-encoding fix; the confirmation removal was verified (directive-only) by code trace + a cross-model review.

…upgrades create the table

The deployment-recorder feature ships in 5.1.0 and depends on
system.hdb_deployment, but its table-creation upgrade directive was tagged
5.2.0. Directives only run when current_version < directive_version <=
upgrade_version, so on every 5.0.x → 5.1.x upgrade the 5.2.0 directive was
filtered out and the table was never created. Replicated deploy_component then
failed on peer nodes ("Deployment tracking is not initialized on this node"),
affecting the entire existing customer base. Fresh 5.1 installs were unaffected
because the table comes from systemSchema.json via mount_hdb.

Renumber the directive (5-2-0.ts → 5-1-0.ts, version 5.1.0) so it runs on the
5.0.x → 5.1.x upgrade path, and add regression tests pinning the directive to
the release that first ships the dependent code.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@sleekmountaincat

sleekmountaincat commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Investigation notes (canonical) — what CI surfaced and why

Renumbering the directive made a 5.0.x → 5.1 upgrade actually run for the first time, which surfaced two things in integrationTests/upgrade/4.x-upgrade.test.ts:

1. Startup confirmation prompt → fixed in this PR

A real upgrade hit the interactive [CONFIRM_UPGRADE] prompt on stdin and hung (or, with no TTY, defaulted to "no" and refused to start). Verified the prompt was exclusively a directive-upgrade gate — upgrade() only runs when getVersionUpdateInfo returns an object, which happens solely when hasUpgradesRequired is true; no non-directive path triggers it (confirmed by code trace + a cross-model review). Fix: directive upgrades run automatically with a non-blocking notice; downgrades still confirm.

2. downgrade and start subtest fails → separate core fix, owned by @kriszyp

Starting the 4.x binary on data a 5.x upgrade has touched fails with a misleading "old instance no longer supported." This is not a property of this PR and not a "downgrade is unsupported" situation — it's a fixable core storage bug:

  • Root cause (confirmed with @kriszyp): the system.__dbis__ metadata records were saved with the wrong encoder setting, producing msgpackr-2.x records. The main data flow is fine — only those metadata records are affected.
  • Mechanism (byte-level, from an instrumented v4 run): v4 (msgpackr 1.11.8) can't decode the v5-written __dbis__ records. A 4.x-written record leads with 0x40 (its record marker); the mis-encoded v5 record leads with 0x20 (= 32 — literally the "end of buffer not reached 32" v4 throws). v4 then loads an empty system DB, defaults its data version to 2.9.9, and checkIfInstallIsSupported throws because 2.9.9 < 3.0.0. So the user-facing error is a red herring.
  • Status: @kriszyp is fixing the encoder setting in core. This subtest is expected-red until that lands, then it goes green. We're keeping it (not skipping) so it verifies his fix.

(Correction to an earlier version of this comment: I'd framed the downgrade failure as a fundamental msgpackr v1↔v2 incompatibility / "downgrade unsupported." That was wrong — the wire format is fine; it's a specific wrong-setting bug on the metadata records, which is fixable.)


Investigation by an AI agent (Claude Code); the directive-only confirmation finding and the byte-level root cause were each cross-checked with a second model.

Now that the hdb_deployment directive is versioned 5.1.0, a 4.x->5.x upgrade
runs a real upgrade directive, so v5 startup prompts [CONFIRM_UPGRADE] on stdin
and blocks until the test's startup timeout. Pass CONFIRM_UPGRADE=yes to the v5
starts (mirroring the existing CONFIRM_DOWNGRADE=yes in the downgrade subtest)
so the upgrade proceeds non-interactively.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this should constitute a requirement to confirm upgrade (which can be problematic for scripts that start harper that aren't expected that). Can we remove the requirement for upgrade confirmation for directives?

@sleekmountaincat

Copy link
Copy Markdown
Contributor Author

I don't think this should constitute a requirement to confirm upgrade (which can be problematic for scripts that start harper that aren't expected that). Can we remove the requirement for upgrade confirmation for directives?

yes ill take a look at that

A directive-driven upgrade runs on the normal `harper run` startup path, where
the upgrade-confirmation prompt (forceUpdatePrompt) blocked on stdin — or, with
no TTY, defaulted to "no" and refused to start — breaking unattended/scripted
starts (systemd, containers, CI). `upgrade()` only ever runs when an upgrade
directive applies (getVersionUpdateInfo returns an object solely when
hasUpgradesRequired is true), so this prompt was exclusively a directive-upgrade
gate. Remove it; directives now run automatically with a non-blocking notice
that keeps the release-notes link. Downgrades still confirm (forceDowngradePrompt).

- bin/upgrade.js: drop the forceUpdatePrompt gate + cancel/exit branch.
- upgrade/upgradePrompt.ts: remove the now-unused forceUpdatePrompt.
- unitTests/bin/upgrade.test.js: remove the obsolete (skipped) upgrade() prompt
  tests + the vars/imports they owned; runUpgrade() tests unchanged.
- integrationTests/upgrade/4.x-upgrade.test.ts: revert the CONFIRM_UPGRADE=yes
  workaround now that startup no longer prompts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sleekmountaincat sleekmountaincat changed the title fix(upgrade): version hdb_deployment directive to 5.1.0 so 5.0.x→5.1 upgrades create the table fix(upgrade): create hdb_deployment on 5.0.x→5.1 upgrades, and run upgrade directives without confirmation Jun 4, 2026
@sleekmountaincat
sleekmountaincat requested a review from kriszyp June 4, 2026 04:30

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Excellent!

@sleekmountaincat

sleekmountaincat commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

CI status — which red checks belong to this PR (TL;DR: none of the code-level ones)

Re-ran all failing jobs to separate real signal from baseline. Result:

Check Status Attribution
Unit Test (Node v22)re-delivered duplicate must not double-apply the commutative op ❌ reproduced Pre-existing on main (latest main run fails Unit Test on v22 and v26). Replication/CRDT test, untouched by this PR.
Integration Tests 2/4 (Bun + Node 22/24/26) — crash with random byte flips in txnlogs ❌ reproduced Pre-existing on main (same jobs red on the latest main run). Chaos/fuzz test, unrelated to this PR.
Integration Tests 1/4 (Node 22/24/26) — downgrade and start ❌ expected The known item: 4.x can't read v5-written system.__dbis__ metadata (wrong encoder setting); @kriszyp is fixing in core. Goes green when that lands.

Net: no failure here is caused by this PR's code. This PR's own tests pass — directivesController regression (5), runUpgrade() unit tests, and the upgrade and start / upgrade and migrate integration subtests (confirming directive upgrades now run unattended with no prompt).

Heads-up for reviewers: main is currently red on the Unit (CRDT) and Integration 2/4 (chaos) suites independent of this branch, so a fully-green check set here depends on those baseline failures + Kris's downgrade fix, none of which are in scope for #1126.

Posted by an AI agent (Claude Code).

@sleekmountaincat
sleekmountaincat merged commit 266d5d8 into main Jun 4, 2026
57 of 74 checks passed
@sleekmountaincat
sleekmountaincat deleted the fix/hdb-deployment-directive-5-1-0 branch June 4, 2026 17:57
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.

3 participants