GrayCodeAI · Patel230 · Aug 15, 2026 · Aug 15, 2026
diff --git a/plans/PHASE1-SQLITE-HARDENING.md b/plans/PHASE1-SQLITE-HARDENING.md
@@ -0,0 +1,224 @@
+# Feature Specification: Phase 1 SQLite Hardening
+
+**Status:** Implemented — merged via five PRs (#47–#51), all CI green
+**Author:** Patel230
+**Date:** 2026-08-15
+**Repos affected:** `GrayCodeAI/yaad` (via hawk submodule `external/yaad`)
+
+## Problem Statement
+
+Yaad's persistence layer (`storage/`) is a single SQLite database (`~/.yaad/data/yaad.db`) holding agent memory. The audit identified five hardening gaps:
+
+1. **No cross-process lock** — two yaad processes can open the same DB concurrently; WAL serializes writes but nothing prevents a second process from starting.
+2. **No schema versioning** — `createTables()` is one monolithic DDL blob; schema evolution has to be hand-stitched and is not repeatable.
+3. **Minimal PRAGMA tuning** — only `journal_mode(WAL)`, `foreign_keys(ON)`, `busy_timeout` are set (storage/sqlite.go:259). No checkpoint tuning, mmap, cache sizing, or `PRAGMA optimize`; zero observability into storage behaviour.
+4. **No vector index** — similarity search over embeddings is a linear scan via `AllEmbeddings`/cosine in Go (storage/vectors.go).
+5. **No backup** — no way to take a consistent snapshot of the memory DB.
+
+## Proposed Solution
+
+Five independently reviewable branches off `main`, merged strictly in order:
+
+| Order | Branch | PR | Content |
+|---|---|---|---|
+| 1 | `feat/storage-process-lock` | #47 | flock(2) cross-process lock |
+| 2 | `feat/storage-migrations` | #48 | versioned, transactional schema migrations |
+| 3 | `feat/storage-pragma-metrics` | #49 | PRAGMA tuning + Prometheus metrics |
+| 4 | `feat/storage-hnsw` | #50 | HNSW approximate nearest-neighbor index |
+| 5 | `feat/storage-backup` | #51 | `VACUUM INTO` online backup |
+
+All changes are additive; no existing behaviour changes for current DB files (migrations bring forward-only changes via `CREATE … IF NOT EXISTS` / `ALTER` with defaults).
+
+## Design Details
+
+### 1. Process lock (`feat/storage-process-lock`, PR #47)
+
+**New files (platform-split — CI builds windows/amd64 and `unix.Flock`
+does not exist on Windows):**
+- `storage/lock_unix.go` (`//go:build !windows`) — `ProcessLock` struct;
+  `AcquireProcessLock(dbPath)` opens `dbPath+.lock` (0600) and takes a
+  non-blocking `unix.Flock` `LOCK_EX|LOCK_NB` (golang.org/x/sys). `Release()`
+  unlocks, closes, best-effort removes the lock file. In-memory DSNs return
+  a no-op lock.
+- `storage/lock_windows.go` (`//go:build windows`) — same API backed by
+  `windows.LockFileEx` with `LOCKFILE_EXCLUSIVE_LOCK|LOCKFILE_FAIL_IMMEDIATELY`
+  for equivalent non-blocking exclusive semantics; `UnlockFileEx` best-effort
+  in `Release()`.
+
+**Modified files:**
+- `storage/sqlite.go`
+  - `Store` struct (:231): add `processLock *ProcessLock`
+  - `NewStore()` (:240): acquire lock right after directory creation (:251), release on any later error path
+  - `Close()` (:305): release the lock
+
+**Acceptance criteria:**
+- Second `NewStore` on the same path fails fast with `"database locked by another yaad process"`; first store unaffected.
+- Lock is released on `Close()` — a fresh `NewStore` after `Close()` succeeds.
+- `:memory:` DSNs never touch the lock path.
+- No new goroutines, no polling — pure flock(2).
+
+**Tests:** `TestProcessLock` in `storage/sqlite_test.go` (open store A → expect error from store B → close A → reopen succeeds).
+**Gate:** `GOWORK=off go test ./storage/... -run TestProcessLock -v`
+
+### 2. Migrations (`feat/storage-migrations`, PR #48)
+
+**New file:**
+- `storage/migrate.go` —
+  - `type Migration struct { Version int; Up func(*sql.Tx) error }`
+  - `var migrations []Migration` (ordered, idempotent):
+    | v | change |
+    |---|--------|
+    | 1 | full base `schema` DDL |
+    | 2 | `idx_nodes_type_project_scope` composite index |
+    | 3 | `node_metadata` table + `idx_node_metadata_key_value` |
+    | 4 | `embeddings_hnsw` table + `idx_embeddings_hnsw_model` |
+    | 5 | `nodes.encrypted` / `nodes.encryption_key_version` columns |
+  - `(s *Store) Migrate(ctx)` — reads `MAX(version)` from `schema_version` (0 if absent), runs pending migrations in one transaction, records each applied version.
+
+**Modified files:**
+- `storage/sqlite.go`
+  - schema version table added to base DDL in `createTables()` (:362): `CREATE TABLE IF NOT EXISTS schema_version (version INTEGER PRIMARY KEY, applied_at DATETIME DEFAULT CURRENT_TIMESTAMP)`
+  - `NewStore()` (:282): call `s.Migrate(ctx)` after `createTables()`, before `newStmtCache`
+
+**Acceptance criteria:**
+- Fresh DB: all 5 migrations applied exactly once; re-open is a no-op.
+- Existing pre-migration DB: opens cleanly; pending migrations applied in order inside a tx (atomic — partial failure leaves version unchanged).
+- `PRAGMA integrity_check` clean after migration.
+
+**Tests:** migration idempotency + partial-failure rollback in `storage/sqlite_test.go` (`TestMigrations*`).
+**Gate:** `GOWORK=off go test ./storage/... -run TestMigration -v`
+
+### 3. PRAGMA tuning + metrics (`feat/storage-pragma-metrics`, PR #49)
+
+**New files:**
+- `storage/metrics.go` — promauto-registered instruments (`yaad_storage_ops_total{op,result}`, `yaad_storage_query_duration_seconds{op}`, plus WAL/db size gauges); one `observe()` helper wrapping hot paths.
+- `applyPragmas(db *sql.DB) error` — executed once per connection via `db.Conn(ctx)` so pooled conns inherit the settings:
+
+  | PRAGMA | Value | Why |
+  |---|---|---|
+  | `synchronous` | `NORMAL` | safe under WAL, ~2x write throughput |
+  | `wal_autocheckpoint` | `1000` (pages) | prevent unbounded WAL growth |
+  | `temp_store` | `MEMORY` | temp tables/sorts off disk |
+  | `mmap_size` | `268435456` (256 MiB) | read-heavy workload |
+  | `cache_size` | `-32768` (32 MiB) | larger page cache |
+  | `page_size` | `4096` | matches default; explicit for determinism |
+  | `recursive_triggers` | `ON` | cascade correctness |
+  | `foreign_keys` | `ON` | (also set via DSN; belt-and-braces) |
+  | `optimize` | run on `Close()` | lets SQLite update planner stats |
+
+**Modified files:**
+- `storage/sqlite.go`
+  - DSN (:259): keep existing three pragmas (they only stick via DSN on modernc.org/sqlite for all pooled connections)
+  - `NewStore()`: after `PingContext` (:264), call `applyPragmas(db)`
+  - `Close()` (:305): `PRAGMA optimize` before closing
+- `go.mod`: add `github.com/prometheus/client_golang` (already fetched via `go get`)
+- Hot paths (sqlite.go/sqlite_nodes.go/sqlite_edges.go read/write helpers): wrap with metrics observe
+
+**Acceptance criteria:**
+- `PRAGMA synchronous` reports `1` (NORMAL) on every pooled connection (`SELECT` via fresh connections from the 5-conn pool).
+- `yaad_storage_ops_total` increments per operation; histogram observes latency.
+- No behaviour change for callers; metrics are read-only observability.
+
+**Tests:** `TestPragmas` (assert pragmas on multiple pooled conns), `TestMetricsRecorded` (run ops, gather from prometheus registry, assert counters > 0).
+**Gate:** `GOWORK=off go test ./storage/... -run 'TestPragmas|TestMetrics' -v`
+
+### 4. HNSW vector index (`feat/storage-hnsw`, PR #50)
+
+**New file:**
+- `storage/hnsw.go` —
+  - `HNSWIndex{vectors, graph map[string]map[int][]string, maxLevel, entryPoint, efConstruction=200, m=16, efSearch=50, rng}`
+  - `Build(ctx, s, model)` — loads `AllEmbeddings(ctx, model)`, inserts in sorted ID order (deterministic build), persists graph via `persist()` into `embeddings_hnsw` (upsert)
+  - `insert()` — greedy entry-point descent from top level, heap-based `searchLayer` (min-heap candidates / max-heap results), neighbor pruning to `m`
+  - `Search(query, k, ef)` — level descent then layer-0 beam search, returns IDs + cosine scores descending
+  - `cosineSim(a, b []float32)` shared with vectors.go semantics
+
+**Modified files:**
+- `storage/interface.go`: `BuildHNSWIndex(ctx, model) error`, `SearchHNSW(ctx, model, query, k, ef) ([]string, []float32, error)` added to `Storage`
+- `storage/sqlite.go`:
+  - `Store` struct (:231): `hnswMu sync.Mutex`, `hnswIndexes map[string]*HNSWIndex` (per-model — vectors from different embedding models occupy incompatible spaces)
+  - `createTables()` (:473, after `embeddings`): `embeddings_hnsw` table (`node_id PK`, `vector BLOB`, `model TEXT`, `neighbors TEXT` JSON, `updated_at`)
+- `storage/vectors.go`: `Store.BuildHNSWIndex` / `Store.SearchHNSW` (build-on-demand under `hnswMu`)
+- `storage/sqlite_tx.go`: both methods return `errors.New("hnsw … not supported inside a transaction")` — index lives outside txs and is rebuilt on demand
+- `storage/mock.go`, `storage/interface_test.go` (mockStorage), `engine/engine_mock_storage_test.go` (engine mockStorage): implement the two new methods
+
+**Acceptance criteria:**
+- `SearchHNSW` returns exact-match vector first, scores in descending order, `len(ids)==k` when corpus ≥ k.
+- Per-model isolation: building model B doesn't disturb model A's results.
+- Transaction rejection: `SearchHNSW` inside `WithTx` returns a clear unsupported error.
+- Deterministic within a process (sorted insertion order + seeded rng).
+
+**Tests:** `TestHNSWIndex` in `storage/sqlite_test.go` (10-node corpus, angle-distinct vectors — NOT collinear, since cosine ties break ranking; exact-match query; second model build-on-demand).
+**Gate:** `GOWORK=off go test ./storage/... -run TestHNSWIndex -v`
+
+### 5. Backup (`feat/storage-backup`, PR #51)
+
+**New file:**
+- `storage/backup.go` — `Store.Backup(ctx, backupPath)`:
+  1. Validate path non-empty; `os.MkdirAll(dir, 0o700)`
+  2. `VACUUM INTO ?` to a temp file (`backupPath.<nanos>.tmp`) via `s.withTimeout`
+  3. `Chmod` temp to 0600, then `os.Rename` into place (atomic)
+  4. On any error: remove the temp file
+
+  `VACUUM INTO` reads the live DB through WAL without blocking writers beyond a moment and emits a compact standalone file — no need to copy `-wal`/`-shm`.
+
+**Modified files:**
+- `storage/interface.go`: `Backup(ctx context.Context, backupPath string) error` added to `Storage` (documented: atomic temp+rename, owner-only)
+- `storage/sqlite_tx.go`: `txStore.Backup` returns `"backup is not supported inside a transaction"`
+- `storage/mock.go`: `MockStorage.Backup` writes an empty file at the path (honours injected error)
+- `storage/interface_test.go`, `engine/engine_mock_storage_test.go`: no-op `Backup` on both test mocks
+
+**Acceptance criteria:**
+- Backup file exists, non-empty, permissions exactly 0600.
+- Backup is a standalone consistent DB: `NewStore(backupPath)` opens it and reads seeded nodes back.
+- Empty path → error; no partial file left on failure.
+- Rejected inside `WithTx`.
+
+**Tests:** `TestBackup`, `TestBackupEmptyPath`, `TestBackupInsideTx` in `storage/backup_test.go`.
+**Gate:** `GOWORK=off go test ./storage/... -run TestBackup -v`
+
+## Implementation Plan
+
+### Phase 1 (this spec) — DONE, merged
+- [x] `feat/storage-process-lock` — PR #47 (`b30fa11`)
+- [x] `feat/storage-migrations` — PR #48 (`58d3eef`)
+- [x] `feat/storage-pragma-metrics` — PR #49 (`09ccc71`)
+- [x] `feat/storage-hnsw` — PR #50 (`6c03254`)
+- [x] `feat/storage-backup` — PR #51 (`dfedbce`)
+
+### Phase 2 (future, out of scope here)
+- Incremental HNSW updates on `SaveEmbedding`/`DeleteEmbedding` (currently full rebuild)
+- Persist-and-restore of the HNSW graph across restarts (read `embeddings_hnsw`)
+- Backup rotation / scheduled backups from the daemon
+- Apply encryption columns (migration v5) with a real key provider
+
+## Testing Strategy
+
+Per-branch gates above, plus after each merge:
+
+```bash
+# Yaad (submodule — parent go.work must be detached)
+GOWORK=off gofmt -l .            # must print nothing
+GOWORK=off go vet ./...
+GOWORK=off go test ./...          # full suite, ~30s
+
+# Hawk parent — must still build after storage/ interface changes
+make build   # from hawk repo root
+```
+
+All five branches individually pass the full `GOWORK=off go test ./...` suite (verified 2026-08-15).
+
+## Risks & Mitigations
+
+| Risk | Impact | Mitigation |
+|------|--------|------------|
+| Merge-order conflicts: later branches each carried a copy of the lock files (`lock_unix.go`/`lock_windows.go`, split for Windows CI) before #47 landed; `sqlite.go`/`sqlite_test.go` touched by several branches | med | Merged strictly #47→#48→#49→#50→#51; resolved by keeping the union (all use `IF NOT EXISTS` / additive edits) |
+| flock unavailable on some platforms (e.g. NFS mount) | low | `AcquireProcessLock` returns wrapped error; host can fall back to advisory lock later |
+| `VACUUM INTO` requires temp space ≈ DB size | low | Documented; backup path is caller-controlled |
+| HNSW full rebuild cost grows with corpus | med | efConstruction/m constants tuned; Phase 2 adds incremental updates |
+| `modernc.org/sqlite` DSN pragma quirks | low | Per-connection pragmas applied via `db.Conn(ctx)` in `applyPragmas`; DSN kept for pool-wide settings |
+
+## References
+
+- PRs: [#47](https://github.com/GrayCodeAI/yaad/pull/47) · [#48](https://github.com/GrayCodeAI/yaad/pull/48) · [#49](https://github.com/GrayCodeAI/yaad/pull/49) · [#50](https://github.com/GrayCodeAI/yaad/pull/50) · [#51](https://github.com/GrayCodeAI/yaad/pull/51)
+- SQLite docs: [WAL](https://www.sqlite.org/wal.html), [VACUUM INTO](https://www.sqlite.org/lang_vacuum.html), [PRAGMA optimize](https://www.sqlite.org/pragma.html#pragma_optimize)
+- HNSW paper: Malkov & Yashunin, arXiv:1603.09320