refactor(storage): drop the retired @hasna/cloud dependency - #19
Merged
Conversation
The shared cloud runtime is retired, so the usage-ledger backends it supplied now live in the repo. src/storage.ts loaded both of its adapters behind the config-driven storage.cloud.backend switch, so both halves are replaced rather than removed: - src/db/sqlite-adapter.ts wraps bun:sqlite and keeps both pragmas the retired adapter set. foreign_keys is per-connection and defaults to OFF in SQLite, so losing it would not raise an error anywhere -- it would silently turn every ON DELETE CASCADE into a no-op. - src/db/pg-adapter.ts wraps pg and rewrites the ledger statements' `?` placeholders into Postgres' numbered form, normalizes bindings, and keeps the "encrypt without verifying" TLS handling for sslmode=require. pg becomes a direct dependency; it was already installed and already bundled into dist through the retired package, so the resolved tree only shrinks. The public storage.cloud config surface is unchanged, and both adapters stay behind lazy imports so the postgres driver is only loaded when a postgres ledger is configured. Removing the dependency also removes it from the published artifact: bun build inlined the whole retired package -- sync daemon included -- into all four dist entry points, each of which now drops ~0.25 MB. Verified: bun run check exits 0 with 232 tests passing (210 before), a --frozen-lockfile install plus typecheck/test/build reproduce green from a pristine tree, and no-cloud-scan on the tracked tree goes from exit 1 with two critical findings to exit 0.
…t output The guard scanned src/**/*.ts only, so a literal import of the retired shared cloud runtime from scripts/ or tests/ passed it, and it never looked at build output — the one place this repo actually was shipping the retired package from. - scan every file git tracks (git ls-files, this guard file excepted) instead of src/, and fail loudly if git ls-files errors rather than scanning nothing - scan dist when a build is present, permitting exactly one occurrence: the FORBIDDEN_SHARED_CLOUD_RUNTIMES declaration that bun build inlines from @hasna/contracts. Any other occurrence is a real bundled edge and fails - build before test in bun run check, prepublishOnly and CI so that assertion runs instead of skipping - note in the guard docstring that byte matching sees literals only, so a specifier assembled at runtime is a known blind spot Also in this commit: - pg-adapter: extract resolveLastInsertRowid and refuse a non-numeric id. The ledger declares id TEXT, so a future RETURNING id would have handed a uuid string through a number | bigint annotation with no error. Covered by three new cases in tests/pg-adapter.test.ts - docs/publishing-and-release.md: the no-cloud boundary check is implemented, so stop describing it as pending; record why the external artifact scan cannot be a hard release gate yet (it reports the inlined denylist constant as critical on a packed tarball) and why a built-output finding must still be read rather than waved off bun run typecheck, bun run build, bun test all exit 0 (236 pass, 0 fail). Guard non-vacuity: a literal import staged under scripts/ fails it, the same under tests/ fails it, and an injected dist edge fails it with file:line.
Contributor
Author
|
Merged as
Verification on the merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Removes the retired
@hasna/clouddependency.src/storage.tsdynamically loadedboth of its adapters behind the config-driven
storage.cloud.backendswitch, so bothhalves are replaced in-repo rather than dropped:
src/db/sqlite-adapter.tsSqliteAdapterbun:sqlitesrc/db/pg-adapter.tsPgAdapterAsyncpgpgbecomes a direct dependency. It was already installed transitively and alreadybundled into
distthrough the retired package, so the resolved tree only shrinks.Lockfile net effect: two packages removed (
@hasna/cloud,@hasna/events— thelatter transitive-only, and referenced nowhere in
src,testsordocs) and oneadded (
@types/pg@8.20.0, a dev type package).The public
storage.cloudconfig surface is unchanged —backend: "sqlite" | "postgres"both still work, no config migration, no breaking change for consumers. Both adapters stay
behind lazy
await import(...)so the Postgres driver is only loaded when a Postgresledger is actually configured.
Two details that are load-bearing
PRAGMA foreign_keys=ONis preserved. It is per-connection in SQLite and defaults toOFF, so dropping it raises no error anywhere — it silently turns every
ON DELETE CASCADEand every foreign key constraint into a no-op.
tests/sqlite-adapter.test.tsproves thepragma is set, proves a cascade actually deletes child rows, and proves a dangling
reference is rejected. Deleting the pragma line makes 3 of those tests fail, so the guard
is not vacuous.
The
?->$1, $2, ...placeholder rewrite is preserved. Postgres has no?placeholder form, and the ledger
INSERTbinds 14 columns; an off-by-one would bind thewrong value to every column after the mistake. That exact 14-binding statement is asserted
in
tests/pg-adapter.test.ts.The rewrite is deliberately a placeholder rewrite only, not the retired package's full
dialect translator. Every statement reaching the adapter is written in
src/storage.ts(the DDL, two
CREATE INDEX, oneINSERT, oneSELECT) and placeholders are the onlyconstruct among them that Postgres spells differently. Notably
REALis left alone —the retired adapter's
exec()path did not rewrite it either, so this stays like-for-likeand existing deployed tables are unaffected.
lastInsertRowidexists only to mirrorbun:sqlite'sRunResult. Postgres has no rowid,so
resolveLastInsertRowidreports 0 unless a statement returns a numericid. Theledger declares
id TEXT, so a futureRETURNING idwould otherwise have pushed a uuidstring through a
number | bigintannotation with no error anywhere.The published artifact was carrying the retired package
This is the part that is not visible from the manifest. On
main,bun buildinlinedthe entire retired package — sync daemon, sync push/pull, dialect translator and all —
into all four
distentry points, reachable from one dynamic import. Every entry pointdrops ~0.25 MB:
dist/index.jsdist/cli/index.jsdist/mcp/index.jsdist/serve.jsRetired sync symbols (
syncPush,syncPull,translateDdl,sqliteToPostgres,runSync,scheduled-sync,HASNA_CLOUD*) now match 0 times in the bundle and 0times in the packed tarball, and the
node_modules/@hasna/cloudbundle banner is gonefrom all four entry points (it was present in all four before).
The boundary guard now covers what it claimed to
tests/no-cloud-boundary.test.tsoriginally scannedsrc/**/*.tsonly, which let aliteral
import "@hasna/cloud"fromscripts/ortests/pass, and it never looked atbuild output — the exact failure mode the section above is about. It now:
git ls-files, this guard file excepted), and throwsrather than scanning nothing if
git ls-filesfails;distwhen a build is present, permitting exactly one occurrence: theFORBIDDEN_SHARED_CLOUD_RUNTIMESdeclarationbun buildinlines from@hasna/contracts. Anything else is a real bundled edge and fails, withfile:line;bun run buildinbun run check,prepublishOnlyand CI, so thebuilt-output assertion executes instead of skipping;
specifier assembled at runtime is invisible to it (as it is to every byte scanner).
Non-vacuity, measured: a literal import staged under
scripts/fails it (exit 1), the sameunder
tests/fails it (exit 1), an injecteddistedge fails it withdist/serve.js:13379(exit 1), and with no build present the built-output assertion reports skip, not pass.
Verification
All exit codes measured unpiped (
cmd >log 2>&1; echo $?).bun install --frozen-lockfilebun install(non-frozen)bun.lockdriftbun run typecheckbun run buildbun testmainis 210 pass / 17 files)bun run check(typecheck + build + test)bun run contracts:validatebun dist/cli/index.js validate --config gateway.config.production-cloud.example.jsonStructural proof that nothing survives:
package.json— 0 occurrencesbun.lock— 0 occurrences (@hasna/cloud,open-cloud)@hasna/cloud/open-cloud/hasna-cloudnode_modules/@hasna/after a from-scratch install — onlycontracts(a plainbun installdid not prune the stale directory, so this was re-verified from anempty
node_modules); the onlycloud-matching directory left ispg-cloudflare,an optional dep of
pgdistand thenpm packtarball — one occurrence each indist/index.js:17728anddist/cli/index.js:16081, both the linevar FORBIDDEN_SHARED_CLOUD_RUNTIMES = [...]vendored from
@hasna/contracts. Zero import or require edges.Note on
no-cloud-scan(measured with@hasna/contracts@0.8.1)git archive HEAD)ok hasna.no_cloud_evidence_pack.v1main, tracked tree, same tool and methodpackage_manifest package.json,lockfile bun.lock) + 2 high (package_manifest,source_import src/storage.ts)highondist/index.js+dist/cli/index.jsnpm pack)criticalaspacked_artifactThe pass on the tracked tree is therefore discriminating, not vacuous — same tool, same
method, opposite result on
main.The two exit-1 rows are a false positive, and a new class beyond the comment/guard-test
one fixed in contracts #32: the scanner reads a repo's own build output, and
@hasna/contractsis itself bundled intodist, so its denylist constantFORBIDDEN_SHARED_CLOUD_RUNTIMES = ["@hasna/cloud", "open-cloud"]is inlined verbatim.That string exists nowhere in this repo's source (
git grep FORBIDDEN_SHARED_CLOUDovertracked files finds only the guard test that whitelists it). Any repo that bundles
@hasna/contractswill hit this, and the packed-artifact form escalates it tocritical,which is the form a release lane would run.
Consequences, now recorded in
docs/publishing-and-release.mdso a future reader does nothave to rediscover them: do not gate the release on
no-cloud-scanagainstdistoragainst an
npm packtarball until the scanner skips build output or exempts thatdeclaration — run it against the tracked tree, and rely on
tests/no-cloud-boundary.test.tsfor built output. And do not wave off a built-outputfinding as "just the vendored constant" without reading it; that excuse is true for exactly
one line and false for everything else, which is why the test encodes the distinction
instead of leaving it to eye.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.