[WIP] Add environment routing for carrying metadata and data API#1202
[WIP] Add environment routing for carrying metadata and data API#1202
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
- Add hostname field to EnvironmentSchema and SysEnvironment object - Create EnvironmentDriverRegistry for lazy driver instantiation with LRU cache - Extend HttpProtocolContext with environmentId and dataDriver fields - Implement resolveEnvironmentContext() with 4-tier precedence (hostname, header, session, default) - Update handleData() to use environment-scoped drivers - Modify handleCloud POST /environments to auto-compute hostname - Export EnvironmentDriverRegistry from runtime package Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/8385b77f-f7ed-4241-8e09-25c65d8d98b0 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
…ar deps - Replace static imports with dynamic imports for drivers - Add NoopSecretEncryptor to runtime package (avoid cross-package dependency) - Export SecretEncryptor interface from runtime - Use correct SqlDriver configuration for SQLite (better-sqlite3 client) Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/8385b77f-f7ed-4241-8e09-25c65d8d98b0 Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
github-actions
Bot
added
the
dependencies
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces multi-environment data-plane routing so /data/* requests can be resolved to an environment (via hostname/header/session) and dispatched against an environment-scoped database/driver, while keeping control-plane routes (/meta/*, /cloud/*) on the global kernel.
Changes:
- Add
hostnameto the environment spec and system environment object schema (unique, used for routing). - Introduce a runtime
EnvironmentDriverRegistrywith TTL caching and dynamic driver loading. - Extend
HttpDispatcherto resolve environment context per request and require an environment for data-plane requests when the registry is enabled.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Adds workspace links for driver packages referenced by runtime registry logic. |
| packages/spec/src/cloud/environment.zod.ts | Adds optional hostname to Environment + ProvisionEnvironmentRequest schemas. |
| packages/services/service-tenant/src/objects/sys-environment.object.ts | Adds hostname field (unique) + index for sys environment rows. |
| packages/services/service-tenant/src/environment-provisioning.ts | Persists hostname when provisioning environments. |
| packages/runtime/tsup.config.ts | Adds a runtime-specific tsup config and marks driver packages as external. |
| packages/runtime/src/index.ts | Exports the new environment registry types/constructors. |
| packages/runtime/src/http-dispatcher.ts | Adds env resolution + enforces env presence for /data/* when enabled; plumbs env context into data calls. |
| packages/runtime/src/environment-registry.ts | New registry that resolves env → driver via control-plane lookups + caches results. |
| packages/runtime/package.json | Adds driver packages as devDependencies and updates build config path. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| */ | ||
| private async callData(action: string, params: any): Promise<any> { | ||
| private async callData(action: string, params: any, dataDriver?: any): Promise<any> { | ||
| const protocol = await this.resolveService('protocol'); |
There was a problem hiding this comment.
callData() currently prefers the kernel-level protocol service for get/query. That service will still be backed by the kernel’s global ObjectQL/driver, so data-plane isolation won’t apply when protocol is registered, even if an env-scoped driver/engine is passed in. To make routing effective, make the protocol layer accept an env-scoped backend (or bypass protocol for env-scoped requests and route everything through the env-scoped ObjectQL/driver).
| const protocol = await this.resolveService('protocol'); | |
| const useEnvScopedBackend = dataDriver != null; | |
| const protocol = useEnvScopedBackend ? null : await this.resolveService('protocol'); |
| const host = context.request?.headers?.host || context.request?.headers?.['Host']; | ||
| if (host) { | ||
| // Strip port if present (e.g., "localhost:3000" → "localhost") | ||
| const hostname = host.split(':')[0]; | ||
| const result = await this.envRegistry.resolveByHostname(hostname); |
There was a problem hiding this comment.
Hostname parsing via host.split(':')[0] will mis-handle IPv6 host headers (e.g. [::1]:3000) and can also behave unexpectedly for non-standard host formats. Use a more robust host/port split (e.g., WHATWG URL parsing with a dummy scheme, or regex that preserves bracketed IPv6) before calling resolveByHostname().
| // Instantiate driver based on driver type | ||
| const driver = await this.createDriver(databaseDriver, databaseUrl, plaintextSecret); | ||
|
|
||
| return { | ||
| environmentId, | ||
| driver, | ||
| expiresAt: Date.now() + this.cacheTTL, | ||
| }; |
There was a problem hiding this comment.
createDriver() returns an IDataDriver but never calls driver.connect(). Both SqlDriver and InMemoryDriver require connect() to perform setup (e.g., ensure SQLite directories exist, initialize persistence). Without this, env-scoped requests can fail at first query. Ensure drivers are connected before caching/returning them, and consider disconnecting them on invalidate()/eviction to avoid leaking resources.
| private async callData(action: string, params: any, dataDriver?: any): Promise<any> { | ||
| const protocol = await this.resolveService('protocol'); | ||
| const qlService = await this.getObjectQLService(); | ||
| const qlService = dataDriver ?? await this.getObjectQLService(); | ||
| const ql = qlService ?? await this.resolveService('objectql'); | ||
|
|
There was a problem hiding this comment.
callData() treats the optional dataDriver as if it were an ObjectQL engine (insert/find/update/delete), but EnvironmentDriverRegistry (and the context comment) imply this value is an IDataDriver (which exposes create/find/update(id, ...), etc.). Passing an IDataDriver here will crash at runtime (e.g., ql.insert is not a function) or silently route to the wrong backend. Align the types/abstraction: either pass an env-scoped ObjectQL instance (and rename the param accordingly) or adapt IDataDriver to the engine API before using it here.
| // Compute hostname if not provided | ||
| // Format: {org-slug}-{env-slug}.{rootDomain} | ||
| // For now, use a simple format. In production, fetch org.slug from database. | ||
| let computedHostname = req.hostname; | ||
| if (!computedHostname) { | ||
| // Try to look up organization slug | ||
| try { | ||
| const orgRow = await findOne('sys__organization', { id: req.organizationId }); | ||
| const orgSlug = orgRow?.slug || req.organizationId; | ||
| const rootDomain = getEnv('ROOT_DOMAIN', 'objectstack.app'); | ||
| computedHostname = `${orgSlug}-${req.slug}.${rootDomain}`; | ||
| } catch { | ||
| // Fallback if sys__organization doesn't exist | ||
| computedHostname = `${req.organizationId}-${req.slug}.objectstack.app`; | ||
| } |
There was a problem hiding this comment.
The comment says “In production, fetch org.slug from database”, but the code already attempts to load sys__organization and use its slug. Update the comment to reflect the current behavior (or remove it) to avoid misleading future edits.
| case 'memory': { | ||
| // Memory driver: URL format is memory://dbname or memory:// | ||
| const { InMemoryDriver } = await import('@objectstack/driver-memory'); | ||
| return new InMemoryDriver({ | ||
| persistence: 'file', // Use file persistence for environments | ||
| }); | ||
| } |
There was a problem hiding this comment.
The memory driver branch always uses persistence: 'file' with the driver’s default file path (.objectstack/data/memory-driver.json). That path is shared across all InMemoryDriver instances, so multiple environments will read/write the same persisted file, breaking environment isolation. Use an environment-specific persistence path derived from databaseUrl/environmentId (or disable persistence entirely) so each environment remains isolated.
| async resolveByHostname(host: string): Promise<{ environmentId: string; driver: IDataDriver } | null> { | ||
| // Check cache first | ||
| const cached = this.hostnameCache.get(host); | ||
| if (cached && cached.expiresAt > Date.now()) { | ||
| return { environmentId: cached.environmentId, driver: cached.driver }; | ||
| } |
There was a problem hiding this comment.
resolveByHostname() caches/looks up hostnames as-is. Incoming Host headers are case-insensitive and may include a trailing dot; without normalization, the same environment can miss cache and fail resolution. Normalize host (e.g., trim(), toLowerCase(), strip trailing '.') before querying/caching.
| /** | ||
| * Environment-scoped driver registry with LRU caching. | ||
| * | ||
| * Resolves environments by hostname or ID, lazily instantiates data drivers, | ||
| * and caches them with TTL to avoid re-querying control plane on every request. | ||
| * | ||
| * Implements ADR-0002 environment routing: request → hostname/header/session → | ||
| * sys__environment → sys__database_credential → env-scoped IDataDriver. | ||
| */ |
There was a problem hiding this comment.
This file header/docstring claims “LRU caching”, but the implementation is a plain Map with TTL and no size bound or LRU eviction policy. Either implement an actual LRU (size limit + recency updates) or update the documentation to avoid promising behavior that doesn’t exist.
| "zod": "^4.3.6" | ||
| }, | ||
| "devDependencies": { | ||
| "@objectstack/driver-memory": "workspace:*", | ||
| "@objectstack/driver-sql": "workspace:*", | ||
| "@objectstack/driver-turso": "workspace:*", |
There was a problem hiding this comment.
@objectstack/runtime dynamically imports @objectstack/driver-* at runtime (and marks them as external in tsup), but these packages are listed under devDependencies. When @objectstack/runtime is installed as a dependency, devDependencies won’t be present, so environment routing will fail at runtime. Move these to dependencies (or peerDependencies/optionalDependencies with a clear runtime error when missing).
| "zod": "^4.3.6" | |
| }, | |
| "devDependencies": { | |
| "@objectstack/driver-memory": "workspace:*", | |
| "@objectstack/driver-sql": "workspace:*", | |
| "@objectstack/driver-turso": "workspace:*", | |
| "@objectstack/driver-memory": "workspace:*", | |
| "@objectstack/driver-sql": "workspace:*", | |
| "@objectstack/driver-turso": "workspace:*", | |
| "zod": "^4.3.6" | |
| }, | |
| "devDependencies": { |
Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.