diff --git a/docs/docs/configure/warehouses.md b/docs/docs/configure/warehouses.md index fc473058ee..d85136be46 100644 --- a/docs/docs/configure/warehouses.md +++ b/docs/docs/configure/warehouses.md @@ -262,6 +262,26 @@ If you're already authenticated via `gcloud`, omit `credentials_path`: | Field | Required | Description | |-------|----------|-------------| | `path` | No | Database file path. Omit or use `":memory:"` for in-memory | +| `create` | No | Create the database file if it is missing (default: `false`) | + +!!! warning "The store must already exist" + Connecting never creates the database. If the file is missing, the connection + fails with an error naming the path it looked for — an empty database would + otherwise answer every query with no rows and no error, which reads as a + successful query against an empty warehouse. Set `"create": true` only when + you intend this store to be created. + +!!! note "How a relative `path` is resolved" + A relative `path` is resolved once, when the config is loaded, against the + directory that declares it: + + - `~/.altimate-code/connections.json` → resolved against `~/.altimate-code` + - `/.altimate-code/connections.json` → resolved against `` + - `ALTIMATE_CODE_CONN_*` environment variables → resolved against the project root + + It is never resolved against the current working directory, so `--dir` cannot + re-point an existing connection at a different file. Absolute paths are always + safest. !!! note "Concurrent access" DuckDB does not support concurrent write access to the same file. If another process holds a write lock, Altimate Code automatically retries the connection in **read-only** mode so you can still query the data. A clear error message is shown if read-only access also fails. @@ -443,10 +463,16 @@ If you're already authenticated via `gcloud`, omit `credentials_path`: |-------|----------|-------------| | `path` | No | Database file path. Omit or use `":memory:"` for in-memory | | `readonly` | No | Open in read-only mode (default: `false`) | +| `create` | No | Create the database file if it is missing (default: `false`) | !!! note SQLite uses Bun's built-in `bun:sqlite` driver. WAL journal mode is enabled automatically for writable databases. +!!! warning "The store must already exist" + As with DuckDB, connecting never creates the database, and a relative `path` + resolves against the directory of the config that declares it — not the current + working directory. See the DuckDB section above for the full rules. + ## SQL Server ```json diff --git a/docs/docs/drivers.md b/docs/docs/drivers.md index fc9c13530a..c741272d35 100644 --- a/docs/docs/drivers.md +++ b/docs/docs/drivers.md @@ -191,6 +191,11 @@ MongoDB supports server versions 3.6 through 8.0. Queries use MQL (MongoDB Query |--------|--------------| | File | `path: "./my-database.sqlite"` | +For both file-backed drivers, the database must already exist — connecting never +creates it. Pass `create: true` to create it deliberately. A relative `path` +resolves against the directory of the config that declares it, not the current +working directory. See [Warehouses](configure/warehouses.md#duckdb) for the full rules. + ## SSH Tunneling Connect through a bastion host by adding SSH config to any connection: diff --git a/packages/drivers/src/duckdb.ts b/packages/drivers/src/duckdb.ts index 32bf58c52a..2854491d89 100644 --- a/packages/drivers/src/duckdb.ts +++ b/packages/drivers/src/duckdb.ts @@ -2,6 +2,7 @@ * DuckDB driver using the `duckdb` package. */ +import { assertStoreExists, requireStorePath } from "./file-store" import type { ConnectionConfig, Connector, ConnectorResult, ExecuteOptions, SchemaColumn } from "./types" import { loadOptionalDriver } from "./resolve" @@ -10,7 +11,9 @@ export async function connect(config: ConnectionConfig): Promise { duckdb = await loadOptionalDriver("duckdb", "duckdb") duckdb = duckdb.default || duckdb - const dbPath = (config.path as string) ?? ":memory:" + // altimate_change start — a missing path must fail loudly, not become :memory: + const dbPath = requireStorePath(config, "DuckDB") + // altimate_change end let db: any let connection: any @@ -48,6 +51,9 @@ export async function connect(config: ConnectionConfig): Promise { return { async connect() { + // altimate_change start — never conjure an empty store on open + assertStoreExists(config, dbPath, "DuckDB") + // altimate_change end // altimate_change start — retry with read-only on lock errors const tryConnect = (accessMode?: string): Promise => new Promise((resolve, reject) => { diff --git a/packages/drivers/src/file-store.ts b/packages/drivers/src/file-store.ts new file mode 100644 index 0000000000..4c4ad3445c --- /dev/null +++ b/packages/drivers/src/file-store.ts @@ -0,0 +1,96 @@ +/** + * Shared guards for file-backed drivers (DuckDB, SQLite). + * + * Both engines create an empty database when asked to open a file that does + * not exist. For a warehouse connection that is never what the caller wants: + * a mistyped or mis-resolved path then yields a working connector over an + * empty database, so every query succeeds and returns nothing. An agent handed + * that result reports "no tables" instead of an error. + * + * Opening a store is therefore read-or-fail by default. Creation is opt-in via + * `create: true`, which the tools that deliberately materialize a local store + * (local test scratch databases, schema sync targets) pass explicitly. + */ + +import * as fs from "fs" +import type { ConnectionConfig } from "./types" + +/** + * Whether `dbPath` names a file on the local filesystem, and so can be + * existence-checked before the driver opens it. + * + * Only the exact string `:memory:` is an in-memory database. Both engines + * treat `:memory:named` — and any other colon-prefixed name — as an ordinary + * (if oddly named) file: DuckDB really does write a file called + * `:memory:named`, so those must stay inside the guard. An empty path is + * DuckDB's in-memory database and SQLite's anonymous temporary one; neither + * touches disk. + * + * A scheme-qualified target is not a local file: MotherDuck (`md:`), object + * storage (`s3://`), DuckLake, and any other scheme a DuckDB extension + * provides. Those are left to the driver, which reports an unknown scheme as a + * missing-extension error rather than silently creating anything. The pattern + * requires two or more characters before the colon so a Windows drive letter + * (`C:\data\wh.duckdb`) stays a path. + */ +export function isLocalFilePath(dbPath: string): boolean { + if (dbPath === "" || dbPath === ":memory:") return false + if (/^[a-zA-Z][a-zA-Z0-9+.-]+:/.test(dbPath)) return false + return true +} + +/** + * The store path a file-backed connection names, or a loud failure. + * + * Both drivers used to read `(config.path as string) ?? ":memory:"`. That turns + * ANY failure to carry a path — a config the registry never loaded, a field + * under the wrong name, a lookup that fell through — into a successful + * connection over an empty in-memory database. Every query then returns no rows + * and no error, which reads as a healthy warehouse that happens to be empty. + * + * It is a worse failure than creating a store on disk: a stray file can at + * least be found afterwards, whereas an in-memory database leaves nothing + * behind to explain the empty answer. `:memory:` remains available, but only + * when a caller asks for it by name. + */ +export function requireStorePath(config: ConnectionConfig, engine: string): string { + const value = config.path + if (typeof value === "string" && value !== "") return value + throw new Error( + `${engine} connection is missing its "path". A file-backed warehouse must name its database explicitly — ` + + `falling back to an in-memory database would answer every query with no rows and no error, ` + + `which is indistinguishable from a healthy but empty warehouse. ` + + `Set "path" to the database file, or to ":memory:" if a throwaway empty database is genuinely what you want.`, + ) +} + +/** Whether the caller explicitly opted in to creating the store. */ +export function allowsCreate(config: ConnectionConfig): boolean { + return config.create === true +} + +/** + * Throw unless the store is safe to open: it already exists, the caller opted + * in to creating it, or the path is not a local file at all. + * + * @param engine Human-readable engine name used in the error message. + * @param allowCreate Whether this open will actually create the store. Defaults + * to the config's `create` flag; a driver passes it explicitly when its own + * options can veto creation — SQLite never creates a read-only connection. + */ +export function assertStoreExists( + config: ConnectionConfig, + dbPath: string, + engine: string, + allowCreate: boolean = allowsCreate(config), +): void { + if (allowCreate) return + if (!isLocalFilePath(dbPath)) return + if (fs.existsSync(dbPath)) return + throw new Error( + `${engine} database file not found: "${dbPath}". ` + + `Opening a warehouse connection never creates the database — an empty store would answer every query with no rows. ` + + `Check the "path" in your connection config (relative paths resolve against the config file's directory, not the current directory), ` + + `or pass "create": true if this store is meant to be created.`, + ) +} diff --git a/packages/drivers/src/index.ts b/packages/drivers/src/index.ts index d3c755c31d..2e10ca75d0 100644 --- a/packages/drivers/src/index.ts +++ b/packages/drivers/src/index.ts @@ -4,6 +4,9 @@ export type { Connector, ConnectorResult, SchemaColumn, ConnectionConfig } from // Re-export config normalization export { normalizeConfig, sanitizeConnectionString } from "./normalize" +// Re-export file-backed store guards +export { allowsCreate, assertStoreExists, isLocalFilePath, requireStorePath } from "./file-store" + // Re-export driver connect functions export { connect as connectPostgres } from "./postgres" export { connect as connectSnowflake } from "./snowflake" diff --git a/packages/drivers/src/sqlite.ts b/packages/drivers/src/sqlite.ts index 48ef8321cd..3c9da882fd 100644 --- a/packages/drivers/src/sqlite.ts +++ b/packages/drivers/src/sqlite.ts @@ -4,19 +4,32 @@ */ import { Database } from "bun:sqlite" +import { allowsCreate, assertStoreExists, requireStorePath } from "./file-store" import type { ConnectionConfig, Connector, ConnectorResult, ExecuteOptions, SchemaColumn } from "./types" export async function connect(config: ConnectionConfig): Promise { - const dbPath = (config.path as string) ?? ":memory:" + // altimate_change start — a missing path must fail loudly, not become :memory: + const dbPath = requireStorePath(config, "SQLite") + // altimate_change end let db: Database | null = null return { async connect() { const isReadonly = config.readonly === true + // altimate_change start — never conjure an empty store on open. + // A read-only connection never creates, so `create: true` cannot excuse a + // missing file there; the guard is told the effective decision. + const willCreate = !isReadonly && allowsCreate(config) + assertStoreExists(config, dbPath, "SQLite", willCreate) db = new Database(dbPath, { readonly: isReadonly, - create: !isReadonly, + // `create` alone is no longer implied by "not readonly", so the + // read-write flag has to be explicit — bun:sqlite rejects an options + // object that sets no open flag at all. + readwrite: !isReadonly, + create: willCreate, }) + // altimate_change end if (!isReadonly) { db.exec("PRAGMA journal_mode = WAL") } diff --git a/packages/drivers/test/driver-security.test.ts b/packages/drivers/test/driver-security.test.ts index 6a0f8d8c5d..3f3f221340 100644 --- a/packages/drivers/test/driver-security.test.ts +++ b/packages/drivers/test/driver-security.test.ts @@ -168,7 +168,7 @@ describe("DuckDB driver", () => { })) const { connect } = await import("../src/duckdb") - const connector = await connect({ type: "duckdb", path: "/tmp/test.duckdb" }) + const connector = await connect({ type: "duckdb", path: "/tmp/test.duckdb", create: true }) await connector.connect() expect(connectAttempts).toBe(2) // First failed, second succeeded in READ_ONLY // The retry must specifically request READ_ONLY — two attempts alone don't @@ -262,7 +262,7 @@ describe("DuckDB driver", () => { })) const { connect } = await import("../src/duckdb") - const connector = await connect({ type: "duckdb", path: "/tmp/sync-lock.duckdb" }) + const connector = await connect({ type: "duckdb", path: "/tmp/sync-lock.duckdb", create: true }) await connector.connect() expect(connectAttempts).toBe(2) expect(await connector.execute("SELECT 1")).toMatchObject({ columns: ["ok"], rows: [[1]], row_count: 1 }) @@ -287,7 +287,7 @@ describe("DuckDB driver", () => { })) const { connect } = await import("../src/duckdb") - const connector = await connect({ type: "duckdb", path: "/tmp/corrupt.duckdb" }) + const connector = await connect({ type: "duckdb", path: "/tmp/corrupt.duckdb", create: true }) await expect(connector.connect()).rejects.toThrow("catalog is corrupt") }) }) diff --git a/packages/drivers/test/file-store-guard.test.ts b/packages/drivers/test/file-store-guard.test.ts new file mode 100644 index 0000000000..391165951c --- /dev/null +++ b/packages/drivers/test/file-store-guard.test.ts @@ -0,0 +1,172 @@ +/** + * Unit tests for the file-backed store guards (src/file-store.ts). + * + * Both DuckDB and SQLite create an empty database when opened on a path that + * does not exist. For a warehouse connection that turns a wrong path into a + * silent empty result set rather than an error, so opening is read-or-fail + * unless the caller passes `create: true`. + * + * The end-to-end proof — a populated store reached through `--dir` from an + * unrelated cwd, run against a compiled binary — lives in + * packages/opencode/test/altimate/store-path-resolution.test.ts. + */ +import { describe, test, expect, mock, afterEach } from "bun:test" +import { Database } from "bun:sqlite" +import * as fs from "fs" +import * as os from "os" +import * as path from "path" +import { allowsCreate, assertStoreExists, isLocalFilePath } from "../src/file-store" + +const CANARY_TABLE = "zorbulax_ledger" + +const tmpDirs: string[] = [] +function tmp(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "file-store-guard-")) + tmpDirs.push(dir) + return dir +} +afterEach(() => { + while (tmpDirs.length) fs.rmSync(tmpDirs.pop()!, { recursive: true, force: true }) +}) + +describe("isLocalFilePath", () => { + test("treats in-memory and remote targets as non-files", () => { + expect(isLocalFilePath(":memory:")).toBe(false) + expect(isLocalFilePath("")).toBe(false) + expect(isLocalFilePath("md:my_database")).toBe(false) + expect(isLocalFilePath("motherduck:my_database")).toBe(false) + expect(isLocalFilePath("s3://bucket/warehouse.duckdb")).toBe(false) + expect(isLocalFilePath("https://example.com/warehouse.duckdb")).toBe(false) + // A scheme from a DuckDB extension we have never heard of is still not a + // local file; the driver reports an unknown scheme rather than creating one. + expect(isLocalFilePath("ducklake:my_catalog")).toBe(false) + }) + + test("treats real paths — including Windows drive letters — as files", () => { + expect(isLocalFilePath("warehouse.duckdb")).toBe(true) + expect(isLocalFilePath("./data/warehouse.duckdb")).toBe(true) + expect(isLocalFilePath("/var/data/warehouse.duckdb")).toBe(true) + expect(isLocalFilePath("C:\\data\\warehouse.duckdb")).toBe(true) + }) + + test("only the exact `:memory:` is in-memory — `:memory:name` is a real file", () => { + // DuckDB writes a file literally named ":memory:named" for this path, and + // a colon-prefixed name is an ordinary file to both engines. Classifying + // them as non-files would let the guard be bypassed by a typo. + expect(isLocalFilePath(":memory:named")).toBe(true) + expect(isLocalFilePath(":memory")).toBe(true) + expect(isLocalFilePath(":foo")).toBe(true) + }) +}) + +describe("assertStoreExists", () => { + test("throws for a missing local file, naming the path it looked for", () => { + const missing = path.join(tmp(), "absent.duckdb") + expect(() => assertStoreExists({ type: "duckdb" }, missing, "DuckDB")).toThrow(missing) + expect(() => assertStoreExists({ type: "duckdb" }, missing, "DuckDB")).toThrow("not found") + expect(fs.existsSync(missing)).toBe(false) + }) + + test("throws for a missing `:memory:`-lookalike rather than letting it be created", () => { + const dir = tmp() + const lookalike = path.join(dir, ":memory:named") + expect(() => assertStoreExists({ type: "duckdb" }, lookalike, "DuckDB")).toThrow("not found") + expect(fs.existsSync(lookalike)).toBe(false) + }) + + test("passes for an existing file, an in-memory target, or an explicit create", () => { + const dir = tmp() + const present = path.join(dir, "present.duckdb") + fs.writeFileSync(present, "") + expect(() => assertStoreExists({ type: "duckdb" }, present, "DuckDB")).not.toThrow() + expect(() => assertStoreExists({ type: "duckdb" }, ":memory:", "DuckDB")).not.toThrow() + expect(() => + assertStoreExists({ type: "duckdb", create: true }, path.join(dir, "new.duckdb"), "DuckDB"), + ).not.toThrow() + }) + + test("allowsCreate only accepts a literal true", () => { + expect(allowsCreate({ type: "duckdb", create: true })).toBe(true) + expect(allowsCreate({ type: "duckdb", create: "true" })).toBe(false) + expect(allowsCreate({ type: "duckdb" })).toBe(false) + }) +}) + +describe("DuckDB driver create-on-open", () => { + test("refuses to open a missing store and creates nothing", async () => { + // The real duckdb addon is an optional dependency; the guard runs before + // any Database is constructed, so a stub is enough to prove it fires first. + let constructed = false + mock.module("duckdb", () => ({ + default: { + Database: class { + constructor(_p: string, optsOrCb: any, cb?: (err: Error | null) => void) { + constructed = true + const done = typeof optsOrCb === "function" ? optsOrCb : cb! + setTimeout(() => done(null), 0) + } + connect() { + return {} + } + close(cb: any) { + if (cb) cb(null) + } + }, + }, + })) + + const missing = path.join(tmp(), "absent.duckdb") + const { connect } = await import("../src/duckdb") + const connector = await connect({ type: "duckdb", path: missing }) + + await expect(connector.connect()).rejects.toThrow("not found") + expect(constructed).toBe(false) + expect(fs.existsSync(missing)).toBe(false) + }) +}) + +describe("SQLite driver create-on-open", () => { + test("reads an existing store", async () => { + const store = path.join(tmp(), "warehouse.db") + const seed = new Database(store, { create: true }) + seed.exec(`CREATE TABLE ${CANARY_TABLE}(id INTEGER)`) + seed.close() + + const { connect } = await import("../src/sqlite") + const connector = await connect({ type: "sqlite", path: store }) + await connector.connect() + const tables = await connector.listTables("main") + await connector.close() + + expect(tables.map((t) => t.name)).toContain(CANARY_TABLE) + }) + + test("refuses to open a missing store and creates nothing", async () => { + const missing = path.join(tmp(), "absent.db") + const { connect } = await import("../src/sqlite") + const connector = await connect({ type: "sqlite", path: missing }) + + await expect(connector.connect()).rejects.toThrow("not found") + expect(fs.existsSync(missing)).toBe(false) + }) + + test("refuses a read-only connection to a missing store even with create: true", async () => { + // A read-only open never creates, so `create` must not excuse the miss. + const missing = path.join(tmp(), "absent-readonly.db") + const { connect } = await import("../src/sqlite") + const connector = await connect({ type: "sqlite", path: missing, readonly: true, create: true }) + + await expect(connector.connect()).rejects.toThrow("not found") + expect(fs.existsSync(missing)).toBe(false) + }) + + test("creates only when the caller opts in", async () => { + const target = path.join(tmp(), "scratch.db") + const { connect } = await import("../src/sqlite") + const connector = await connect({ type: "sqlite", path: target, create: true }) + await connector.connect() + await connector.close() + + expect(fs.existsSync(target)).toBe(true) + }) +}) diff --git a/packages/opencode/src/altimate/native/connections/registry.ts b/packages/opencode/src/altimate/native/connections/registry.ts index 8be3bfc672..4f2735918e 100644 --- a/packages/opencode/src/altimate/native/connections/registry.ts +++ b/packages/opencode/src/altimate/native/connections/registry.ts @@ -7,14 +7,21 @@ * 3. ALTIMATE_CODE_CONN_* environment variables * * Connectors are created lazily via dynamic import of the appropriate driver. + * + * A relative `path` on a file-backed connection (duckdb, sqlite) is resolved + * once at load time against the directory that declared it — the global config + * resolves against ~/.altimate-code, a project config and the environment + * variables resolve against the project root. It is never re-resolved against + * the working directory at connect time, which `--dir` changes. */ import * as fs from "fs" import * as path from "path" import * as os from "os" +import { Instance } from "@/project/instance" import { Log } from "@/altimate/util/log" import type { ConnectionConfig, Connector } from "@altimateai/drivers" -import { normalizeConfig } from "@altimateai/drivers" +import { isLocalFilePath, normalizeConfig } from "@altimateai/drivers" import { resolveConfig, saveConnection } from "./credential-store" import { startTunnel, extractSshConfig, closeTunnel } from "./ssh-tunnel" import type { WarehouseInfo } from "../types" @@ -40,9 +47,87 @@ function globalConfigPath(): string { return path.join(os.homedir(), ".altimate-code", "connections.json") } +// altimate_change start — the project root, not the process working directory +/** + * The directory the current request is working in. + * + * A server or `run --attach` session never chdirs: it carries the project in + * the instance context and leaves the working directory wherever the server was + * launched (server.ts routes every request through `Instance.provide`). Reading + * `process.cwd()` there picks up the launch directory, which is somebody else's + * project. `Instance.directory` throws synchronously when there is no instance + * context — early CLI paths and unit tests — and the working directory is the + * right answer in exactly those cases, since `run --dir` has already chdir'd. + * + * The registry itself is still process-global: `loaded` latches on first + * access, so one server process serving two projects keeps the first project's + * configs for both. That predates this change — the base was simply the launch + * directory for everyone — and making the registry per-instance is a larger + * change than this fix. What this does guarantee is that whichever project + * loads resolves against itself, not against wherever the server was started. + */ +function projectRoot(): string { + try { + return Instance.directory + } catch { + return process.cwd() + } +} +// altimate_change end + function localConfigPath(): string { - return path.join(process.cwd(), ".altimate-code", "connections.json") + // altimate_change start — resolve against the project, not the launch cwd + return path.join(projectRoot(), ".altimate-code", "connections.json") + // altimate_change end +} + +// --------------------------------------------------------------------------- +// Store path resolution +// --------------------------------------------------------------------------- + +// altimate_change start — resolve file-backed store paths against a stable base +/** Driver types whose `path` field names a database file on local disk. */ +const FILE_STORE_TYPES = new Set(["duckdb", "sqlite"]) + +/** + * Absolutize the `path` of every file-backed connection in `entries` against + * `baseDir`, once, at load time. + * + * A relative store path must not follow the process working directory. `run` + * and `attach` call `process.chdir()` for `--dir` (see cli/cmd/run.ts), so a + * path resolved lazily at connect time points somewhere different depending on + * how the CLI was invoked — and both file-backed engines answer a miss by + * creating an empty database rather than failing. + * + * The base is the directory that declared the path, which is stable for the + * life of the config file: + * - global config -> ~/.altimate-code (the config file's own directory) + * - project config -> the project root (the parent of its .altimate-code) + * - environment -> the project root, the only directory an ambient + * variable can reasonably mean + */ +function resolveStorePaths( + entries: Record, + baseDir: string, +): Record { + const resolved: Record = {} + for (const [name, config] of Object.entries(entries)) { + const storePath = config?.path + const type = typeof config?.type === "string" ? config.type.toLowerCase() : "" + if ( + !FILE_STORE_TYPES.has(type) || + typeof storePath !== "string" || + !isLocalFilePath(storePath) || + path.isAbsolute(storePath) + ) { + resolved[name] = config + continue + } + resolved[name] = { ...config, path: path.resolve(baseDir, storePath) } + } + return resolved } +// altimate_change end // --------------------------------------------------------------------------- // Loading @@ -85,9 +170,13 @@ function loadFromEnv(): Record { export function load(): void { configs.clear() - const global = loadFromFile(globalConfigPath()) - const local = loadFromFile(localConfigPath()) - const env = loadFromEnv() + // altimate_change start — absolutize store paths against the directory that + // declared them, so a later process.chdir() (--dir) cannot move the store. + const base = projectRoot() + const global = resolveStorePaths(loadFromFile(globalConfigPath()), path.dirname(globalConfigPath())) + const local = resolveStorePaths(loadFromFile(localConfigPath()), base) + const env = resolveStorePaths(loadFromEnv(), base) + // altimate_change end // Merge: global < local < env for (const [name, config] of Object.entries(global)) { @@ -436,7 +525,11 @@ export async function add( // Normalize field names before saving so sensitive fields under alias // names (e.g., keyfileJson → credentials_json) are properly detected - const normalized = normalizeConfig(config) + // altimate_change start — a relative store path means "relative to where + // the user typed this"; persist it absolute so the saved global config is + // not silently re-pointed by the next invocation's working directory. + const normalized = resolveStorePaths({ [name]: normalizeConfig(config) }, projectRoot())[name] + // altimate_change end // Store credentials in keychain, get sanitized config const { sanitized, warnings } = await saveConnection(name, normalized) diff --git a/packages/opencode/src/altimate/native/local/schema-sync.ts b/packages/opencode/src/altimate/native/local/schema-sync.ts index a0b53a5838..9af941357a 100644 --- a/packages/opencode/src/altimate/native/local/schema-sync.ts +++ b/packages/opencode/src/altimate/native/local/schema-sync.ts @@ -96,7 +96,10 @@ export async function syncSchema(params: LocalSchemaSyncParams): Promise { +async function preValidateSql( + sql: string, + warehouse: string | undefined, + queryType: string, +): Promise { const startTime = Date.now() // Yield the event loop before heavy synchronous SQLite work so concurrent // tasks aren't blocked. Bun's sqlite API is sync and listColumns can touch diff --git a/packages/opencode/src/altimate/tools/warehouse-add.ts b/packages/opencode/src/altimate/tools/warehouse-add.ts index d31cb2f0a1..a0eb0668a1 100644 --- a/packages/opencode/src/altimate/tools/warehouse-add.ts +++ b/packages/opencode/src/altimate/tools/warehouse-add.ts @@ -31,10 +31,11 @@ export const WarehouseAddTool = Tool.define("warehouse_add", { - mysql: host, port, database, user, password, ssl (or ssl_ca, ssl_cert, ssl_key) - sqlserver: host, port, database, user, password, encrypt, trust_server_certificate - oracle: connection_string (or host, port, service_name), user, password -- duckdb: path (file path or ":memory:") -- sqlite: path (file path) +- duckdb: path (file path or ":memory:"), create (optional, default false) +- sqlite: path (file path), create (optional, default false) - clickhouse: host, port, database, user, password, protocol (http/https), connection_string, request_timeout, tls_ca_cert, tls_cert, tls_key, clickhouse_settings - trino: host, port, catalog, schema, user, password, protocol (http/https), connection_string, access_token, extra_headers +File-backed stores (duckdb, sqlite): the store must already exist — connecting never creates it, because an empty database answers every query with no rows. Pass "create": true only when the store is meant to be created. A relative "path" is resolved against the directory of the config that declares it (the global config resolves against ~/.altimate-code), never against the current working directory; prefer an absolute path. Snowflake auth examples: (1) Password: {"type":"snowflake","account":"xy12345","user":"admin","password":"secret","warehouse":"WH","database":"db"}. (2) Key-pair: {"type":"snowflake","account":"xy12345","user":"admin","private_key_path":"/path/rsa_key.p8","warehouse":"WH","database":"db"}. (3) OAuth: {"type":"snowflake","account":"xy12345","authenticator":"oauth","token":"","warehouse":"WH","database":"db"}. (4) SSO: {"type":"snowflake","account":"xy12345","user":"admin","authenticator":"externalbrowser","warehouse":"WH","database":"db"}. IMPORTANT: For private key file paths, always use "private_key_path" (not "private_key").`, ), diff --git a/packages/opencode/test/altimate/drivers-e2e.test.ts b/packages/opencode/test/altimate/drivers-e2e.test.ts index 77a857eaf3..36df1f3a54 100644 --- a/packages/opencode/test/altimate/drivers-e2e.test.ts +++ b/packages/opencode/test/altimate/drivers-e2e.test.ts @@ -304,7 +304,9 @@ describe("DuckDB Driver E2E", () => { const dbFile = join(tmpDir, "test.duckdb") try { const mod = await import("@altimateai/drivers/duckdb") - const fileConn = await mod.connect({ type: "duckdb", path: dbFile }) + // First open materializes the store, so it opts in to create; the + // reopen below deliberately does NOT, proving the file really persisted. + const fileConn = await mod.connect({ type: "duckdb", path: dbFile, create: true }) await fileConn.connect() await fileConn.execute("CREATE TABLE persist (x INT)") @@ -495,7 +497,8 @@ describe("SQLite Driver E2E", () => { tmpDir = mkdtempSync(join(tmpdir(), "sqlite-test-")) const dbFile = join(tmpDir, "test.sqlite") const mod = await import("@altimateai/drivers/sqlite") - connector = await mod.connect({ type: "sqlite", path: dbFile }) + // This suite materializes its own scratch store, so it opts in to create. + connector = await mod.connect({ type: "sqlite", path: dbFile, create: true }) await connector.connect() }) @@ -648,7 +651,7 @@ describe("SQLite Driver E2E", () => { const dbFile = join(tmpDir2, "close.sqlite") try { const mod = await import("@altimateai/drivers/sqlite") - const conn = await mod.connect({ type: "sqlite", path: dbFile }) + const conn = await mod.connect({ type: "sqlite", path: dbFile, create: true }) await conn.connect() await conn.execute("SELECT 1") await conn.close() diff --git a/packages/opencode/test/altimate/fixtures/store-path-probe.ts b/packages/opencode/test/altimate/fixtures/store-path-probe.ts new file mode 100644 index 0000000000..f7ee1cc93b --- /dev/null +++ b/packages/opencode/test/altimate/fixtures/store-path-probe.ts @@ -0,0 +1,100 @@ +/** + * Compiled-binary probe for warehouse store path resolution. + * + * This entrypoint is compiled with `bun build --compile` using the same + * options the production binary uses (see packages/opencode/script/build.ts): + * bundled sources, `duckdb` left external, no bunfig/dotenv autoload. It then + * reproduces the exact `--dir` handling of `altimate-code run` + * (packages/opencode/src/cli/cmd/run.ts) — `process.chdir(args.dir)` — before + * touching the connection registry. + * + * Running it from an unrelated cwd is the only way to observe the real + * resolution behaviour: a `bun test` run keeps the package's own node_modules + * reachable and its cwd is not the rig's, so it cannot see this defect. + * + * Output is a single line of JSON so the harness can assert on it. + */ + +import * as Registry from "../../../src/altimate/native/connections/registry" + +function arg(name: string): string | undefined { + const idx = process.argv.indexOf(`--${name}`) + if (idx === -1 || idx === process.argv.length - 1) return undefined + return process.argv[idx + 1] +} + +async function main() { + // Load the native duckdb addon before any chdir. In the shipped binary the + // `bin/altimate` wrapper makes `duckdb` resolvable via NODE_PATH; here it is + // bundled, and Bun locates its embedded `.node` relative to the startup cwd. + // Warming the module cache first keeps that resolution out of the experiment — + // the behaviour under test (path resolution and create-on-open) all happens + // inside `new duckdb.Database(...)`, well after this point. + await import("duckdb").catch(() => {}) + + const connection = arg("connection") ?? "probe" + const schema = arg("schema") ?? "main" + + // `--via dispatcher` (the default) drives the same entry point the CLI's + // sql_execute tool uses: Dispatcher.call -> sql.execute -> Registry -> driver. + // Reaching the driver any other way can pass while the product path is broken, + // which is exactly how a guard gets shipped that never runs. + const via = arg("via") ?? "dispatcher" + + const read = async (): Promise<{ tables: string[]; error?: string }> => { + if (via === "registry") { + const connector = await Registry.get(connection) + const tables = await connector.listTables(schema) + await connector.close() + return { tables: tables.map((t) => t.name).sort() } + } + // Import the native index, not the dispatcher module: the index is what + // installs the lazy registration hook in production. Importing the + // dispatcher alone yields "No native handler for sql.execute". + const { Dispatcher } = await import("../../../src/altimate/native") + const result = (await Dispatcher.call("sql.execute", { + sql: "SELECT name FROM sqlite_master WHERE type = 'table'", + warehouse: connection, + })) as { rows?: unknown[][]; error?: unknown } + if (result.error !== undefined) return { tables: [], error: String(result.error) } + return { tables: (result.rows ?? []).map((r) => String(r[0])).sort() } + } + + // `--instance-dir` models a server or `run --attach` request: the working + // directory stays where the process started and the project arrives through + // the instance context instead (server.ts wraps every request this way). + // `--dir` models the local `run --dir` path, which chdirs (run.ts). + const instanceDir = arg("instance-dir") + const dir = arg("dir") + if (!instanceDir && dir) process.chdir(dir) + + try { + const outcome = instanceDir + ? await ( + await import("../../../src/project/instance") + ).Instance.provide({ + directory: instanceDir, + fn: read, + }) + : await read() + console.log( + JSON.stringify( + outcome.error !== undefined + ? { ok: false, cwd: process.cwd(), error: outcome.error } + : { ok: true, cwd: process.cwd(), tables: outcome.tables }, + ), + ) + } catch (e) { + console.log( + JSON.stringify({ + ok: false, + cwd: process.cwd(), + error: e instanceof Error ? e.message : String(e), + }), + ) + } +} + +await main() +// The duckdb native addon keeps handles alive; exit explicitly. +process.exit(0) diff --git a/packages/opencode/test/altimate/schema-cache.test.ts b/packages/opencode/test/altimate/schema-cache.test.ts index 55c70f1d8b..2d5ac92bd2 100644 --- a/packages/opencode/test/altimate/schema-cache.test.ts +++ b/packages/opencode/test/altimate/schema-cache.test.ts @@ -644,7 +644,7 @@ describe("SQLite driver PRAGMA handling", () => { test("PRAGMA statements work without LIMIT clause error", async () => { const { connect } = await import("@altimateai/drivers/sqlite") const dbPath = join(tmpDir, "pragma-test.db") - const connector = await connect({ type: "sqlite", path: dbPath }) + const connector = await connect({ type: "sqlite", path: dbPath, create: true }) await connector.connect() // These should all work without "near LIMIT: syntax error" @@ -664,7 +664,7 @@ describe("SQLite driver PRAGMA handling", () => { test("SELECT statements still get LIMIT applied", async () => { const { connect } = await import("@altimateai/drivers/sqlite") const dbPath = join(tmpDir, "limit-test.db") - const connector = await connect({ type: "sqlite", path: dbPath }) + const connector = await connect({ type: "sqlite", path: dbPath, create: true }) await connector.connect() await connector.execute("CREATE TABLE nums (n INTEGER)") @@ -691,7 +691,7 @@ describe("SQLite driver readonly connections", () => { const dbPath = join(tmpDir, "readonly-test.db") // Create a database with data first - const writer = await connect({ type: "sqlite", path: dbPath }) + const writer = await connect({ type: "sqlite", path: dbPath, create: true }) await writer.connect() await writer.execute("CREATE TABLE items (id INTEGER, name TEXT)") await writer.execute("INSERT INTO items VALUES (1, 'test')") @@ -710,7 +710,7 @@ describe("SQLite driver readonly connections", () => { const dbPath = join(tmpDir, "readonly-write-test.db") // Create a database first - const writer = await connect({ type: "sqlite", path: dbPath }) + const writer = await connect({ type: "sqlite", path: dbPath, create: true }) await writer.connect() await writer.execute("CREATE TABLE items (id INTEGER)") await writer.close() diff --git a/packages/opencode/test/altimate/store-path-resolution.test.ts b/packages/opencode/test/altimate/store-path-resolution.test.ts new file mode 100644 index 0000000000..fec5d3870b --- /dev/null +++ b/packages/opencode/test/altimate/store-path-resolution.test.ts @@ -0,0 +1,294 @@ +/** + * Regression tests for warehouse store path resolution and create-on-open. + * + * Reported defect: `altimate-code run --dir ` made a populated DuckDB + * store read as empty, with no error. `--dir` calls `process.chdir()` + * (src/cli/cmd/run.ts), a relative store path in a connection config resolved + * against the new working directory, and the file-backed driver answered the + * miss by CREATING an empty database. Every query then succeeded and returned + * nothing. + * + * These tests run a COMPILED binary built with the production build options + * (packages/opencode/script/build.ts: bundled sources, warehouse SDKs external, + * no bunfig/dotenv autoload), invoked with `--dir` from an unrelated working + * directory. An in-process `bun test` cannot see this defect: the package's own + * node_modules stays reachable and the cwd is the test runner's, not the rig's. + * + * The store is driven through SQLite rather than DuckDB so the binary needs no + * native addon — `bun:sqlite` is built in. The resolution code under test is + * type-agnostic (see FILE_STORE_TYPES in native/connections/registry.ts) and + * both engines create-on-open, so SQLite exercises the same two defects. + * The DuckDB driver's own guard is covered in + * packages/drivers/test/file-store-guard.test.ts. + */ + +import { describe, expect, test, beforeAll, afterAll } from "bun:test" +import { Database } from "bun:sqlite" +import { spawnSync } from "child_process" +import * as fs from "fs" +import * as os from "os" +import * as path from "path" + +const OPENCODE_ROOT = path.resolve(import.meta.dir, "../..") +const PROBE = path.join(OPENCODE_ROOT, "test/altimate/fixtures/store-path-probe.ts") + +/** An unguessable name — a pass cannot come from anything but reading the store. */ +const CANARY_TABLE = "zorbulax_ledger" + +let rig: string +let binary: string + +/** Compile the probe the way the shipped binary is compiled. */ +function compileProbe(outfile: string) { + const result = spawnSync( + process.execPath, + [ + "build", + "--compile", + "--conditions=browser", + "--tsconfig-override", + "./tsconfig.json", + "--target=bun", + // Match script/build.ts's compile options exactly. + "--no-compile-autoload-bunfig", + "--no-compile-autoload-dotenv", + "--compile-autoload-tsconfig", + "--compile-autoload-package-json", + // Mirrors optionalExternals in script/build.ts — the warehouse SDKs are + // native addons the user installs on demand, never bundled. + ...[ + "pg", + "snowflake-sdk", + "@google-cloud/bigquery", + "@databricks/sql", + "mysql2", + "mssql", + "oracledb", + "duckdb", + "keytar", + "ssh2", + "dockerode", + ].flatMap((pkg) => ["--external", pkg]), + "--define", + "OPENCODE_VERSION='0.0.0-test'", + "--define", + "OPENCODE_CHANNEL='test'", + "--define", + "OPENCODE_LIBC=undefined", + "--define", + "OPENCODE_MIGRATIONS=[]", + "--define", + "OPENCODE_BUILTIN_SKILLS=[]", + "--define", + "OPENCODE_CHANGELOG=[]", + "--outfile", + outfile, + PROBE, + ], + { cwd: OPENCODE_ROOT, encoding: "utf-8" }, + ) + if (result.status !== 0 || !fs.existsSync(outfile)) { + throw new Error(`probe compile failed (${result.status}):\n${result.stdout}\n${result.stderr}`) + } +} + +/** Run the compiled binary from `cwd`, with `home` as its HOME. */ +function runProbe(args: string[], opts: { cwd: string; home: string }) { + // `ALTIMATE_CODE_CONN_*` variables override both config files, so an ambient + // one on the developer's or CI's environment would decide these assertions + // instead of the config the test wrote. Strip them. + const env: Record = {} + for (const [key, value] of Object.entries(process.env)) { + if (!key.startsWith("ALTIMATE_CODE_CONN_")) env[key] = value + } + + const result = spawnSync(binary, args, { + cwd: opts.cwd, + encoding: "utf-8", + env: { + ...env, + HOME: opts.home, + USERPROFILE: opts.home, + // Global.Path derives data/cache/config/state from xdg-basedir, which + // prefers XDG_* over HOME. Leaving those ambient would put this probe's + // project storage outside the rig on any machine that sets them, so the + // arm that boots an instance would depend on shared state and leave dirs + // behind. Pin them inside the rig alongside HOME. + XDG_DATA_HOME: path.join(opts.home, ".local", "share"), + XDG_CACHE_HOME: path.join(opts.home, ".cache"), + XDG_CONFIG_HOME: path.join(opts.home, ".config"), + XDG_STATE_HOME: path.join(opts.home, ".local", "state"), + OPENCODE_TEST_HOME: opts.home, + ALTIMATE_TELEMETRY_DISABLED: "1", + OPENCODE_DISABLE_AUTOUPDATE: "1", + }, + }) + const line = (result.stdout ?? "") + .split("\n") + .reverse() + .find((l) => l.trim().startsWith("{")) + if (!line) throw new Error(`probe produced no JSON:\n${result.stdout}\n${result.stderr}`) + return JSON.parse(line) as { ok: boolean; cwd: string; tables?: string[]; error?: string } +} + +/** Every file under `dir` that looks like a database, ignoring `skip`. */ +function databaseFiles(dir: string, skip: string[] = []): string[] { + const found: string[] = [] + const walk = (current: string) => { + for (const entry of fs.readdirSync(current, { withFileTypes: true })) { + const full = path.join(current, entry.name) + if (entry.isDirectory()) walk(full) + else if (/\.(db|sqlite|duckdb)(-wal|-shm|\.wal)?$/.test(entry.name) && !skip.includes(full)) found.push(full) + } + } + walk(dir) + return found +} + +/** Write a connections.json into `home`'s global config directory. */ +function writeGlobalConfig(home: string, config: Record) { + const dir = path.join(home, ".altimate-code") + fs.mkdirSync(dir, { recursive: true }) + fs.writeFileSync(path.join(dir, "connections.json"), JSON.stringify(config, null, 2)) +} + +// Compiling the probe takes ~2s locally; allow generous headroom on slow CI +// machines so the hook cannot time out and mask a real result. +beforeAll(() => { + rig = fs.mkdtempSync(path.join(os.tmpdir(), "store-path-")) + binary = path.join(rig, "probe-bin") + compileProbe(binary) +}, 180_000) + +afterAll(() => { + if (rig) fs.rmSync(rig, { recursive: true, force: true }) +}) + +describe("warehouse store path resolution", () => { + test("a populated store reached through --dir from an unrelated cwd returns its real tables", () => { + const home = path.join(rig, "case1-home") + const project = path.join(rig, "case1-project") + const unrelatedCwd = path.join(rig, "case1-cwd") + fs.mkdirSync(path.join(home, ".altimate-code"), { recursive: true }) + fs.mkdirSync(project, { recursive: true }) + fs.mkdirSync(unrelatedCwd, { recursive: true }) + + // The store lives beside the global config that names it, under a RELATIVE + // path — the shape that made the reported case silently read as empty. + const store = path.join(home, ".altimate-code", "warehouse.db") + const seed = new Database(store, { create: true }) + seed.exec(`CREATE TABLE ${CANARY_TABLE}(id INTEGER, memo TEXT)`) + seed.exec(`INSERT INTO ${CANARY_TABLE} VALUES (1, 'real')`) + seed.close() + + writeGlobalConfig(home, { probe: { type: "sqlite", path: "warehouse.db" } }) + + const result = runProbe(["--connection", "probe", "--dir", project], { cwd: unrelatedCwd, home }) + + expect(result.cwd).toBe(fs.realpathSync(project)) + expect(result.ok).toBe(true) + expect(result.tables).toContain(CANARY_TABLE) + + // Nothing may have been conjured under --dir or the invoking directory. + expect(databaseFiles(project)).toEqual([]) + expect(databaseFiles(unrelatedCwd)).toEqual([]) + }) + + test("a missing store fails loudly instead of reading as empty", () => { + const home = path.join(rig, "case2-home") + const project = path.join(rig, "case2-project") + const absent = path.join(rig, "case2-absent") + fs.mkdirSync(path.join(home, ".altimate-code"), { recursive: true }) + fs.mkdirSync(project, { recursive: true }) + fs.mkdirSync(absent, { recursive: true }) + + const missing = path.join(absent, "definitely-absent.db") + writeGlobalConfig(home, { probe: { type: "sqlite", path: missing } }) + + const result = runProbe(["--connection", "probe", "--dir", project], { cwd: project, home }) + + expect(result.ok).toBe(false) + expect(result.error).toContain("not found") + expect(result.error).toContain("definitely-absent.db") + + // The whole point: no database was conjured at the missing path. + expect(fs.existsSync(missing)).toBe(false) + expect(databaseFiles(absent)).toEqual([]) + expect(databaseFiles(project)).toEqual([]) + }) + + test("a project-local config resolves against the --dir project, not the invoking cwd", () => { + const home = path.join(rig, "case5-home") + const project = path.join(rig, "case5-project") + const unrelatedCwd = path.join(rig, "case5-cwd") + fs.mkdirSync(path.join(home, ".altimate-code"), { recursive: true }) + fs.mkdirSync(path.join(project, ".altimate-code"), { recursive: true }) + fs.mkdirSync(unrelatedCwd, { recursive: true }) + + const store = new Database(path.join(project, "warehouse.db"), { create: true }) + store.exec(`CREATE TABLE ${CANARY_TABLE}(id INTEGER)`) + store.close() + + fs.writeFileSync( + path.join(project, ".altimate-code", "connections.json"), + JSON.stringify({ probe: { type: "sqlite", path: "warehouse.db" } }), + ) + + const result = runProbe(["--connection", "probe", "--dir", project], { cwd: unrelatedCwd, home }) + + expect(result.ok).toBe(true) + expect(result.tables).toContain(CANARY_TABLE) + expect(databaseFiles(unrelatedCwd)).toEqual([]) + }) + + test("a server-style request resolves against its instance directory, not the launch cwd", () => { + // A server or `run --attach` request never chdirs: the project arrives in + // the instance context while the working directory stays where the process + // was started. Reading process.cwd() there picks up the launch directory, + // which is somebody else's project. + const home = path.join(rig, "case6-home") + const project = path.join(rig, "case6-project") + const launchCwd = path.join(rig, "case6-launch-cwd") + fs.mkdirSync(path.join(home, ".altimate-code"), { recursive: true }) + fs.mkdirSync(path.join(project, ".altimate-code"), { recursive: true }) + fs.mkdirSync(path.join(launchCwd, ".altimate-code"), { recursive: true }) + + const real = new Database(path.join(project, "warehouse.db"), { create: true }) + real.exec(`CREATE TABLE ${CANARY_TABLE}(id INTEGER)`) + real.close() + + // A decoy project config and store sit in the launch directory. + const decoy = new Database(path.join(launchCwd, "warehouse.db"), { create: true }) + decoy.exec("CREATE TABLE decoy_table(id INTEGER)") + decoy.close() + for (const dir of [project, launchCwd]) { + fs.writeFileSync( + path.join(dir, ".altimate-code", "connections.json"), + JSON.stringify({ probe: { type: "sqlite", path: "warehouse.db" } }), + ) + } + + const result = runProbe(["--connection", "probe", "--instance-dir", project], { cwd: launchCwd, home }) + + expect(result.cwd).toBe(fs.realpathSync(launchCwd)) + expect(result.ok).toBe(true) + expect(result.tables).toContain(CANARY_TABLE) + expect(result.tables).not.toContain("decoy_table") + }) + + test("an explicit create: true still materializes a store", () => { + const home = path.join(rig, "case3-home") + const project = path.join(rig, "case3-project") + fs.mkdirSync(path.join(home, ".altimate-code"), { recursive: true }) + fs.mkdirSync(project, { recursive: true }) + + const target = path.join(project, "scratch.db") + writeGlobalConfig(home, { probe: { type: "sqlite", path: target, create: true } }) + + const result = runProbe(["--connection", "probe"], { cwd: project, home }) + + expect(result.ok).toBe(true) + expect(result.tables).toEqual([]) + expect(fs.existsSync(target)).toBe(true) + }) +}) diff --git a/packages/opencode/test/altimate/warehouse-failure-visibility.test.ts b/packages/opencode/test/altimate/warehouse-failure-visibility.test.ts new file mode 100644 index 0000000000..d8fc90753b --- /dev/null +++ b/packages/opencode/test/altimate/warehouse-failure-visibility.test.ts @@ -0,0 +1,158 @@ +/** + * A warehouse failure must never reach the agent as an empty result. + * + * Field report against the real rig: a compiled binary returned "no tables" + * with no error and no file created anywhere on disk. The cause was not the + * silent-creation path — it is that a failure was shaped like success all the + * way to the model: + * + * 1. `packages/drivers/src/duckdb.ts` read `(config.path as string) ?? ":memory:"`, + * so a config that arrived without a `path` became an in-memory database. + * Nothing is created, nothing is on disk, every query returns no rows. + * 2. `sql.execute` never throws. It catches every connection and query error + * and returns `{ columns: [], rows: [], row_count: 0, error }` + * (native/connections/register.ts). + * 3. `formatResult()` rendered that as the literal string `"(0 rows)"` and + * never read `error` — so the agent saw a healthy empty table. + * + * The layers are tested where each one breaks: the guard against the real + * registry, and the rendering against a stubbed `sql.execute` that returns the + * error-carrying shape the real handler produces. The stub is deliberate — + * `Dispatcher.reset()` leaks between test files in Bun, and the true + * end-to-end through the live dispatcher is covered by the compiled-binary + * test in store-path-resolution.test.ts, which runs in its own process. + */ + +import { afterEach, beforeEach, describe, expect, test } from "bun:test" +import { Database } from "bun:sqlite" +import * as fs from "fs" +import * as os from "os" +import * as path from "path" +import { initTool } from "./tool-fixture" +import * as Dispatcher from "../../src/altimate/native/dispatcher" +import * as Registry from "../../src/altimate/native/connections/registry" +import { SqlExecuteTool } from "../../src/altimate/tools/sql-execute" + +/** Unguessable, so a pass cannot come from anything but the real store. */ +const CANARY_TABLE = "zorbulax_ledger" + +const ctx = { + sessionID: "test-session", + messageID: "test-message", + callID: "test-call", + agent: "test", + abort: new AbortController().signal, + messages: [], + metadata: () => {}, +} + +const tmpDirs: string[] = [] +function tmp(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "warehouse-failure-")) + tmpDirs.push(dir) + return dir +} + +beforeEach(() => { + process.env.ALTIMATE_TELEMETRY_DISABLED = "true" + Registry.reset() +}) + +afterEach(() => { + Registry.reset() + while (tmpDirs.length) fs.rmSync(tmpDirs.pop()!, { recursive: true, force: true }) +}) + +const LIST_TABLES = `SELECT name FROM sqlite_master WHERE type = 'table'` + +describe("a file-backed connection that cannot be resolved fails loudly", () => { + test("a config with no path is rejected instead of becoming an in-memory database", async () => { + // The reported shape: nothing on disk, no error, every query empty. A + // `?? ":memory:"` default turns any failure to carry a path into a + // successful query against nothing. + Registry.setConfigs({ wh: { type: "sqlite" } }) + + await expect(Registry.get("wh")).rejects.toThrow(/path/) + }) + + test("a missing store file is rejected, and no store is conjured", async () => { + const missing = path.join(tmp(), "definitely-absent.db") + Registry.setConfigs({ wh: { type: "sqlite", path: missing } }) + + await expect(Registry.get("wh")).rejects.toThrow(/not found/) + expect(fs.existsSync(missing)).toBe(false) + }) + + test("an explicit :memory: is still honoured — it just has to be asked for", async () => { + Registry.setConfigs({ wh: { type: "sqlite", path: ":memory:" } }) + + const connector = await Registry.get("wh") + const result = await connector.execute("SELECT 1 AS n") + expect(result.rows[0][0]).toBe(1) + }) + + test("a populated store still opens and returns its real tables", async () => { + const store = path.join(tmp(), "warehouse.db") + const seed = new Database(store, { create: true }) + seed.exec(`CREATE TABLE ${CANARY_TABLE}(id INTEGER)`) + seed.close() + Registry.setConfigs({ wh: { type: "sqlite", path: store } }) + + const connector = await Registry.get("wh") + const result = await connector.execute(LIST_TABLES) + expect(result.rows.map((r) => r[0])).toContain(CANARY_TABLE) + }) +}) + +describe("sql_execute renders a warehouse failure as a failure", () => { + // Reset first, then register: `call()` runs the lazy registration hook + // before dispatching, and that would re-import the real handler and clobber + // the stub. This is the same order tool-response-normalization.test.ts uses. + beforeEach(() => { + Dispatcher.reset() + }) + + /** The exact shape sql.execute returns for any connection or query error. */ + function stubFailure(message: string) { + Dispatcher.register("sql.execute" as any, async () => ({ + columns: [], + rows: [], + row_count: 0, + truncated: false, + error: message, + })) + } + + test("an error-carrying result is reported, not printed as (0 rows)", async () => { + // This is the step that turned a broken connection into "no tables": + // formatResult() returned "(0 rows)" for row_count === 0 and never looked + // at `error`, so the model was told the warehouse was simply empty. + stubFailure('SQLite database file not found: "/nowhere/absent.db".') + + const tool = await initTool(SqlExecuteTool) + const result = await tool.execute({ query: LIST_TABLES }, ctx as any) + + expect(result.output).not.toContain("(0 rows)") + expect(result.title).toContain("ERROR") + expect(result.output).toContain("not found") + expect(result.metadata.error).toBeDefined() + }) + + test("a genuinely empty result set is still reported as empty, not as an error", async () => { + // The distinction that matters: zero rows from a healthy warehouse is not + // a failure, and must not start reporting as one. + Dispatcher.register("sql.execute" as any, async () => ({ + columns: ["name"], + rows: [], + row_count: 0, + truncated: false, + })) + + const tool = await initTool(SqlExecuteTool) + const result = await tool.execute({ query: LIST_TABLES }, ctx as any) + + expect(result.title).not.toContain("ERROR") + expect(result.output).toContain("(0 rows)") + expect(result.metadata.error).toBeUndefined() + }) +})