Skip to content

feat: state.db with encrypted auth, in-memory pi settings, models allowlist#35

Open
sasicodes wants to merge 5 commits into
feat/storage-pr1-repoint-pifrom
feat/storage-pr2-state-db
Open

feat: state.db with encrypted auth, in-memory pi settings, models allowlist#35
sasicodes wants to merge 5 commits into
feat/storage-pr1-repoint-pifrom
feat/storage-pr2-state-db

Conversation

@sasicodes
Copy link
Copy Markdown
Owner

Summary

PR 2 of the storage migration plan (see STORAGE-PLAN.md). Stacks on top of #34 (PR 1) — base is feat/storage-pr1-repoint-pi. Will retarget to main once #34 lands.

Introduces <baseDir>/state.db (better-sqlite3 + WAL) and replaces three of Pi's default storage backends with custom ones. After this PR, the only JSON file anywhere under <baseDir>/ is gone — auth/settings/models/app-state all live in SQLite or in code.

what changes

  • main/db.tsopenStartDb() opens <baseDir>/state.db with WAL/NORMAL/mmap/cache PRAGMAs, runs idempotent migrations for schema_version, app_state, and auth tables. Prepared statements cached.
  • main/storage.ts — backs readStartState/writeStartState/updateStartState onto the app_state kv table (one row per logical field). Public API unchanged so call sites and tests are untouched. ~/.start/state.json no longer exists.
  • main/auth-backend.tsKeychainAuthBackend implements AuthStorageBackend. Single-row store (provider='__all__'). safeStorage.encryptStringdecryptString round-trips the credential JSON; ciphertext lives in auth.ciphertext BLOB. Returns undefined gracefully if safeStorage isn't ready yet so the constructor-time AuthStorage reload never crashes; a later refreshAuth() picks up real creds.
  • main/settings-backend.tsInMemorySettingsBackend implements the SettingsStorage shape (withLock(scope, fn)). Pi defaults (compaction, retry, transport, etc.) apply at runtime; no settings.json ever written.
  • main/models.ts — hardcoded allowlist of { provider, id } pairs (Claude 4.x, GPT-5.x, Gemini 3.x). helpers.ts now imports the lists from this file. When a new model ships, add a line and release.
  • chat.tsChatService wires the new backends; threads settingsManager into all three createAgentSession call sites.

what we drop in this PR

Dropped Replaced by
<baseDir>/state.json state.db.app_state table
<baseDir>/agent/auth.json state.db.auth + safeStorage
<baseDir>/agent/settings.json InMemorySettingsBackend (Pi defaults in code)
<baseDir>/agent/models.json main/models.ts constant

Test plan

  • pnpm check — lint, format, typecheck all green
  • pnpm test — 132/132 desktop tests passing (added FakeAuthStorage.fromStorage, FakeSettingsManager.fromStorage, mocked @main/db)
  • Manual smoke: launch packaged build, verify ~/.start/state.db is created on first launch and contains only the three tables; verify no auth.json/settings.json/models.json/state.json ever appears
  • Manual smoke: log into a provider via OAuth or API key, restart app, verify auth survives (round-trip through safeStorage + DB)
  • Manual smoke: open state.db in a hex viewer or sqlite3 ".dump auth" — confirm ciphertext is opaque bytes, not plaintext tokens
  • Manual smoke: launch dev build, verify only ~/.start-dev/state.db is touched; prod tree unchanged

next

PR 3 — sessions pointer table + fast recents + archive flag. Will branch from this PR.

@sasicodes sasicodes force-pushed the feat/storage-pr2-state-db branch from bf8e76b to 5ad16f9 Compare May 25, 2026 08:47
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 25, 2026

Greptile Summary

This PR (storage migration step 2) introduces state.db (better-sqlite3 + WAL) and replaces Pi's default JSON-file backends with three custom implementations: KeychainAuthBackend (encrypted creds in SQLite BLOB via safeStorage), InMemorySettingsBackend (Pi defaults in code), and a SQLite-backed app_state kv store replacing state.json. The models.ts allowlist is extracted from helpers.ts into its own module, and ChatService is wired with the new backends.

  • db.ts: Opens state.db with WAL/NORMAL pragmas; runs idempotent transactional migrations for schema_version, app_state, and auth tables.
  • auth-backend.ts: Single-row encrypted credential store with a promise-queue mutex for async serialization; readCurrent() silently returns undefined when safeStorage is unavailable, but does not guard against decryptString throwing on corrupted ciphertext.
  • storage.ts: Module-level cachedStatements is prepared against the initial DB connection but is never cleared when closeStartDb() is called — a post-close state write would use stale statements against a new connection.

Confidence Score: 3/5

Two real defects need fixing before merge: unguarded decryptString in the auth backend and stale prepared statements after DB close.

The auth backend's readCurrent() calls safeStorage.decryptString() without a try/catch — a corrupted ciphertext or keychain key rotation will throw and propagate unhandled into Pi's auth layer. In storage.ts, the module-level cachedStatements is never invalidated when closeStartDb() resets cachedDb; any state write that fires after DB close (e.g., a pending IPC handler during window teardown) will open a new connection and then execute old statements against it, causing a better-sqlite3 cross-connection error that writeStartState does not catch.

packages/desktop/src/main/auth-backend.ts (readCurrent decryptString error path) and packages/desktop/src/main/storage.ts (cachedStatements lifecycle)

Important Files Changed

Filename Overview
packages/desktop/src/main/auth-backend.ts New KeychainAuthBackend wiring safeStorage encryption to SQLite BLOB; async queue mutex is correctly implemented, but readCurrent() missing try/catch around decryptString — a corrupted or cross-keychain ciphertext would crash instead of gracefully returning undefined.
packages/desktop/src/main/db.ts New openStartDb/closeStartDb singleton with WAL pragmas and transactional migrations; migration correctness is solid, but closeStartDb() only clears cachedDb without a way to signal storage.ts to invalidate its own cachedStatements.
packages/desktop/src/main/storage.ts Migrated from JSON file to SQLite kv table; module-level cachedStatements is never cleared by closeStartDb(), so a post-close write opens a new DB connection but executes statements compiled on the old one, causing a cross-connection error.
packages/desktop/src/main/chat.ts Wires new backends into ChatService; settingsManager threaded into all three createAgentSession call sites; closeStartDb() called in cleanup path.
packages/desktop/src/main/settings-backend.ts Simple in-memory Map-backed SettingsStorage; synchronous withLock with correct undefined-means-delete semantics.
packages/desktop/src/main/models.ts Hardcoded allowlist extracted from helpers.ts into its own module; Sets and arrays pre-computed at module load; no logic changes.

Sequence Diagram

sequenceDiagram
    participant App as Electron Main
    participant CS as ChatService
    participant DB as db.ts
    participant ST as storage.ts
    participant AB as KeychainAuthBackend
    participant SS as safeStorage

    App->>CS: new ChatService()
    CS->>DB: openStartDb()
    DB-->>CS: Database instance
    CS->>AB: new KeychainAuthBackend(db)
    CS->>CS: AuthStorage.fromStorage(backend)
    CS->>CS: SettingsManager.fromStorage(backend)

    Note over ST: First state access
    App->>ST: readStartState()
    ST->>DB: openStartDb()
    DB-->>ST: cachedDb
    ST->>ST: prepare and cache statements
    ST-->>App: StartState

    Note over AB: Auth read
    CS->>AB: withLockAsync(fn)
    AB->>SS: isEncryptionAvailable()
    SS-->>AB: true
    AB->>AB: readStmt.get
    AB->>SS: decryptString(ciphertext)
    SS-->>AB: credentials JSON

    Note over CS: Cleanup path
    CS->>DB: closeStartDb()
    DB->>DB: close and clear cachedDb
    Note over ST: cachedStatements still set to old connection
Loading

Reviews (2): Last reviewed commit: "chore: drop unused resetAppStateCache ex..." | Re-trigger Greptile

Comment thread packages/desktop/src/main/pi/auth.ts
Comment thread packages/desktop/src/main/storage.ts Outdated
Comment thread packages/desktop/src/main/storage.ts
Comment thread packages/desktop/src/main/pi/auth.ts
sasicodes added 5 commits May 25, 2026 15:31
Native dep for PR 2 (state.db). Added better-sqlite3 to pnpm
onlyBuiltDependencies so its install script runs and the platform binary
is fetched/built.
…owlist

PR 2 of the storage migration plan.

- main/db.ts: openStartDb() opens <baseDir>/state.db with WAL + NORMAL sync
  + mmap + cache PRAGMAs, runs idempotent migrations for schema_version,
  app_state, and auth tables. Prepared statements cached.
- main/storage.ts: readStartState/writeStartState/updateStartState back onto
  the app_state kv table; one row per logical field (composer_shortcut,
  last_workspace, selected_model_key, selected_thinking_level, session_notices,
  workspace_bookmarks). Public API unchanged so call sites and tests are
  untouched. ~/.start/state.json is no longer read or written.
- main/auth-backend.ts: KeychainAuthBackend implements AuthStorageBackend.
  Single-row store ('__all__' provider). safeStorage.encryptString round-trips
  the credential JSON; ciphertext lives in the auth.ciphertext BLOB column.
  Returns undefined gracefully if safeStorage isn't ready yet so constructor-
  time reload() never crashes; a later refreshAuth() picks up real creds.
- main/settings-backend.ts: InMemorySettingsBackend implements the
  SettingsStorage shape (withLock(scope, fn)). Pi defaults apply at runtime;
  no settings.json is ever written.
- main/models.ts: hardcoded allowlist of provider/id pairs (Claude 4.x,
  GPT-5.x, Gemini 3.x). When a new model ships, add a line and release.
  helpers.ts now imports the lists from this file.
- chat.ts: ChatService uses AuthStorage.fromStorage(new KeychainAuthBackend),
  ModelRegistry.create(authStorage) (built-ins only — no models.json),
  SettingsManager.fromStorage(new InMemorySettingsBackend()). Threads
  settingsManager into all three createAgentSession call sites.

Tests: FakeAuthStorage.fromStorage / FakeSettingsManager.fromStorage stubs.
@main/db mocked to return a no-op stub so ChatService construction works
without touching disk. 132/132 tests pass.

After this change, no JSON config files exist anywhere under <baseDir>/agent/
or <baseDir>/state.json. The only on-disk state outside session JSONLs is
<baseDir>/state.db (+ -wal/-shm sidecars).
…stmts

Greptile flagged two issues:

1. withLockAsync had a real concurrency gap. It read the current
credential blob, awaited the caller-provided fn, then wrote — letting
two concurrent calls each base their next-blob on the same stale read
and silently drop one of the writes. In production this would bite
during background OAuth token refresh, where two refresh calls can
overlap. Fixed with a serialization chain (a Promise-chained queue
that ensures one withLockAsync invocation finishes its persist before
the next one reads).

2. storage.ts was preparing fresh SQL statements on every helper call
(SELECT, INSERT, DELETE), violating the project's "prepare once at
module load" rule. Hoisted the three statements into a cached
AppStateStatements object resolved lazily on first use, exposed
resetAppStateCache() for tests/dev.
- resetAppStateCache had no callers; per "don't add features beyond the
  task" rule, drop the export. If a future test needs to clear caches
  we'll add it back when there's a caller.
- ChatService.dispose now calls closeStartDb so the singleton DB is
  closed cleanly on app shutdown (PRAGMA optimize runs, WAL is
  checkpointed). Wires the lifecycle that was already partially set up.
…ests

agents.md / user-flagged cleanups on PR 2:

File naming:
- main/auth-backend.ts -> main/pi/auth.ts
- main/settings-backend.ts -> main/pi/settings.ts
Drops the redundant "-backend" suffix and groups Pi-SDK adapter
implementations under a dedicated pi/ folder so future additions land
beside their siblings.

Dev/prod auth split:
- Introduces an AuthCodec abstraction (encode/decode/available) so the
  single DbAuthBackend works for both environments.
- safeStorageCodec drives the packaged build (encrypted at rest via
  Electron safeStorage / OS keychain).
- plaintextCodec drives unsigned dev builds — credentials persist in
  the same auth.ciphertext column as raw bytes, no keychain access at
  all. This eliminates the macOS keychain prompts the user was seeing
  on every dev launch.
- resolveAuthBackend(db) is the only export the rest of the app touches;
  it picks the codec based on app.isPackaged.

Sort + small fixes:
- AllowedModel interface members sorted by length.
- DbAuthBackend / Codec types' fields sorted by length.

Tests:
- New units/models-allowlist.test.ts: provider coverage, set membership,
  declaration-order preservation, non-empty per provider.
- New units/pi-settings-backend.test.ts: read-before-write returns
  undefined, write-then-read persists, undefined returned from callback
  clears, scopes (global vs project) are isolated.
@sasicodes sasicodes force-pushed the feat/storage-pr2-state-db branch from 71589a5 to d007a0e Compare May 25, 2026 10:05
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.

1 participant