refactor(storage)!: remove deployment-mode branching — collapse local|hybrid|remote to a sqlite|postgresql backend - #15
Merged
Conversation
…ith a sqlite|postgresql backend Deployment location was never a property of the data layer. `StorageMode` named three places something could run; nothing in this repo ever branched on it. Its only two readers were `getStorageStatus()` and its own test, so the enum bought vocabulary and no behaviour. The real defect was underneath it. `normalizeStorageMode` returned `undefined` for any value it did not recognise, and `getStorageMode` then fell through to `getStorageDatabaseUrl() ? "hybrid" : "local"`. Measured on origin/main: HASNA_HOOKS_STORAGE_MODE=self_hosted -> rc=0, "Mode: local" HASNA_HOOKS_STORAGE_MODE=mysql -> rc=0, "Mode: local" A typo and the fleet's own canonical mode word were both silently normalised into a mode the operator did not ask for, and reported back as if configured. Now: - `StorageBackend = "sqlite" | "postgresql"` replaces `StorageMode`. `local` collapses to sqlite; hybrid, remote, self_hosted, self-hosted and cloud all collapse to postgresql, because they only ever differed in who operated the server. - `HASNA_HOOKS_STORAGE_BACKEND` (fallback `HOOKS_STORAGE_BACKEND`) selects it. `sqlite3`, `postgres` and `pg` are accepted aliases. Any other value throws and names the accepted set. - `HASNA_HOOKS_STORAGE_MODE` and `HOOKS_STORAGE_MODE` are no longer read. Setting either throws, naming the replacement variable and the backend that value maps to, rather than being ignored. Deleting the words while keeping the silent fall-through would have removed the symptom and kept the hole. - Inference is unchanged: unset plus a database URL yields postgresql (previously reported `hybrid`); unset with no URL yields sqlite (`local`). Breaking for API consumers: `StorageStatus.mode` becomes `.backend`, the CLI prints `Backend:`, and `StorageMode` / `getStorageMode` / `HOOKS_STORAGE_MODE_ENV` / `HOOKS_STORAGE_MODE_FALLBACK_ENV` / `STORAGE_MODE_ENV` are unexported. A workspace scan found no importer of `@hasna/hooks` outside this repo, and the rename makes a stale reading of the field a compile error rather than a silent mismatch, since the values changed meaning too. Hook evaluation is untouched: no hook and no part of the prompt path reads the backend. No client-side PostgresStore is introduced or retained. `PgAdapterAsync` remains reachable only from the operator-invoked `hooks storage push|pull|sync` commands; the live data layer is bun:sqlite only.
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.
Removes the three-way deployment-mode branching from
open-hooksper the owner directivedirectives/2026-07-29-owner-directive-remove-deployment-modes.md. Do not merge without anadversarial review pass. No
@hasna/contractsbump; no npm publish.Enumerated surface — before and after
open-hookswas never scoped by this programme (not in the directive's table, not in thedesign knowledge
k_ms3e6v41_zbe7m8scope list, which classes hooks-like local CLIs as N/A).Two corrections to the scoping it was given:
hasna.contract.jsondoes not exist in this repo. There is no manifest surface, no@hasna/contractsdependency, and no vendoredstorage-kit. 100% of the branching here washand-rolled and repo-local — which answers directive §7's open question for this repo.
src/, so it missedREADME.md; and the pattern useddouble-quoted
"hybrid"with no bareremote, soREADME.md:112— whereremotesat aloneon a wrapped line — was invisible. A real line-wrapping false negative.
Verified before: 5 files / 25 lines. After: 0 live mode surfaces. Every remaining
occurrence of the retired words is the rejection machinery, its tests, or documentation of the
retirement.
src/db/storage-sync.tsStorageMode = "local" | "hybrid" | "remote";HOOKS_STORAGE_MODE_ENV;HOOKS_STORAGE_MODE_FALLBACK_ENV;STORAGE_MODE_ENV;StorageStatus.mode;normalizeStorageMode(silentundefined);getStorageMode(silent fallback)StorageBackend = "sqlite" | "postgresql";STORAGE_BACKENDS;HOOKS_STORAGE_BACKEND_ENV;STORAGE_BACKEND_ENV;StorageStatus.backend;getStorageBackend(throws);RETIRED_STORAGE_MODE_ENVrejection tablesrc/storage.ts,src/index.tsStorageModetypeStorageBackendtypesrc/cli/index.tsx:1230Mode: ${status.mode}Backend: ${status.backend}src/storage.test.tsREADME.md:110-112, 117Words kept, because they are not the concept
76 occurrences of
remote; 71 are legitimate and untouched —git remote(inhook-permissionguard,hook-gitguard,worktree-guard,codewith-native-common), "remote codeexecution" rule descriptions,
ssh host '…'remote-command parsing, andremoteas the localvariable/filename for the Postgres connection (a location word the directive explicitly keeps;
not renamed, to avoid a gratuitous public-export path change).
localis kept throughout. Thesingle
cloud(README:117) was a mode surface and is rewritten.CHANGELOG.md's existing"Remote layers…" and repos-lookup entries are historical record and left byte-identical.
The actual defect, and what replaces it
Nothing in this repo ever branched on the mode.
getStorageMode()had exactly two callers —getStorageStatus()and its own test — so the enum bought vocabulary and no behaviour. Thedefect was underneath:
normalizeStorageModereturnedundefinedfor anything it did notrecognise, and
getStorageModethen fell through togetStorageDatabaseUrl() ? "hybrid" : "local".Measured on
origin/main(worktree atorigin/main, unpipedrc):A typo and the fleet's own canonical mode word were both silently turned into a mode the
operator never asked for, then reported back as if configured. On this branch:
Deleting the words while keeping the silent fall-through would have removed the symptom and kept
the hole, so the retired names are rejected with the fix named, not merely absent.
local→sqlite;hybrid/remote/self_hosted/self-hosted/cloud→postgresql.sqlite3,postgresandpgare accepted aliases of the two canonical values.Behaviour preservation
Inference is unchanged: with the variable unset, a configured database URL yields
postgresql(previously reported as
hybrid) and its absence yieldssqlite(previouslylocal). Nothingrouted on the mode, so no routing changed.
Hook evaluation is untouched. No hook, and no part of the prompt path, reads the backend —
getStorageBackend()is reachable only fromgetStorageStatus()(hooks storage statusand thestorage_statusMCP tool). This matters because hooks run on every agent prompt on this fleet.One behaviour deliberately not preserved, called out rather than hidden:
StorageStatus.modeis renamed to
.backendand the mode symbols are unexported, which is breaking for an APIconsumer. Keeping the field name while changing what its values mean would let a consumer testing
mode === "local"fail silently; the rename makes it a compile error or an obvious absence. Aworkspace-wide scan found no importer of
@hasna/hooksoutside this repo.CHANGELOG.mdcarries the full before/after under a
### ChangedBREAKING entry.Owner ruling already satisfied, nothing to remove: there is no client-side
PostgresStore.PgAdapterAsync(src/db/remote-storage.ts, 40 lines) is reached only viagetStoragePg(), usedonly by the operator-invoked
hooks storage push|pull|sync. The live data layer isbun:sqliteonly.
Verification
Base
origin/main=8d9b6ac. All exit codes measured unpiped (cmd > log 2>&1; rc=$?).Tests failed first.
src/storage.test.tswas rewritten before the implementation existed:rc=1,SyntaxError: Export named 'HOOKS_STORAGE_BACKEND_ENV' not found. After implementing:rc=0, 18 pass / 0 fail (up from 3 tests; adds both rejection paths, each retired value andeach retired env var as a
test.eachcase, the alias set, and an assertion that the removedexports are gone from the module surface).
Typecheck:
bunx tsc --noEmitrc=0;bunx tsc -p tsconfig.build.json --noEmitrc=0;bun run buildrc=0.Baseline established before attributing any failure — the suite is already red on
origin/main. Full suite on unmodifiedorigin/main:rc=1, 1025 pass / 13 fail. On thisbranch:
rc=1, 1032 pass / 18 fail. Zero storage-related failures in either. Every failure inboth runs is the subprocess-timeout flake family (
this test timed out after 5000ms, pluskilled N dangling processcascades) in the CLI install/update E2E, destructive-shell-guard andMCP-transport suites. Proven pre-existing rather than assumed: the working tree was
git stashedback to clean
origin/mainand the four affected test files re-run —rc=1, 8 failures,including
codewith native common helpers > managedWorktreeInfo … accepts ordinary directory names, one of the names that had looked new, and a different member of the sameparameterised E2E family (
Stop hook E2Ewhere the branch run hitPreToolUse hook E2E). Thefailing set is unstable run-to-run on unchanged code.
published ≠ installed ≠ running — checked, and there is no gap here: repo
main0.5.0,npm
@hasna/hooks0.5.0, installed CLI0.5.0. The running installed artifact reproduces thedefect (
HASNA_HOOKS_STORAGE_MODE=self_hosted hooks storage status→rc=0,Mode: local), somerging and publishing this will change what runs on the fleet.
Positive controls, because a grep returning 0 is not evidence: the surface pattern was
confirmed to detect a planted
deploymentMode: "self_hosted"in a git intent-added file (rc=0)before its 0-counts were trusted, and the staged secrets scanner was confirmed to fire on a
planted
sk-ant-value before the clean result over 483 staged diff lines was accepted. NoCo-Authored-Bytrailer.Further unscoped surfaces noticed — reported, not touched
14
iapp-*internal apps carry the same copy-pasted_STORAGE_MODEpattern and appear in noscope list (the directive and the design knowledge enumerate only
open-*repos plushasna/contracts):iapp-conversations,iapp-deployment,iapp-domains,iapp-files,iapp-identities,iapp-knowledge,iapp-marks,iapp-mementos,iapp-projects,iapp-secrets,iapp-sessions,iapp-skills,iapp-telephony,iapp-todos. Several(
iapp-projects,iapp-todos,iapp-mementos,iapp-knowledge) carry astorage-sync.tsstructurally identical to this repo's, which is where the silent-normalisation defect lives — so
the same fix likely applies verbatim.
iapp-knowledgeadditionally ships a builtdist/withthe vocabulary baked in. This is the same "missed repos it never examined" direction the directive
warns about, one directory tree over.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.