fix(contracts): migrate storage.mode to storage.backend for contract kit 0.9.0 - #28
Conversation
…kit 0.9.0 @hasna/contracts 0.9.0 removed the storage.mode enum (local | cloud) and replaced it with a required storage.backend (sqlite | postgresql). The storage object is additionalProperties:false, so the manifest was not merely stale — it was invalid on arrival against the kit this package depends on. The dependency was also unresolvable: @hasna/contracts sat at ^0.4.2, and on 0.x a caret range cannot cross a minor, so no published 0.9.0 could satisfy it. Changes: - package.json: @hasna/contracts ^0.4.2 -> ^0.9.0 (runtime dependency, not dev) - hasna.contract.json: storage.mode "local" -> storage.backend "sqlite", and storage.engines ["sqlite","postgresql"] to carry the postgres capability the removed enum used to imply - hasna.contract.json: kitVersion 0.4.2 -> 0.9.0 - hasna.contract.json: serviceSurfaces added — 0.9.0 requires class:"service" repos to declare at least one surface. api/mcp/cli are supported; the SDK surface is deferred with a reason, because it ships as the sibling package @hasna/economy-sdk and the v1 contract models SDK surfaces only as an exportSubpath of the same package - hasna.contract.json: hosting ["user-hosted"] - src/contract-manifest.test.ts: regression guard. Nothing in this repo read hasna.contract.json — no test, no build step, no CI job — which is why the declaration was free to drift. The guard asserts no storage-scoped validation issue and that kitVersion tracks the installed kit. The subpath import @hasna/contracts/auth used by src/server/serve.ts, src/server/index.ts and src/db/cloud.ts survives 0.9.0 unchanged; typecheck and the full suite are green. Known remaining, and deliberately NOT addressed here: `contracts validate` still reports bins.3 — the bin "economy-otel" is outside the allowlist that @hasna/contracts hardcodes (<name>, -cli, -mcp, -serve, -worker, -runner, -daemon, -migrate, -doctor). That failure reproduces identically against kit 0.4.2, so it predates this migration. Resolving it needs either a rename of a published binary or a widening of the allowlist upstream, neither of which belongs in a dependency bump. Verification: - bun run typecheck: clean - bun test: 362 pass, 0 fail across 42 files - regression guard against the parent manifest: 0 pass, 4 fail Refs: todos b89b27db Agent: Silvanus
|
[REVIEW] GO — #28 @ af1f846 — lens: regression-test-honesty, reviewer economy-pr28-reviewer (1 of 1) Lens: does the regression test actually constrain what this PR changed, or was it scoped to dodge a failure it could not fix? Answer: it does constrain the change, and the scoping is honest — but it is avoidably narrower than it needed to be. Details and the one recommended follow-up below. Independently re-measured in my own worktree; I did not reuse the author's numbers. The scoping question, settledThe narrow scoping is not evasive. Three independent measurements say so. 1. The guard genuinely fails on the parent manifest, for the right reasons. Parent manifest + kit 0.9.0 installed — real assertion diffs, not an import error or a missing file: At head: 2. The 3. It is genuinely pre-existing. Confirmed independently against kit 0.4.2 in a clean probe dir: The second arm is worth noting on its own: it proves the dependency bump is mandatory, not cosmetic. The two kits cannot both be satisfied by one manifest. F1 — P2, non-blocking: the guard leaves the PR's largest addition unconstrainedThe filter is Demonstrated by corrupting only PR-authored content ( The guard is blind to it. A stricter assertion — tolerate only the known So it can fail on regression and does pass on the PR as it stands. This matters because the PR's own stated rationale is that unguarded declarative content silently drifts — and What I verified as true, not plausible filler
Non-blocking residuals
What I did NOT checkServer routes verified by source read and the existing Verdict: GO. The migration is correct, mandatory, and properly constrained where it counts; F1 is a recommended tightening, not a blocker. |
|
[REVIEW] GO — #28 @ af1f846 — lens: correctness+security+gates, reviewer unresolved-account001 (1 of 1) Reviewed the exact PR head against fetched What I ran:
What I read:
Blocking P0/P1 findings: none. The dependency resolves to 0.9.0, the runtime auth subpaths still resolve and typecheck, the new storage declaration satisfies the 0.9.0 storage schema, and the declared gates are green. Non-blocking follow-up:
|
…eclares (#29) fix(server): make the runtime speak the 0.9.0 backend vocabulary it declares PR #28 moved economy's manifest to @hasna/contracts 0.9.0 — storage.mode removed, storage.backend added — but left the runtime speaking the retired vocabulary. This closes that divergence. - /health, /ready and /version now emit the three contract shapes; the shared envelope that leaked `mode` into all three is split, and the extra `service` key is dropped because HealthResponseSchema is strict. - The server backend resolves from DSN presence alone. Rejection of the retired storage-mode variables is delegated to the contract's own assertNoLegacyStorageMode so the migration hint matches the gate verbatim. - Local DSN resolution is retained rather than adopting the contract resolver wholesale, because that resolver does not read the bare DATABASE_URL economy has always honoured; adopting it would silently downgrade such a deployment to sqlite. - Dockerfile, Dockerfile.runtime and docker-compose.yml all baked the retired variable in and would have refused to boot against the fail-closed guard. - docs/configuration.md no longer instructs operators to set the now-fatal variable, and names all four retired aliases. The client half still accepts it and the docs say so explicitly. One passing assertion was deleted (serve.test.ts, `mode` defined) and replaced by schema conformance. Removing a green assertion is deliberate here and follows the owner directive of 2026-07-29 removing deployment modes. Reviewed: [REVIEW] GO at 12f4d84, lens boot-safety+client-boundary, independent fresh-context reviewer. It re-ran both arms and corrected the PR body's evidence claim: the parent arm is 0 pass / 6 fail / 1 error, and one of the two files fails at import rather than on value. Head 14/0, suite 376/0, tsc clean all reproduced. KNOWN AND DELIBERATE: the client half is unchanged and still reads the retired variable. The reviewer tested that intermediate state and found no new silent-wrong-store path — the one config where the variable is load-bearing already threw before this PR. DEPLOYMENT CAVEAT: out-of-repo deployment definitions are UNMEASURED. Any environment still setting HASNA_ECONOMY_STORAGE_MODE will fail closed on deploy — loudly, with a migration hint, but it will not start. Merging changes no running system. Agent: Silvanus
What
Migrates
@hasna/economyonto@hasna/contracts0.9.0, whose storage modelreplaced the
storage.modeenum (local|cloud) with a requiredstorage.backend(sqlite|postgresql).Two independent problems were in play, and both are fixed here:
@hasna/contractswas pinned at^0.4.2. On0.xa caret range cannot cross a minor, so no published0.9.0could ever satisfy it.storageisadditionalProperties: falsein 0.9.0, so a manifest still declaringmodeis rejected outright, not merely treated as stale.
@hasna/contractsis a runtime dependency here (dependencies, notdevDependencies), unlike several sibling packages —src/server/serve.ts,src/server/index.tsandsrc/db/cloud.tsall import the@hasna/contracts/authsubpath. That subpath still exists in 0.9.0 and typechecks unchanged.
Changes
package.json@hasna/contracts^0.4.2→^0.9.0hasna.contract.jsonstorage.mode: "local"→storage.backend: "sqlite"hasna.contract.jsonstorage.engines: ["sqlite","postgresql"]addedhasna.contract.jsonkitVersion0.4.2→0.9.0hasna.contract.jsonserviceSurfacesadded (0.9.0 requires it forclass: "service")hasna.contract.jsonhosting: ["user-hosted"]addedsrc/contract-manifest.test.tsbun.lockstorage.enginesis optional, and it is here on purpose: the removedmode: local | cloudenum implicitly carried the fact that this package supportsPostgres as well as SQLite.
backendalone loses that;enginesis the fieldthat carries it in the new model.
The SDK surface is declared
deferredwith a reason rather thansupported. A supported SDK surface requiresexportSubpath, and economy's SDKships as the sibling package
@hasna/economy-sdkrather than as a subpathexport of
@hasna/economy. Declaring anexportSubpathwould have been a falsestatement about the package layout.
Why a regression guard
Nothing in this repo read
hasna.contract.json— no test, no build step, andno CI job. That is exactly why the declaration was free to drift out of sync with
the pinned contract kit, and why a green suite said nothing about manifest
validity.
The guard is deliberately narrow: it asserts that the manifest raises no
storage-scoped validation issue, and that
kitVersiontracks the installedkit. It does not assert whole-manifest validity, because that check could not
pass today (see below) — and a check that cannot pass is worth no more than one
that cannot fail.
Known remaining — pre-existing, deliberately not fixed here
contracts validate hasna.contract.jsonstill reports one error:This reproduces identically against contract kit 0.4.2, so it predates this
migration and is not caused by it:
The allowlist is a fixed suffix set hardcoded in
@hasna/contracts. Resolving itneeds either renaming a published binary (breaking for anyone invoking
economy-otel) or widening the allowlist upstream. Neither belongs in adependency bump.
Note also that
repo-conformanceshort-circuits onmanifest_valid, so thedeeper conformance checks for this repo cannot run until that bin question is
settled.
Verification
Regression guard exercised in both directions — against the parent commit's
manifest with 0.9.0 installed:
and on this branch:
Storage errors present before the change and absent after:
Baseline note
The suite on the parent commit reported
357 pass, 1 fail— a 5000 ms timeout insrc/cli/index-help.test.tsunder a station load average of29.37on 20 cores.Re-run in isolation on the same commit it returned
21 pass, 0 fail. That failureis load contention, not a defect, and it is unrelated to this change.
Refs: todos
b89b27dbNeed help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.