From 5cb074ece5d0b39add0c658c446c2d61c0387f06 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 12 Jul 2026 22:34:53 -0700 Subject: [PATCH] refactor: dedupe small async primitives (#105961) * refactor(sqlite): remove unused async Kysely driver Co-authored-by: Sarah Fortune * refactor(media): share promise memoization * chore(sqlite): prune stale Kysely guardrails * chore(media): lower runner LOC baseline * style(shared): format promise memo tests * chore(deadcode): refresh main export baseline --------- Co-authored-by: Sarah Fortune --- scripts/check-kysely-guardrails.mjs | 10 +- scripts/deadcode-exports.baseline.mjs | 1 + scripts/ts-max-loc-baseline-v2.json | 2 +- src/infra/kysely-node-sqlite.test.ts | 170 --------------------- src/infra/kysely-node-sqlite.ts | 198 ++++--------------------- src/infra/kysely-sync.test.ts | 39 ++++- src/infra/kysely-sync.ts | 73 +-------- src/media-understanding/local-audio.ts | 21 +-- src/media-understanding/runner.ts | 21 +-- src/shared/lazy-promise.test.ts | 22 +++ src/shared/lazy-promise.ts | 15 ++ 11 files changed, 114 insertions(+), 458 deletions(-) delete mode 100644 src/infra/kysely-node-sqlite.test.ts diff --git a/scripts/check-kysely-guardrails.mjs b/scripts/check-kysely-guardrails.mjs index c83f4b0d7303..0ad4819ce4d5 100644 --- a/scripts/check-kysely-guardrails.mjs +++ b/scripts/check-kysely-guardrails.mjs @@ -19,15 +19,9 @@ const ts = require("typescript"); const repoRoot = resolveRepoRoot(import.meta.url); const sourceRoots = [path.join(repoRoot, "src")]; -const kyselyRawAllowPaths = new Set([ - "src/infra/kysely-node-sqlite.test.ts", - "src/infra/kysely-sync.ts", -]); +const kyselyRawAllowPaths = new Set(["src/infra/kysely-sync.ts"]); -const compiledRawAllowPaths = new Set([ - "src/infra/kysely-node-sqlite.ts", - "src/infra/kysely-node-sqlite.test.ts", -]); +const compiledRawAllowPaths = new Set(["src/infra/kysely-node-sqlite.ts"]); const rawSqliteAllowPathGroups = { "native Kysely adapter and sync execution": [ diff --git a/scripts/deadcode-exports.baseline.mjs b/scripts/deadcode-exports.baseline.mjs index 3ad16b8279bc..340f9b848bf5 100644 --- a/scripts/deadcode-exports.baseline.mjs +++ b/scripts/deadcode-exports.baseline.mjs @@ -3114,6 +3114,7 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [ "src/gateway/session-transcript-readers.ts: readRecentSessionUsageFromTranscriptAsync", "src/gateway/session-transcript-readers.ts: readSessionMessageCount", "src/gateway/session-transcript-readers.ts: readSessionMessages", + "src/gateway/session-transcript-readers.ts: ResolvedTranscriptReadTarget", "src/gateway/session-transcript-readers.ts: visitSessionMessages", "src/gateway/session-utils.fs.ts: archiveFileOnDisk", "src/gateway/session-utils.fs.ts: archiveSessionTranscripts", diff --git a/scripts/ts-max-loc-baseline-v2.json b/scripts/ts-max-loc-baseline-v2.json index cfed77252f02..cb3d309f0fe1 100644 --- a/scripts/ts-max-loc-baseline-v2.json +++ b/scripts/ts-max-loc-baseline-v2.json @@ -1025,7 +1025,7 @@ "src/media-understanding/attachments.cache.ts": 523, "src/media-understanding/image.ts": 678, "src/media-understanding/runner.entries.ts": 1122, - "src/media-understanding/runner.ts": 1117, + "src/media-understanding/runner.ts": 1106, "src/media-understanding/shared.ts": 689, "src/media/fetch.ts": 689, "src/media/parse.ts": 725, diff --git a/src/infra/kysely-node-sqlite.test.ts b/src/infra/kysely-node-sqlite.test.ts deleted file mode 100644 index 021870541d0a..000000000000 --- a/src/infra/kysely-node-sqlite.test.ts +++ /dev/null @@ -1,170 +0,0 @@ -// Covers the Kysely dialect backed by Node sqlite. -import { DatabaseSync } from "node:sqlite"; -import { CompiledQuery, Kysely, sql, type Generated } from "kysely"; -import { afterEach, describe, expect, it, vi } from "vitest"; -import { NodeSqliteKyselyDialect } from "./kysely-node-sqlite.js"; - -type TestDatabase = { - person: { - id: Generated; - name: string; - }; -}; - -describe("NodeSqliteKyselyDialect", () => { - let db: Kysely | undefined; - - afterEach(async () => { - await db?.destroy(); - db = undefined; - }); - - it("uses node:sqlite with raw row-returning queries and returning clauses", async () => { - db = await createTestDb(); - - await expect(db.selectFrom("person").selectAll().execute()).resolves.toEqual([ - { id: 1, name: "Ada" }, - ]); - await expect(sql`select name from person where id = ${1}`.execute(db)).resolves.toEqual({ - rows: [{ name: "Ada" }], - }); - await expect( - db.insertInto("person").values({ name: "Grace" }).returning(["id", "name"]).execute(), - ).resolves.toEqual([{ id: 2, name: "Grace" }]); - await expect( - sql`insert into person (name) values ('Lin') returning *`.execute(db), - ).resolves.toEqual({ - rows: [{ id: 3, name: "Lin" }], - }); - - const ignoredInsert = await sql` - insert or ignore into person (id, name) values (${1}, ${"Ada Again"}) - `.execute(db); - expect(ignoredInsert.insertId).toBeUndefined(); - expect(ignoredInsert.numAffectedRows).toBe(0n); - - const update = await sql`update person set name = ${"Ada Lovelace"} where id = ${1}`.execute( - db, - ); - expect(update.insertId).toBeUndefined(); - expect(update.numAffectedRows).toBe(1n); - }); - - it("creates the database lazily and runs the connection hook once", async () => { - const sqlite = new DatabaseSync(":memory:"); - const createDatabase = vi.fn(() => sqlite); - const onCreateConnection = vi.fn(async (connection) => { - await connection.executeQuery(CompiledQuery.raw("pragma user_version = 7")); - }); - - db = new Kysely({ - dialect: new NodeSqliteKyselyDialect({ - database: createDatabase, - onCreateConnection, - }), - }); - - await expect(sql<{ user_version: number }>`pragma user_version`.execute(db)).resolves.toEqual({ - rows: [{ user_version: 7 }], - }); - expect(createDatabase).toHaveBeenCalledTimes(1); - expect(onCreateConnection).toHaveBeenCalledTimes(1); - }); - - it("returns insert metadata only for changed insert statements", async () => { - db = new Kysely({ - dialect: new NodeSqliteKyselyDialect({ - database: new DatabaseSync(":memory:"), - }), - }); - await createPersonTable(db); - - const insertResult = await db - .insertInto("person") - .values({ name: "Ada" }) - .executeTakeFirstOrThrow(); - expect(insertResult.insertId).toBe(1n); - expect(insertResult.numInsertedOrUpdatedRows).toBe(1n); - - const updateResult = await db - .updateTable("person") - .set({ name: "Ada Lovelace" }) - .where("id", "=", 1) - .executeTakeFirstOrThrow(); - expect(updateResult.numUpdatedRows).toBe(1n); - - const ignoredInsert = await sql` - insert or ignore into person (id, name) values (${1}, ${"Ada Again"}) - `.execute(db); - expect(ignoredInsert.insertId).toBeUndefined(); - expect(ignoredInsert.numAffectedRows).toBe(0n); - }); - - it("rolls back transactions and controlled savepoints", async () => { - db = new Kysely({ - dialect: new NodeSqliteKyselyDialect({ - database: new DatabaseSync(":memory:"), - }), - }); - await createPersonTable(db); - - await expect( - db.transaction().execute(async (trx) => { - await trx.insertInto("person").values({ name: "Rollback" }).execute(); - throw new Error("rollback outer"); - }), - ).rejects.toThrow("rollback outer"); - await expect(db.selectFrom("person").selectAll().execute()).resolves.toStrictEqual([]); - - const trx = await db.startTransaction().execute(); - await trx.insertInto("person").values({ name: "Ada" }).execute(); - const afterAda = await trx.savepoint("after_ada").execute(); - await afterAda.insertInto("person").values({ name: "Grace" }).execute(); - const afterRollback = await afterAda.rollbackToSavepoint("after_ada").execute(); - await afterRollback.insertInto("person").values({ name: "Lin" }).execute(); - await afterRollback.commit().execute(); - - await expect(db.selectFrom("person").select("name").orderBy("id").execute()).resolves.toEqual([ - { name: "Ada" }, - { name: "Lin" }, - ]); - }); - - it("streams selected rows through node:sqlite iteration", async () => { - db = await createTestDb(); - await db - .insertInto("person") - .values([{ name: "Grace" }, { name: "Lin" }]) - .execute(); - - const rows: Array<{ id: number; name: string }> = []; - for await (const row of db.selectFrom("person").selectAll().orderBy("id").stream(1)) { - rows.push(row); - } - - expect(rows).toEqual([ - { id: 1, name: "Ada" }, - { id: 2, name: "Grace" }, - { id: 3, name: "Lin" }, - ]); - }); -}); - -async function createTestDb(): Promise> { - const testDb = new Kysely({ - dialect: new NodeSqliteKyselyDialect({ - database: new DatabaseSync(":memory:"), - }), - }); - await createPersonTable(testDb); - await testDb.insertInto("person").values({ name: "Ada" }).execute(); - return testDb; -} - -async function createPersonTable(testDb: Kysely): Promise { - await testDb.schema - .createTable("person") - .addColumn("id", "integer", (col) => col.primaryKey().autoIncrement()) - .addColumn("name", "text", (col) => col.notNull()) - .execute(); -} diff --git a/src/infra/kysely-node-sqlite.ts b/src/infra/kysely-node-sqlite.ts index 16679bc68432..5fe9c5c7e5b8 100644 --- a/src/infra/kysely-node-sqlite.ts +++ b/src/infra/kysely-node-sqlite.ts @@ -1,5 +1,4 @@ -// Adapts Node's sync sqlite API to Kysely. -import type { DatabaseSync, SQLInputValue } from "node:sqlite"; +// Build-only node:sqlite dialect for the synchronous execution helpers. import type { DatabaseConnection, DatabaseIntrospector, @@ -8,40 +7,14 @@ import type { Driver, Kysely, QueryCompiler, - QueryResult, TransactionSettings, } from "kysely"; -import { - CompiledQuery, - IdentifierNode, - RawNode, - SqliteAdapter, - SqliteIntrospector, - SqliteQueryCompiler, - createQueryId, -} from "kysely"; +import { SqliteAdapter, SqliteIntrospector, SqliteQueryCompiler } from "kysely"; -// Kysely dialect for Node's synchronous node:sqlite API. The driver serializes -// connection use because DatabaseSync is single-connection and blocking. -type MaybePromise = T | Promise; - -/** Configuration for the node:sqlite Kysely dialect. */ -export type NodeSqliteKyselyDialectConfig = { - database: DatabaseSync | (() => MaybePromise); - onCreateConnection?: (connection: DatabaseConnection) => MaybePromise; - transactionMode?: "deferred" | "immediate" | "exclusive"; -}; - -/** Kysely dialect backed by a node:sqlite DatabaseSync instance. */ +/** Kysely dialect that compiles node:sqlite queries without executing them. */ export class NodeSqliteKyselyDialect implements Dialect { - readonly #config: NodeSqliteKyselyDialectConfig; - - constructor(config: NodeSqliteKyselyDialectConfig) { - this.#config = Object.freeze({ ...config }); - } - createDriver(): Driver { - return new NodeSqliteKyselyDriver(this.#config); + return new CompileOnlySqliteDriver(); } createQueryCompiler(): QueryCompiler { @@ -49,7 +22,7 @@ export class NodeSqliteKyselyDialect implements Dialect { } createAdapter(): DialectAdapter { - return new SqliteAdapter(); + return new CompileOnlySqliteAdapter(); } createIntrospector(db: Kysely): DatabaseIntrospector { @@ -57,163 +30,44 @@ export class NodeSqliteKyselyDialect implements Dialect { } } -class NodeSqliteKyselyDriver implements Driver { - readonly #config: NodeSqliteKyselyDialectConfig; - readonly #mutex = new ConnectionMutex(); - - #db?: DatabaseSync; - #connection?: DatabaseConnection; - - constructor(config: NodeSqliteKyselyDialectConfig) { - this.#config = Object.freeze({ ...config }); - } - - async init(): Promise { - this.#db = - typeof this.#config.database === "function" - ? await this.#config.database() - : this.#config.database; - - this.#connection = new NodeSqliteKyselyConnection(this.#db); - await this.#config.onCreateConnection?.(this.#connection); - } +class CompileOnlySqliteDriver implements Driver { + async init(): Promise {} async acquireConnection(): Promise { - // Kysely expects async acquisition even though node:sqlite is sync; the - // mutex preserves transaction ordering across concurrent callers. - await this.#mutex.lock(); - return this.#connection!; + throw createCompileOnlyExecutionError(); } async beginTransaction( - connection: DatabaseConnection, + _connection: DatabaseConnection, _settings: TransactionSettings, ): Promise { - const mode = this.#config.transactionMode ?? "deferred"; - await connection.executeQuery(CompiledQuery.raw(`begin ${mode}`)); + throw createCompileOnlyExecutionError(); } - async commitTransaction(connection: DatabaseConnection): Promise { - await connection.executeQuery(CompiledQuery.raw("commit")); + async commitTransaction(_connection: DatabaseConnection): Promise { + throw createCompileOnlyExecutionError(); } - async rollbackTransaction(connection: DatabaseConnection): Promise { - await connection.executeQuery(CompiledQuery.raw("rollback")); + async rollbackTransaction(_connection: DatabaseConnection): Promise { + throw createCompileOnlyExecutionError(); } - async savepoint( - connection: DatabaseConnection, - savepointName: string, - compileQuery: QueryCompiler["compileQuery"], - ): Promise { - await connection.executeQuery( - compileQuery(createSavepointCommand("savepoint", savepointName), createQueryId()), - ); - } + async releaseConnection(_connection: DatabaseConnection): Promise {} - async rollbackToSavepoint( - connection: DatabaseConnection, - savepointName: string, - compileQuery: QueryCompiler["compileQuery"], - ): Promise { - await connection.executeQuery( - compileQuery(createSavepointCommand("rollback to", savepointName), createQueryId()), - ); - } - - async releaseSavepoint( - connection: DatabaseConnection, - savepointName: string, - compileQuery: QueryCompiler["compileQuery"], - ): Promise { - await connection.executeQuery( - compileQuery(createSavepointCommand("release", savepointName), createQueryId()), - ); - } - - async releaseConnection(): Promise { - this.#mutex.unlock(); - } - - async destroy(): Promise { - this.#db?.close(); - this.#db = undefined; - this.#connection = undefined; - } + async destroy(): Promise {} } -class NodeSqliteKyselyConnection implements DatabaseConnection { - readonly #db: DatabaseSync; - - constructor(db: DatabaseSync) { - this.#db = db; - } - - executeQuery(compiledQuery: CompiledQuery): Promise> { - const { sql, parameters } = compiledQuery; - const stmt = this.#db.prepare(sql); - const sqliteParameters = parameters as SQLInputValue[]; - - if (stmt.columns().length > 0) { - return Promise.resolve({ rows: stmt.all(...sqliteParameters) as O[] }); - } - - const { changes, lastInsertRowid } = stmt.run(...sqliteParameters); - const baseResult: QueryResult = { - numAffectedRows: BigInt(changes), - rows: [], - }; - if (isInsertStatement(sql) && changes > 0) { - return Promise.resolve({ - ...baseResult, - insertId: BigInt(lastInsertRowid), - }); - } - return Promise.resolve(baseResult); - } - - async *streamQuery( - compiledQuery: CompiledQuery, - _chunkSize?: number, - ): AsyncIterableIterator> { - const { sql, parameters } = compiledQuery; - const stmt = this.#db.prepare(sql); - - for (const row of stmt.iterate(...(parameters as SQLInputValue[]))) { - yield { rows: [row as O] }; - } - } +function createCompileOnlyExecutionError(): Error { + return new Error( + "getNodeSqliteKysely() returns a compile-only Kysely facade; use executeSqliteQuerySync() to execute node:sqlite queries.", + ); } -function isInsertStatement(sql: string): boolean { - return sql.trimStart().toLowerCase().startsWith("insert"); -} - -function createSavepointCommand(command: string, savepointName: string): RawNode { - return RawNode.createWithChildren([ - RawNode.createWithSql(`${command} `), - IdentifierNode.create(savepointName), - ]); -} - -class ConnectionMutex { - #promise?: Promise; - #resolve?: () => void; - - async lock(): Promise { - while (this.#promise) { - await this.#promise; - } - - this.#promise = new Promise((resolve) => { - this.#resolve = resolve; - }); - } - - unlock(): void { - const resolve = this.#resolve; - this.#promise = undefined; - this.#resolve = undefined; - resolve?.(); +class CompileOnlySqliteAdapter extends SqliteAdapter { + override get supportsMultipleConnections(): boolean { + // Kysely's SQLite adapter installs a single-connection mutex. This facade + // never opens a real connection, so direct execution should reject from + // acquisition without leaving controlled transaction calls wedged. + return true; } } diff --git a/src/infra/kysely-sync.test.ts b/src/infra/kysely-sync.test.ts index 6cc3bfe25212..be90733c832e 100644 --- a/src/infra/kysely-sync.test.ts +++ b/src/infra/kysely-sync.test.ts @@ -1,15 +1,18 @@ // Covers the compile-only Kysely facade used by sync node:sqlite helpers. import { DatabaseSync } from "node:sqlite"; +import type { Generated } from "kysely"; import { afterEach, describe, expect, it } from "vitest"; import { clearNodeSqliteKyselyCacheForDatabase, executeSqliteQuerySync, + executeSqliteQueryTakeFirstSync, getNodeSqliteKysely, + iterateSqliteQuerySync, } from "./kysely-sync.js"; type SyncHelperTestDatabase = { items: { - id: number; + id: Generated; name: string; }; }; @@ -26,15 +29,41 @@ describe("kysely sync helpers", () => { database = undefined; }); + it("executes compiled queries through the sync helpers", () => { + database = new DatabaseSync(":memory:"); + database.exec( + "create table items (id integer primary key autoincrement, name text not null unique)", + ); + const db = getNodeSqliteKysely(database); + + const insertResult = executeSqliteQuerySync( + database, + db.insertInto("items").values({ name: "Ada" }), + ); + expect(insertResult.insertId).toBe(1n); + expect(insertResult.numAffectedRows).toBe(1n); + + expect( + executeSqliteQuerySync( + database, + db.insertInto("items").values({ name: "Grace" }).returning(["id", "name"]), + ).rows, + ).toEqual([{ id: 2, name: "Grace" }]); + + const select = db.selectFrom("items").selectAll().orderBy("id"); + expect(executeSqliteQueryTakeFirstSync(database, select)).toEqual({ id: 1, name: "Ada" }); + expect([...iterateSqliteQuerySync(database, select)]).toEqual([ + { id: 1, name: "Ada" }, + { id: 2, name: "Grace" }, + ]); + }); + it("keeps the builder facade compile-only and fails direct execution", async () => { database = new DatabaseSync(":memory:"); database.exec("create table items (id integer primary key, name text not null)"); const db = getNodeSqliteKysely(database); - const insertQuery = db.insertInto("items").values({ id: 1, name: "Ada" }); - expect(insertQuery.compile().sql).toContain("insert into"); - - executeSqliteQuerySync(database, insertQuery); + executeSqliteQuerySync(database, db.insertInto("items").values({ id: 1, name: "Ada" })); expect(executeSqliteQuerySync(database, db.selectFrom("items").selectAll()).rows).toEqual([ { id: 1, name: "Ada" }, ]); diff --git a/src/infra/kysely-sync.ts b/src/infra/kysely-sync.ts index fe61465cdf04..e53b029c0212 100644 --- a/src/infra/kysely-sync.ts +++ b/src/infra/kysely-sync.ts @@ -1,14 +1,7 @@ // Adapts node:sqlite sync database calls for Kysely-style query execution. import type { DatabaseSync, SQLInputValue } from "node:sqlite"; -import type { - CompiledQuery, - DatabaseConnection, - Driver, - Kysely, - QueryResult, - TransactionSettings, -} from "kysely"; -import { InsertQueryNode, Kysely as KyselyInstance, SqliteAdapter } from "kysely"; +import type { CompiledQuery, Kysely, QueryResult } from "kysely"; +import { InsertQueryNode, Kysely as KyselyInstance } from "kysely"; import { NodeSqliteKyselyDialect } from "./kysely-node-sqlite.js"; // Sync query helpers execute compiled Kysely SQL against node:sqlite without @@ -25,7 +18,7 @@ export function getNodeSqliteKysely(db: DatabaseSync): Kysely; } const kysely = new KyselyInstance({ - dialect: new CompileOnlyNodeSqliteKyselyDialect(), + dialect: new NodeSqliteKyselyDialect(), }); kyselyByDatabase.set(db, kysely as Kysely); return kysely; @@ -91,63 +84,3 @@ export function executeSqliteQueryTakeFirstSync( export function clearNodeSqliteKyselyCacheForDatabase(db: DatabaseSync): void { kyselyByDatabase.delete(db); } - -class CompileOnlyNodeSqliteKyselyDialect extends NodeSqliteKyselyDialect { - constructor() { - super({ database: createUnavailableDatabase }); - } - - override createDriver(): Driver { - return new CompileOnlySqliteDriver(); - } - - override createAdapter(): SqliteAdapter { - return new CompileOnlySqliteAdapter(); - } -} - -class CompileOnlySqliteDriver implements Driver { - async init(): Promise {} - - async acquireConnection(): Promise { - throw createCompileOnlyExecutionError(); - } - - async beginTransaction( - _connection: DatabaseConnection, - _settings: TransactionSettings, - ): Promise { - throw createCompileOnlyExecutionError(); - } - - async commitTransaction(_connection: DatabaseConnection): Promise { - throw createCompileOnlyExecutionError(); - } - - async rollbackTransaction(_connection: DatabaseConnection): Promise { - throw createCompileOnlyExecutionError(); - } - - async releaseConnection(_connection: DatabaseConnection): Promise {} - - async destroy(): Promise {} -} - -function createCompileOnlyExecutionError(): Error { - return new Error( - "getNodeSqliteKysely() returns a compile-only Kysely facade; use executeSqliteQuerySync() to execute node:sqlite queries.", - ); -} - -function createUnavailableDatabase(): never { - throw createCompileOnlyExecutionError(); -} - -class CompileOnlySqliteAdapter extends SqliteAdapter { - override get supportsMultipleConnections(): boolean { - // Kysely's SQLite adapter installs a single-connection mutex. This facade - // never opens a real connection, so direct execution should reject from - // acquisition without leaving controlled transaction calls wedged. - return true; - } -} diff --git a/src/media-understanding/local-audio.ts b/src/media-understanding/local-audio.ts index 54b3888bc2bc..0699a3265f23 100644 --- a/src/media-understanding/local-audio.ts +++ b/src/media-understanding/local-audio.ts @@ -3,6 +3,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import type { MediaUnderstandingModelConfig } from "../config/types.tools.js"; import { runExec } from "../process/exec.js"; +import { getOrCreatePromise } from "../shared/lazy-promise.js"; import { fileExists } from "./fs.js"; export type LocalAudioCandidate = { @@ -154,11 +155,7 @@ async function findBinary( checkExecutable: (filePath: string, platform: NodeJS.Platform) => Promise = isExecutable, ): Promise { const key = `${platform}\0${env.PATH ?? ""}\0${env.PATHEXT ?? ""}\0${name}`; - const cached = binaryCache.get(key); - if (cached) { - return await cached; - } - const lookup = (async () => { + return await getOrCreatePromise(binaryCache, key, async () => { const direct = name.trim(); const candidates = binaryNames(direct, platform, env); if (direct.includes("/") || direct.includes("\\")) { @@ -186,9 +183,7 @@ async function findBinary( } } return null; - })(); - binaryCache.set(key, lookup); - return await lookup; + }); } async function inspectLinkedLibraries( @@ -196,11 +191,7 @@ async function inspectLinkedLibraries( platform: NodeJS.Platform, ): Promise { const key = `${platform}\0${filePath}`; - const cached = libraryCache.get(key); - if (cached) { - return await cached; - } - const inspection = (async () => { + return await getOrCreatePromise(libraryCache, key, async () => { const command = platform === "darwin" ? "otool" : platform === "linux" ? "readelf" : null; if (!command) { return null; @@ -212,9 +203,7 @@ async function inspectLinkedLibraries( } catch { return null; } - })(); - libraryCache.set(key, inspection); - return await inspection; + }); } async function inspectWhisperBackend(params: { diff --git a/src/media-understanding/runner.ts b/src/media-understanding/runner.ts index 8fe7581838fa..28453f83a303 100644 --- a/src/media-understanding/runner.ts +++ b/src/media-understanding/runner.ts @@ -41,6 +41,7 @@ import { logWarn } from "../logger.js"; import { resolveChannelInboundAttachmentRoots } from "../media/channel-inbound-roots.js"; import { getDefaultMediaLocalRoots } from "../media/local-roots.js"; import { runExec } from "../process/exec.js"; +import { getOrCreatePromise } from "../shared/lazy-promise.js"; import { createLazyRuntimeModule, createLazyRuntimeNamedExport } from "../shared/lazy-runtime.js"; import { MediaAttachmentCache, selectAttachments } from "./attachments.js"; import { @@ -390,11 +391,7 @@ async function isExecutable(filePath: string): Promise { } async function findBinary(name: string): Promise { - const cached = binaryCache.get(name); - if (cached) { - return cached; - } - const resolved = (async () => { + return await getOrCreatePromise(binaryCache, name, async () => { const direct = expandHomeDir(name.trim()); if (direct && hasPathSeparator(direct)) { for (const candidate of candidateBinaryNames(direct)) { @@ -424,9 +421,7 @@ async function findBinary(name: string): Promise { } return null; - })(); - binaryCache.set(name, resolved); - return resolved; + }); } async function probeAntigravityCliCandidate(command: string): Promise { @@ -455,11 +450,7 @@ async function probeAntigravityCliCandidate(command: string): Promise { - const cached = antigravityCliCache.get("agy"); - if (cached) { - return cached; - } - const resolved = (async () => { + return await getOrCreatePromise(antigravityCliCache, "agy", async () => { const configured = process.env.OPENCLAW_ANTIGRAVITY_CLI?.trim(); const candidates = [configured, "agy", "antigravity"].filter((value): value is string => Boolean(value), @@ -471,9 +462,7 @@ async function resolveAntigravityCliBinary(): Promise { } } return null; - })(); - antigravityCliCache.set("agy", resolved); - return resolved; + }); } async function resolveAntigravityCliEntry( diff --git a/src/shared/lazy-promise.test.ts b/src/shared/lazy-promise.test.ts index a273666b53bb..124b6a251c5f 100644 --- a/src/shared/lazy-promise.test.ts +++ b/src/shared/lazy-promise.test.ts @@ -4,8 +4,30 @@ import { createLazyImportLoader, createLazyPromise, createLazyPromiseLoader, + getOrCreatePromise, } from "./lazy-promise.js"; +describe("getOrCreatePromise", () => { + it("dedupes each key and leaves cache lifecycle to the caller", async () => { + const cache = new Map>(); + const create = vi.fn(async (key: string) => `loaded-${key}`); + + const first = getOrCreatePromise(cache, "a", async () => await create("a")); + expect(getOrCreatePromise(cache, "a", async () => await create("a"))).toBe(first); + await expect(first).resolves.toBe("loaded-a"); + await expect(getOrCreatePromise(cache, "b", async () => await create("b"))).resolves.toBe( + "loaded-b", + ); + expect(create).toHaveBeenCalledTimes(2); + + cache.clear(); + await expect(getOrCreatePromise(cache, "a", async () => await create("a"))).resolves.toBe( + "loaded-a", + ); + expect(create).toHaveBeenCalledTimes(3); + }); +}); + describe("createLazyPromise", () => { it("returns a reusable single-flight loader", async () => { let calls = 0; diff --git a/src/shared/lazy-promise.ts b/src/shared/lazy-promise.ts index b4b7295b56d4..cedc7441bb59 100644 --- a/src/shared/lazy-promise.ts +++ b/src/shared/lazy-promise.ts @@ -8,6 +8,21 @@ export type LazyPromiseLoader = { clear: () => void; }; +/** Returns the cached promise for a key, creating and storing it when absent. */ +export function getOrCreatePromise( + cache: Map>, + key: K, + create: () => Promise, +): Promise { + const cached = cache.get(key); + if (cached) { + return cached; + } + const created = create(); + cache.set(key, created); + return created; +} + /** Options for controlling lazy promise cache behavior. */ type LazyPromiseLoaderOptions = { /** Keep rejected promises cached instead of allowing the next caller to retry. */