feat: add contacts + contact_channels tables to gateway SQLite#25951
Conversation
Gateway cutover step 1: declare contacts and contact_channels tables in the gateway DB schema. This is the foundation for moving contact auth/authz ownership from the assistant daemon to the gateway. - contacts: mirrors assistant's contacts table (auth/authz fields only) - contact_channels: mirrors assistant's contact_channels table with same indexes (type+external_user_id, type+external_chat_id) - m0002-seed-contacts: one-time data migration that seeds both tables from assistant.db on first startup (INSERT OR IGNORE, transactional) - ContactStore: read-only store with prepared-statement queries (getContact, listContacts, getContactByChannel, getChannelsForContact) - IPC handlers: list_contacts, get_contact, get_contact_by_channel, get_channels_for_contact — wired into the gateway IPC server - Tests: ContactStore unit tests + IPC round-trip tests
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 464db23e47
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!tableCheck) { | ||
| log.info("No contacts table in assistant.db — nothing to seed"); | ||
| return "done"; |
There was a problem hiding this comment.
Retry seeding when source contacts table is not ready
Returning "done" when assistant.db lacks contacts permanently records m0002-seed-contacts as completed (via runDataMigrations), so the seed will never run again for that instance. This can happen when upgrading from older assistant databases (before contacts existed) or when gateway starts before assistant schema migrations finish; once the table appears later, gateway contact data still remains empty because this migration is no longer retried.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
not a real concern - in that case it's empty
| `SELECT id, display_name, notes, role, principal_id, user_file, contact_type, created_at, updated_at | ||
| FROM contacts`, |
There was a problem hiding this comment.
Handle older assistant contact schemas during seed
The migration selects recently-added columns (notes, role, principal_id, user_file, contact_type, and similarly newer channel fields) unconditionally, but these were introduced across later assistant migrations, so older assistant.db files with a valid contacts table will throw no such column and skip seeding. In that version-skew scenario, gateway contact data is missing until a manual restart after assistant migrations complete, which breaks cutover reliability for upgraded installs.
Useful? React with 👍 / 👎.
| db.exec(` | ||
| CREATE INDEX IF NOT EXISTS idx_contact_channels_type_ext_user | ||
| ON contact_channels(type, external_user_id) | ||
| `); |
There was a problem hiding this comment.
🚩 No UNIQUE constraint on (type, external_user_id) may return ambiguous results
The contact_channels table has an index on (type, external_user_id) (gateway/src/db/connection.ts:129-132) but no UNIQUE constraint. The getContactByChannel query at gateway/src/db/contact-store.ts:148-151 uses LIMIT 1, which means if multiple channels share the same (type, external_user_id) pair, the returned contact is nondeterministic (depends on SQLite's row ordering). This may be intentional (allowing multiple contacts to share an external user ID), but if the intent is 1:1 mapping between external identities and contacts, a UNIQUE constraint would catch data integrity issues at write time rather than silently returning an arbitrary match.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const { GatewayIpcServer } = await import("../ipc/server.js"); | ||
| const { contactRoutes } = await import("../ipc/contact-handlers.js"); | ||
| const { ContactStore } = await import("../db/contact-store.js"); | ||
| const { getGatewayDb } = await import("../db/connection.js"); |
There was a problem hiding this comment.
Move these imports to the top instead of inline
|
|
||
| db.exec( | ||
| `INSERT INTO contact_channels (id, contact_id, type, address, is_primary, external_user_id, external_chat_id, status, policy, interaction_count, created_at) | ||
| VALUES ('ch2', 'c1', 'slack', 'U05D5EGNNMS', 0, 'U05D5EGNNMS', 'D09Q9FG277H', 'active', 'allow', 10, ${now})`, |
There was a problem hiding this comment.
Make these channel ids look fake, they look too real
| if (!tableCheck) { | ||
| log.info("No contacts table in assistant.db — nothing to seed"); | ||
| return "done"; |
There was a problem hiding this comment.
not a real concern - in that case it's empty
| @@ -0,0 +1,189 @@ | |||
| /** | |||
There was a problem hiding this comment.
Let's actually hold off on doing any data migrations until the endpoints have cutover to the new table and we're dual writing.
| display_name TEXT NOT NULL, | ||
| notes TEXT, |
There was a problem hiding this comment.
We don't need to carry over columns that will not be used during actor validation. including these two
| notes TEXT, | ||
| role TEXT NOT NULL DEFAULT 'contact', | ||
| principal_id TEXT, | ||
| user_file TEXT, |
There was a problem hiding this comment.
I think we can remove this column for now
- Strip contacts table to auth/authz-only: remove notes, user_file, contact_type columns (not needed for actor validation) - Remove m0002-seed-contacts data migration — hold off until endpoints have cutover and we're dual-writing - Move test imports to top level (no more inline await import()) - Use fake channel IDs in tests instead of real ones - Clean test state between runs (DELETE before seed) - Update ARCHITECTURE.md + gateway/ARCHITECTURE.md to document the contacts ownership migration direction - Add Drizzle migration + test preload env var cleanup tasks to workstream Up Next
| getContactByChannel( | ||
| channelType: string, | ||
| externalUserId: string, | ||
| ): Contact | null { | ||
| const stmt = | ||
| this._getContactByChannel ?? | ||
| (this._getContactByChannel = this.db.prepare( | ||
| `SELECT c.* FROM contacts c | ||
| JOIN contact_channels cc ON cc.contact_id = c.id | ||
| WHERE cc.type = ? AND cc.external_user_id = ? | ||
| LIMIT 1`, | ||
| )); | ||
| const row = stmt.get(channelType, externalUserId) as ContactRow | null; | ||
| return row ? toContact(row) : null; | ||
| } |
There was a problem hiding this comment.
🚩 Gateway getContactByChannel only supports externalUserId lookup, not externalChatId fallback
The assistant's findContactChannel (assistant/src/contacts/contact-store.ts:806-840) supports a dual-lookup strategy: first by (channelType, externalUserId), then falling back to (channelType, externalChatId). The gateway's ContactStore.getContactByChannel (gateway/src/db/contact-store.ts:132-146) only supports lookup by externalUserId, and the IPC schema at gateway/src/ipc/contact-handlers.ts:26-29 requires externalUserId as a mandatory string field. This means callers that only have an externalChatId cannot use this IPC endpoint. The architecture doc at gateway/ARCHITECTURE.md:619 acknowledges this is a migration in progress ('Endpoint cutover and data migration to follow'), so this appears intentional for the initial step. However, any daemon code that migrates to use the gateway IPC for contact lookups will need to handle the missing externalChatId fallback path itself, or a new IPC method will need to be added.
Was this helpful? React with 👍 or 👎 to provide feedback.
* feat: add contacts + contact_channels tables to gateway SQLite Gateway cutover step 1: declare contacts and contact_channels tables in the gateway DB schema. This is the foundation for moving contact auth/authz ownership from the assistant daemon to the gateway. - contacts: mirrors assistant's contacts table (auth/authz fields only) - contact_channels: mirrors assistant's contact_channels table with same indexes (type+external_user_id, type+external_chat_id) - m0002-seed-contacts: one-time data migration that seeds both tables from assistant.db on first startup (INSERT OR IGNORE, transactional) - ContactStore: read-only store with prepared-statement queries (getContact, listContacts, getContactByChannel, getChannelsForContact) - IPC handlers: list_contacts, get_contact, get_contact_by_channel, get_channels_for_contact — wired into the gateway IPC server - Tests: ContactStore unit tests + IPC round-trip tests * review: address Vargas feedback on PR #25951 - Strip contacts table to auth/authz-only: remove notes, user_file, contact_type columns (not needed for actor validation) - Remove m0002-seed-contacts data migration — hold off until endpoints have cutover and we're dual-writing - Move test imports to top level (no more inline await import()) - Use fake channel IDs in tests instead of real ones - Clean test state between runs (DELETE before seed) - Update ARCHITECTURE.md + gateway/ARCHITECTURE.md to document the contacts ownership migration direction - Add Drizzle migration + test preload env var cleanup tasks to workstream Up Next --------- Co-authored-by: root <root@assistant-89f9b42a-2563-4bbe-96b7-b2840c145b37-0.assistant-89f9b42a-2563-4bbe-96b7-b2840c145b37.warm-pool.svc.cluster.local>
…25977) * fix(gmail): make retry sleeps signal-aware in Gmail client The abort controller from sender-digest can now interrupt retry sleep delays, preventing Promise.allSettled from hanging past the deadline when batchGetMessages enters exponential backoff. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(macos): gate thinking-anchor reset to toolRunning only (#25968) Resetting the thinking anchor on .streamingCode can erase valid post-tool thinking intervals when a late code preview fires after tools complete. Restrict the reset to .toolRunning so the thinking duration is accurate. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(gmail): safe default for has_prior_reply, time-budget enrichment (#25967) - Default has_prior_reply to true on API errors (safe direction per SKILL.md) - Skip reply checks when already rate-limited to avoid wasting quota - Add time budget to enrichment step using remaining TIME_BUDGET_MS - Over-fetch sender candidates before capping to max_senders Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(gmail): restrict archive fallback to expired scans, sanitize query (#25966) * fix(gmail): restrict archive fallback to expired scans, sanitize query - Only fall back to query-based archiving when scan is truly expired (null), not when sender IDs don't match (empty array). - Quote emails in fallback query to prevent Gmail query injection. - Update SKILL.md to reflect new fallback behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(gmail): clarify SKILL.md scan expiration fallback behavior Document the distinction between expired scan (null, falls back to query) vs sender ID mismatch (empty array, returns error). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(oauth): add gmail.settings.basic scope to Google OAuth defaults (#25970) The Gmail settings scope is required for filter creation/deletion, label management, and other settings-level operations. Without it, the gmail_filters tool fails with 403 ACCESS_TOKEN_SCOPE_INSUFFICIENT. Existing tokens will be flagged by the credential health service's scope drift detection, prompting users to re-authenticate. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add contacts + contact_channels tables to gateway SQLite (#25951) * feat: add contacts + contact_channels tables to gateway SQLite Gateway cutover step 1: declare contacts and contact_channels tables in the gateway DB schema. This is the foundation for moving contact auth/authz ownership from the assistant daemon to the gateway. - contacts: mirrors assistant's contacts table (auth/authz fields only) - contact_channels: mirrors assistant's contact_channels table with same indexes (type+external_user_id, type+external_chat_id) - m0002-seed-contacts: one-time data migration that seeds both tables from assistant.db on first startup (INSERT OR IGNORE, transactional) - ContactStore: read-only store with prepared-statement queries (getContact, listContacts, getContactByChannel, getChannelsForContact) - IPC handlers: list_contacts, get_contact, get_contact_by_channel, get_channels_for_contact — wired into the gateway IPC server - Tests: ContactStore unit tests + IPC round-trip tests * review: address Vargas feedback on PR #25951 - Strip contacts table to auth/authz-only: remove notes, user_file, contact_type columns (not needed for actor validation) - Remove m0002-seed-contacts data migration — hold off until endpoints have cutover and we're dual-writing - Move test imports to top level (no more inline await import()) - Use fake channel IDs in tests instead of real ones - Clean test state between runs (DELETE before seed) - Update ARCHITECTURE.md + gateway/ARCHITECTURE.md to document the contacts ownership migration direction - Add Drizzle migration + test preload env var cleanup tasks to workstream Up Next --------- Co-authored-by: root <root@assistant-89f9b42a-2563-4bbe-96b7-b2840c145b37-0.assistant-89f9b42a-2563-4bbe-96b7-b2840c145b37.warm-pool.svc.cluster.local> * fix(runtime): wake adapter drains queue, persists with metadata, broadcasts to all clients (#25972) * meet-join: SKILL.md guidance for voice participation (#25973) * fix(gmail): persist blocklist only after archive succeeds (#25971) Move addToBlocklist() call from before the batch archive operation to after it succeeds. Prevents corrupted cleanup state when archiving fails. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(environments): env-aware data and config path layout (#25499) * feat(environments): daemon vellumRoot() honors BASE_DATA_DIR per-instance override (#25455) * feat(environments): Swift-side VellumPaths env-aware helpers (#25457) * feat(environments): route CLI lockfile R/W and allocator through environment helpers; delete getDataDir (#25458) * feat(environments): route CLI platform token, guardian token, and device-id paths through getConfigDir(env) (#25456) * feat(environments): route CLI platform token, guardian token, and device-id paths through getConfigDir(env) * fix(cli): use spyOn for platform-client and guardian-token in teleport tests `mock.module()` in bun:test replaces a module globally in the process and provides no way to unmock. `teleport.test.ts` was using it to stub both `../lib/platform-client.js` and `../lib/guardian-token.js`, so those mocks leaked into `platform-client.test.ts` and `guardian-token.test.ts` when they ran in the same bun test process — every call to `readPlatformToken()` in the platform-client tests returned the literal string "platform-token" from the stale teleport mock, and `loadGuardianToken()` in the guardian-token tests returned a minimal fake object missing the `guardianPrincipalId` field the tests assert on. Mirror the existing `assistant-config` pattern (already using `spyOn` for the same reason per its inline comment) for `platform-client` and `guardian-token`. `spyOn()` mutates the imported module namespace object only, and `mockRestore()` in `afterAll` fully reverts the stubs so other test files see the real implementations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(cli): delete unused LOCKFILE_NAMES export (#25488) * fix(environments): route daemon protected/ callers through platform helpers (#25493) * fix(environments): orphan-detection and recover find daemons across all lockfile entries (#25481) * fix(environments): orphan-detection and recover find daemons across all lockfile entries * fix(recover): scope collision check to recovering entry's own target path Addresses Codex P1 and Devin P1 on #25481: the iterate-all-entries loop blocked recovery whenever any unrelated local assistant was still installed. * refactor(environments): route Swift client path sites through VellumPaths (#25483) * refactor(environments): route Swift client path sites through VellumPaths * fix(environments): remove dead xdgDataHome field + reject relative XDG paths Addresses Devin and Codex P2 findings on PR #25457: - xdgDataHome was stored but never read; remove the field and its resolver helper - resolveXdgConfigHome() no longer rewrites relative XDG_CONFIG_HOME values against cwd — relative values are rejected for parity with the TypeScript env package * fix(environments): make daemon XDG platform-token and device-id env-aware (#25497) * refactor(device-id): inline base-dir helper into migration 003 Removes the stale getDeviceIdBaseDir() export from device-id.ts — getDeviceId() no longer uses it, so it was a maintenance trap whose return value diverged from where device.json actually resolves in non-production envs. Its sole remaining caller was workspace migration 003, so inline the 2-line containerized-vs-homedir branch there. Brings migration 003 closer to the self-containment rule in assistant/src/workspace/migrations/AGENTS.md (no external imports beyond types/logger). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs(environments): update AGENTS.md and ARCHITECTURE.md for per-assistant data layout (#25504) * fix(environments): CLI falls back to production on unknown VELLUM_ENVIRONMENT (parity with daemon and Swift) (#25541) * fix(permissions): restore legacy signing-key path in risk classification (#25542) * fix(config-watcher): use || for GATEWAY_SECURITY_DIR fallback to match sibling convention (#25543) * fix(cli): read platformBaseUrl from lockfile instead of legacy workspace config path (#25544) * fix(chrome-ext): native host reads env-aware lockfile path (#25547) * fix(environments): VellumPaths accepts relative XDG_CONFIG_HOME to match TS/daemon (#25550) * refactor(recover): drop unreachable legacy fallback in collision check (#25575) * fix(cli): sync platformBaseUrl to lockfile on vellum use / vellum wake (#25578) * fix(environments): env-seed fallback for getPlatformUrl, revert H1 sync-on-switch (#25595) Under our invariant "each env has its own lockfile, and all assistants in that lockfile share a platform URL", the H1 workspace-config→lockfile sync on `vellum use`/`vellum wake` was load-bearing for nothing: switching the active assistant within a single env cannot change the platform URL. Revert that sync. When no lockfile is seeded yet, fall back to the current environment's seed URL instead of the hardcoded production default so `VELLUM_ENVIRONMENT=dev vellum …` targets `dev-platform.vellum.ai` out of the box. - Revert H1: `vellum use` no longer calls `syncActiveAssistantConfigToLockfile`, `vellum wake` no longer re-runs `syncConfigToLockfile`, and the helper itself is deleted (the hatch-time sync is still done by `syncConfigToLockfile`, unchanged). - `getPlatformUrl()` fallback: prefer `getCurrentEnvironment().platformUrl` over the hardcoded prod URL so non-prod CLI users get the right tenant before any assistant is registered. - Tests: drop the H1 sync-on-switch suite, add a dev-env seed fallback test, keep the existing prod fallback test. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(environments): drift-guard for KNOWN_ENVIRONMENTS across TS sites (#25617) The set of recognized environment names is duplicated in three TS locations: `cli/src/lib/environments/seeds.ts` (SEEDS), the daemon's `assistant/src/util/platform.ts` (KNOWN_ENVIRONMENTS), and the Chrome native host's `native-host/src/lockfile.ts` (NON_PRODUCTION_ENVIRONMENTS). Cross-package imports don't work today — assistant's tsconfig restricts `include` to its own src tree, and the native host is a standalone TS project with `rootDir: ./src`. Add a drift-guard test in cli that parses the literal Set bodies from both external files and asserts they agree with CLI's SEEDS (minus `production` for the native host set). Catches any future addition to the seed table that fails to propagate to the other two sites. Also refresh the comments on all three declarations to point at the drift-guard test and the fast-follow plan: hoist the shared name list into a `packages/environments` package (mirroring `packages/ces-contracts` etc.) so this check becomes a compile-time import instead of a runtime regex. That refactor is planned alongside CLI-driven context support. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(environments): forward VELLUM_ENVIRONMENT across desktop→CLI→daemon handoffs (#25633) Two call sites were stripping VELLUM_ENVIRONMENT from spawn whitelists, breaking environment isolation for the main desktop launch path: 1. macOS `VellumCli.makeBaseEnvironment()` — `forwardedEnvKeys` did not include `VELLUM_ENVIRONMENT`, so every bundled-CLI command launched from the app (hatch, wake, sleep, retire, …) ran as production even when the app itself was built for a non-production environment. The app's Info.plist sets `VELLUM_ENVIRONMENT` at build time (`build.sh:1054`), so forwarding it is sufficient. 2. `cli/src/lib/local.ts` compiled-daemon spawn — the `daemonEnv` whitelist used when `bun run` is unavailable (packaged desktop builds) also omitted `VELLUM_ENVIRONMENT`. Even when the CLI process itself had the variable set, the spawned daemon fell back to production path/env behavior, so assistant-side env-scoped state (device ID, XDG-backed tokens and config reads) bled into prod. Note: the source/watch daemon spawn path in `local.ts:281` is unaffected — it uses `{...process.env}` and inherits everything. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * default to dev environment * remove duplicate env var --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * meet-bot: implement POST /play_audio streaming endpoint (#25974) * fix(macos): reset thinking anchor on streamingCode when tools active Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: vellum-apollo-bot[bot] <242025090+vellum-apollo-bot[bot]@users.noreply.github.com> Co-authored-by: root <root@assistant-89f9b42a-2563-4bbe-96b7-b2840c145b37-0.assistant-89f9b42a-2563-4bbe-96b7-b2840c145b37.warm-pool.svc.cluster.local> Co-authored-by: siddseethepalli <siddseethepalli@gmail.com> Co-authored-by: clopen-set <33433326+clopen-set@users.noreply.github.com>
Gateway Cutover — Step 1: Contacts Schema
First step of the gateway cutover plan: declare
contactsandcontact_channelstables in the gateway-owned SQLite database.What
contactsandcontact_channelstables added to gatewaymigrate()(declarativeCREATE TABLE IF NOT EXISTS). Mirrors the assistant's existing schema — same columns, same indexes.m0002-seed-contacts— one-time migration that reads fromassistant.dband inserts intogateway.sqliteinside a single transaction. UsesINSERT OR IGNOREfor idempotency. Recorded inone_time_migrationstable so it never reruns.getContact,listContacts,getContactByChannel,getChannelsForContact.list_contacts,get_contact,get_contact_by_channel,get_channels_for_contact.Why
The gateway is the ingress boundary — it receives inbound messages and needs to make trust decisions about senders. Currently, contact auth/authz data lives in the assistant's database where the daemon has full write access. This is a security liability (prompt injection → modify contact trust) and a UX liability (daemon bug → accidental lockout).
Moving contacts to a gateway-owned DB enforced read-only at the volume mount level is the architectural fix. This PR lays the schema foundation — future PRs will wire the gateway's inbound handlers to read from these tables and eventually remove the assistant's copies.
Files
gateway/src/db/connection.tscontacts+contact_channelstable DDL + indexesgateway/src/db/contact-store.tsgateway/src/db/data-migrations/m0002-seed-contacts.tsgateway/src/db/data-migrations/index.tsgateway/src/ipc/contact-handlers.tsgateway/src/index.tsgateway/src/__tests__/ipc-contact-routes.test.ts