diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index e035959be..993141199 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `SqliteAuthCredentialStore.close()` leaking one-off prepared statements created by inline `this.#db.prepare()` calls in `#authCredentialsTableExists`, `#readAuthSchemaVersion`, `#inferAuthSchemaVersion`, `#migrateAuthSchemaV0ToV1`, `#backfillCredentialIdentityKeys`, and `updateAuthCredential`. Each statement is now wrapped in `try/finally` with `stmt.finalize()`, and the `close()` method finalizes `#insertUsageCostStmt` and `#listUsageCostsStmt` which were previously missed. This caused EBUSY on Windows when tests tried to delete temp dirs containing open SQLite handles. + ## [16.0.10] - 2026-06-18 ### Added diff --git a/packages/ai/src/auth-storage.ts b/packages/ai/src/auth-storage.ts index 87e2ced77..84107619a 100644 --- a/packages/ai/src/auth-storage.ts +++ b/packages/ai/src/auth-storage.ts @@ -4742,25 +4742,47 @@ export class SqliteAuthCredentialStore implements AuthCredentialStore { } #authCredentialsTableExists(): boolean { - const row = this.#db - .prepare("SELECT 1 AS present FROM sqlite_master WHERE type = 'table' AND name = 'auth_credentials'") - .get() as { present?: number } | undefined; - return row?.present === 1; + const stmt = this.#db.prepare( + "SELECT 1 AS present FROM sqlite_master WHERE type = 'table' AND name = 'auth_credentials'", + ); + try { + const row = stmt.get() as { present?: number } | undefined; + return row?.present === 1; + } finally { + stmt.finalize(); + } } #readAuthSchemaVersion(): number | null { - const row = this.#db.prepare("SELECT version FROM auth_schema_version WHERE id = 1").get() as - | { version?: number } - | undefined; - return typeof row?.version === "number" ? row.version : null; + const stmt = this.#db.prepare("SELECT version FROM auth_schema_version WHERE id = 1"); + try { + const row = stmt.get() as { version?: number } | undefined; + return typeof row?.version === "number" ? row.version : null; + } finally { + stmt.finalize(); + } } #writeAuthSchemaVersion(version: number): void { - this.#db.prepare("INSERT OR REPLACE INTO auth_schema_version(id, version) VALUES (1, ?)").run(version); + const stmt = this.#db.prepare("INSERT OR REPLACE INTO auth_schema_version(id, version) VALUES (1, ?)"); + try { + stmt.run(version); + } finally { + stmt.finalize(); + } } #inferAuthSchemaVersion(): number { - const cols = this.#db.prepare("PRAGMA table_info(auth_credentials)").all() as Array<{ name?: string }>; + const stmt = this.#db.prepare("PRAGMA table_info(auth_credentials)"); + try { + const cols = stmt.all() as Array<{ name?: string }>; + return this.#inferAuthSchemaVersionFromColumns(cols); + } finally { + stmt.finalize(); + } + } + + #inferAuthSchemaVersionFromColumns(cols: Array<{ name?: string }>): number { const hasDisabledCause = cols.some(column => column.name === "disabled_cause"); const hasIdentityKey = cols.some(column => column.name === "identity_key"); const hasAccountId = cols.some(column => column.name === "account_id"); @@ -4808,8 +4830,14 @@ export class SqliteAuthCredentialStore implements AuthCredentialStore { #migrateAuthSchemaV0ToV1(): void { const migrate = this.#db.transaction(() => { - const v0Cols = this.#db.prepare("PRAGMA table_info(auth_credentials)").all() as Array<{ name?: string }>; - const hasDisabled = v0Cols.some(col => col.name === "disabled"); + const stmt = this.#db.prepare("PRAGMA table_info(auth_credentials)"); + let hasDisabled = false; + try { + const v0Cols = stmt.all() as Array<{ name?: string }>; + hasDisabled = v0Cols.some(col => col.name === "disabled"); + } finally { + stmt.finalize(); + } this.#db.run("ALTER TABLE auth_credentials RENAME TO auth_credentials_v0"); this.#db.run(` @@ -4885,21 +4913,29 @@ export class SqliteAuthCredentialStore implements AuthCredentialStore { } #backfillCredentialIdentityKeys(): void { - const rows = this.#db - .prepare( - "SELECT id, provider, credential_type, data, disabled_cause, identity_key FROM auth_credentials WHERE identity_key IS NULL ORDER BY id ASC", - ) - .all() as AuthRow[]; + const selectRowsStmt = this.#db.prepare( + "SELECT id, provider, credential_type, data, disabled_cause, identity_key FROM auth_credentials WHERE identity_key IS NULL ORDER BY id ASC", + ); + let rows: AuthRow[]; + try { + rows = selectRowsStmt.all() as AuthRow[]; + } finally { + selectRowsStmt.finalize(); + } if (rows.length === 0) return; let updateIdentity: Statement | null = null; - for (const row of rows) { - const identityKey = resolveRowCredentialIdentityKey(row.provider, row); - // Rows whose identity cannot be derived stay NULL; writing NULL over - // NULL would just burn a write transaction on every boot. - if (identityKey === null) continue; - updateIdentity ??= this.#db.prepare("UPDATE auth_credentials SET identity_key = ? WHERE id = ?"); - updateIdentity.run(identityKey, row.id); + try { + for (const row of rows) { + const identityKey = resolveRowCredentialIdentityKey(row.provider, row); + // Rows whose identity cannot be derived stay NULL; writing NULL over + // NULL would just burn a write transaction on every boot. + if (identityKey === null) continue; + updateIdentity ??= this.#db.prepare("UPDATE auth_credentials SET identity_key = ? WHERE id = ?"); + updateIdentity.run(identityKey, row.id); + } + } finally { + updateIdentity?.finalize(); } } @@ -5063,9 +5099,13 @@ export class SqliteAuthCredentialStore implements AuthCredentialStore { updateAuthCredential(id: number, credential: AuthCredential): void { try { - const providerRow = this.#db.prepare("SELECT provider FROM auth_credentials WHERE id = ?").get(id) as - | { provider?: string } - | undefined; + const providerStmt = this.#db.prepare("SELECT provider FROM auth_credentials WHERE id = ?"); + let providerRow: { provider?: string } | undefined; + try { + providerRow = providerStmt.get(id) as { provider?: string } | undefined; + } finally { + providerStmt.finalize(); + } const provider = providerRow?.provider ?? ""; const serialized = serializeCredential(provider, credential); if (!serialized) return; @@ -5334,6 +5374,8 @@ export class SqliteAuthCredentialStore implements AuthCredentialStore { this.#lastUsageHistoryStmt.finalize(); this.#listUsageHistoryStmt.finalize(); this.#updateUsageHistoryStmt.finalize(); + this.#insertUsageCostStmt.finalize(); + this.#listUsageCostsStmt.finalize(); this.#db.close(); } } diff --git a/packages/catalog/CHANGELOG.md b/packages/catalog/CHANGELOG.md index d7c4edbad..15447b5fe 100644 --- a/packages/catalog/CHANGELOG.md +++ b/packages/catalog/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `readModelCache`/`writeModelCache` using a process-global shared database even when a custom `dbPath` was provided. Custom-path cache operations now open and close a per-call database via `withModelCacheDb`, preventing leaked SQLite handles on Windows + ## [16.0.9] - 2026-06-18 ### Fixed diff --git a/packages/catalog/src/model-cache.ts b/packages/catalog/src/model-cache.ts index c54f8e804..6e5fe9ab4 100644 --- a/packages/catalog/src/model-cache.ts +++ b/packages/catalog/src/model-cache.ts @@ -46,14 +46,7 @@ interface CacheEntry { let sharedDb: Database | null = null; let sharedDbPath: string | null = null; -function getDb(dbPath?: string): Database { - const resolvedPath = dbPath ?? getModelDbPath(); - if (sharedDb && sharedDbPath === resolvedPath) { - return sharedDb; - } - if (sharedDb) { - sharedDb.close(); - } +function openDb(resolvedPath: string): Database { const db = new Database(resolvedPath, { create: true }); // Install the busy handler BEFORE any lock-taking statement. See // https://github.com/can1357/oh-my-pi/issues/2421. @@ -70,16 +63,42 @@ function getDb(dbPath?: string): Database { ) `); migrateCacheSchema(db); + return db; +} +function getSharedDb(): Database { + const resolvedPath = getModelDbPath(); + if (sharedDb && sharedDbPath === resolvedPath) { + return sharedDb; + } + if (sharedDb) { + sharedDb.close(); + } + const db = openDb(resolvedPath); sharedDb = db; sharedDbPath = resolvedPath; return db; } +function withModelCacheDb(dbPath: string | undefined, useDb: (db: Database) => T): T { + if (!dbPath) return useDb(getSharedDb()); + const db = openDb(dbPath); + try { + return useDb(db); + } finally { + db.close(); + } +} + function migrateCacheSchema(db: Database): void { - const columns = db.prepare("PRAGMA table_info(model_cache)").all() as TableInfoRow[]; - if (!columns.some(column => column.name === "static_fingerprint")) { - db.run("ALTER TABLE model_cache ADD COLUMN static_fingerprint TEXT NOT NULL DEFAULT ''"); + const stmt = db.prepare("PRAGMA table_info(model_cache)"); + try { + const columns = stmt.all() as TableInfoRow[]; + if (!columns.some(column => column.name === "static_fingerprint")) { + db.run("ALTER TABLE model_cache ADD COLUMN static_fingerprint TEXT NOT NULL DEFAULT ''"); + } + } finally { + stmt.finalize(); } db.run("UPDATE model_cache SET version = ? WHERE version = 2", [CACHE_SCHEMA_VERSION]); } @@ -91,21 +110,27 @@ export function readModelCache( dbPath?: string, ): CacheEntry | null { try { - const db = getDb(dbPath); - const row = db.query("SELECT * FROM model_cache WHERE provider_id = ?").get(providerId); - if (!row || row.version !== CACHE_SCHEMA_VERSION) { - return null; - } - const models = JSON.parse(row.models) as ModelSpec[]; - const ageMs = now() - row.updated_at; - const fresh = Number.isFinite(ageMs) && ageMs >= 0 && ageMs <= ttlMs; - return { - models, - fresh, - authoritative: row.authoritative === 1, - updatedAt: row.updated_at, - staticFingerprint: row.static_fingerprint ?? "", - }; + return withModelCacheDb(dbPath, db => { + const stmt = db.query("SELECT * FROM model_cache WHERE provider_id = ?"); + try { + const row = stmt.get(providerId); + if (!row || row.version !== CACHE_SCHEMA_VERSION) { + return null; + } + const models = JSON.parse(row.models) as ModelSpec[]; + const ageMs = now() - row.updated_at; + const fresh = Number.isFinite(ageMs) && ageMs >= 0 && ageMs <= ttlMs; + return { + models, + fresh, + authoritative: row.authoritative === 1, + updatedAt: row.updated_at, + staticFingerprint: row.static_fingerprint ?? "", + }; + } finally { + stmt.finalize(); + } + }); } catch { return null; } @@ -120,19 +145,20 @@ export function writeModelCache( dbPath?: string, ): void { try { - const db = getDb(dbPath); - db.run( - `INSERT OR REPLACE INTO model_cache (provider_id, version, updated_at, authoritative, static_fingerprint, models) - VALUES (?, ?, ?, ?, ?, ?)`, - [ - providerId, - CACHE_SCHEMA_VERSION, - updatedAt, - authoritative ? 1 : 0, - staticFingerprint, - JSON.stringify(models.map(model => ({ ...model, compat: model.compatConfig, compatConfig: undefined }))), - ], - ); + withModelCacheDb(dbPath, db => { + db.run( + `INSERT OR REPLACE INTO model_cache (provider_id, version, updated_at, authoritative, static_fingerprint, models) + VALUES (?, ?, ?, ?, ?, ?)`, + [ + providerId, + CACHE_SCHEMA_VERSION, + updatedAt, + authoritative ? 1 : 0, + staticFingerprint, + JSON.stringify(models.map(model => ({ ...model, compat: model.compatConfig, compatConfig: undefined }))), + ], + ); + }); } catch { // Cache writes are best-effort; failures should not break model resolution. } diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ae4c6891c..3fa6173ba 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,16 @@ ## [Unreleased] +### Fixed + +- Fixed Windows test failures caused by path handling: tests now use `pathToFileURL`, `path.resolve`, and `path.join` instead of hard-coded POSIX paths; `shortenPath()` normalizes backslashes to forward slashes after `~` and respects home directory boundaries; shell-escaped interpolated paths in bash tool tests to prevent Git Bash eating backslashes +- Fixed `HistoryStorage.resetInstance()` leaking its SQLite database handle on Windows by adding a `#close()` method that finalizes all prepared statements and closes the database; `AgentStorage` gained the same `resetInstance()`/`#close()` pattern +- Fixed `createAgentSession` leaking the internally-created `AuthStorage` when session construction fails before the session takes ownership, causing EBUSY on Windows temp dir cleanup +- Fixed `MnemopiBackend.removeDbFiles()` throwing on Windows when the database handle is still being released; it is now truly best-effort (logs failures instead of silently swallowing) +- Fixed Windows EBUSY test failures by replacing raw `fs.rmSync`/`fs.rm` cleanup with `TempDir` (which retries) and best-effort `.catch(() => {})` where SQLite handles outlive the test +- Fixed `TempDir` prefix convention: non-`@` prefixes created temp dirs relative to cwd instead of `os.tmpdir()`, causing module resolution failures on Windows +- Fixed git line-ending mismatches in autoresearch tests by setting `core.autocrlf false` in test repo initialization + ## [16.0.11] - 2026-06-19 ### Added @@ -36,6 +46,13 @@ ### Removed - Removed `display.tabWidth` setting and configurable tab width support +- Fixed Windows test failures caused by path handling: tests now use `pathToFileURL`, `path.resolve`, and `path.join` instead of hard-coded POSIX paths; `shortenPath()` normalizes backslashes to forward slashes after `~` and respects home directory boundaries; shell-escaped interpolated paths in bash tool tests to prevent Git Bash eating backslashes +- Fixed `HistoryStorage.resetInstance()` leaking its SQLite database handle on Windows by adding a `#close()` method that finalizes all prepared statements and closes the database; `AgentStorage` gained the same `resetInstance()`/`#close()` pattern +- Fixed `createAgentSession` leaking the internally-created `AuthStorage` when session construction fails before the session takes ownership, causing EBUSY on Windows temp dir cleanup +- Fixed `MnemopiBackend.removeDbFiles()` throwing on Windows when the database handle is still being released; it is now truly best-effort +- Fixed Windows EBUSY test failures by replacing raw `fs.rmSync`/`fs.rm` cleanup with `TempDir` (which retries) and best-effort `.catch(() => {})` where SQLite handles outlive the test +- Fixed `TempDir` prefix convention: non-`@` prefixes created temp dirs relative to cwd instead of `os.tmpdir()`, causing module resolution failures on Windows +- Fixed git line-ending mismatches in autoresearch tests by setting `core.autocrlf false` in test repo initialization ## [16.0.10] - 2026-06-18 diff --git a/packages/coding-agent/src/mnemopi/backend.ts b/packages/coding-agent/src/mnemopi/backend.ts index 291ec9233..00694f2ff 100644 --- a/packages/coding-agent/src/mnemopi/backend.ts +++ b/packages/coding-agent/src/mnemopi/backend.ts @@ -120,6 +120,10 @@ export const mnemopiBackend: MemoryBackend = { const config = previous?.config ?? (session ? loadMnemopiConfig(session.settings, agentDir) : undefined); if (!config) return; await loadMnemopiCore(); + // Close the cached default Mnemopi instance so its SQLite handle doesn't + // keep the DB files locked on Windows when removeDbFiles tries to delete. + requireMnemopi().resetMemoryForTests(); + await Bun.sleep(0); await removeDbFiles(getMnemopiScopedDbPaths(config)); }, @@ -557,10 +561,48 @@ export function getMnemopiDbDirForTests(session: AgentSession): string | undefin return state ? path.dirname(state.config.dbPath) : undefined; } +/** + * Best-effort removal of a SQLite DB file and its WAL/SHM sidecars. + * + * Windows keeps `-wal`/`-shm` busy briefly after the DB handle closes, so a + * single `rm` races with EBUSY/EPERM. Retry a handful of times before giving + * up; `force: true` already makes "missing" a non-error. + */ async function removeDbFiles(dbPaths: readonly string[]): Promise { for (const dbPath of dbPaths) { - await rm(dbPath, { force: true }); - await rm(`${dbPath}-wal`, { force: true }); - await rm(`${dbPath}-shm`, { force: true }); + for (const suffix of ["", "-wal", "-shm"]) { + await removeWithRetries(`${dbPath}${suffix}`).catch(error => { + // `force: true` already makes ENOENT a non-error; anything else + // after the full retry window means the DB is genuinely locked and + // the user's "Memory cleared" message would be misleading. Log so + // the failure is diagnosable without blocking the clear flow. + const code = typeof error === "object" && error !== null && "code" in error ? error.code : undefined; + if (code !== "ENOENT") { + logger.warn("Mnemopi: failed to remove DB file after retries", { path: `${dbPath}${suffix}`, code }); + } + }); + } + } +} + +const kRemoveRetries = 40; +const kRemoveRetryDelayMs = 25; +const kRetryableRemoveErrorCodes = new Set(["EBUSY", "EPERM", "ENOTEMPTY"]); + +async function removeWithRetries(target: string): Promise { + for (let attempt = 0; ; attempt++) { + try { + await rm(target, { force: true }); + return; + } catch (err) { + const retryable = + typeof err === "object" && + err !== null && + "code" in err && + typeof err.code === "string" && + kRetryableRemoveErrorCodes.has(err.code); + if (!retryable || attempt >= kRemoveRetries) throw err; + await Bun.sleep(kRemoveRetryDelayMs); + } } } diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 971800519..773ba8208 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -1124,6 +1124,9 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} const modelRegistry = options.modelRegistry ?? new ModelRegistry(options.authStorage ?? (await logger.time("discoverModels", discoverAuthStorage, agentDir))); + // Track whether we internally created the authStorage so we can close it + // if construction fails before the session takes ownership. + const ownsAuthStorage = !options.authStorage && !options.modelRegistry; const authStorage = modelRegistry.authStorage; if (options.authStorage && options.authStorage !== authStorage) { throw new Error( @@ -2905,6 +2908,7 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} await asyncJobManager.dispose({ timeoutMs: 3_000 }); } await disposeKernelSessionsByOwner(evalKernelOwnerId); + if (ownsAuthStorage) authStorage.close(); } } catch (cleanupError) { logger.warn("Failed to clean up createAgentSession resources after startup error", { diff --git a/packages/coding-agent/src/session/agent-storage.ts b/packages/coding-agent/src/session/agent-storage.ts index 39c51c46e..abb9c7eac 100644 --- a/packages/coding-agent/src/session/agent-storage.ts +++ b/packages/coding-agent/src/session/agent-storage.ts @@ -247,6 +247,20 @@ FROM model_usage_legacy { cause: lastError }, ); } + /** @internal Reset all singletons and close their databases — test-only. */ + static resetInstance(): void { + for (const storage of instances.values()) storage.#close(); + instances.clear(); + } + + #close(): void { + this.#listSettingsStmt.finalize(); + this.#upsertModelUsageStmt.finalize(); + this.#listModelUsageStmt.finalize(); + // SqliteAuthCredentialStore.close() finalizes its own statements and + // closes the shared #db handle — must run after our statements finalize. + this.#authStore.close(); + } /** * Reads legacy settings persisted in the agent.db `settings` table. diff --git a/packages/coding-agent/src/session/history-storage.ts b/packages/coding-agent/src/session/history-storage.ts index fe86d3c20..584431d43 100644 --- a/packages/coding-agent/src/session/history-storage.ts +++ b/packages/coding-agent/src/session/history-storage.ts @@ -145,9 +145,21 @@ CREATE TRIGGER IF NOT EXISTS history_ai AFTER INSERT ON history BEGIN return HistoryStorage.#instance; } - /** @internal Reset the singleton — test-only. */ + /** @internal Reset the singleton and close its database — test-only. */ static resetInstance(): void { + const instance = HistoryStorage.#instance; HistoryStorage.#instance = undefined; + if (instance) instance.#close(); + } + + #close(): void { + for (const stmt of this.#substringStmts.values()) stmt.finalize(); + this.#substringStmts.clear(); + this.#insertRowStmt.finalize(); + this.#recentStmt.finalize(); + this.#searchStmt.finalize(); + this.#lastPromptStmt.finalize(); + this.#db.close(); } #insertBatch(rows: Array>): void { diff --git a/packages/coding-agent/src/tools/render-utils.ts b/packages/coding-agent/src/tools/render-utils.ts index 4c27b9b4e..b47484669 100644 --- a/packages/coding-agent/src/tools/render-utils.ts +++ b/packages/coding-agent/src/tools/render-utils.ts @@ -657,7 +657,10 @@ export function truncateDiffByHunk( export function shortenPath(filePath: string, homeDir?: string): string { const home = homeDir ?? os.homedir(); if (home && filePath.startsWith(home)) { - return `~${filePath.slice(home.length)}`; + const suffix = filePath.slice(home.length); + if (suffix === "" || suffix.startsWith(path.posix.sep) || suffix.startsWith(path.win32.sep)) { + return `~${suffix.replaceAll(path.win32.sep, path.posix.sep)}`; + } } return filePath; } diff --git a/packages/coding-agent/test/acp-agent.test.ts b/packages/coding-agent/test/acp-agent.test.ts index 1f131d878..ccdd1875f 100644 --- a/packages/coding-agent/test/acp-agent.test.ts +++ b/packages/coding-agent/test/acp-agent.test.ts @@ -20,6 +20,7 @@ import { import type { Model } from "@oh-my-pi/pi-ai"; import { buildModel } from "@oh-my-pi/pi-catalog/build"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { resolveLocalUrlToPath } from "@oh-my-pi/pi-coding-agent/internal-urls"; import { ACP_BOOTSTRAP_RACE_GUARD_MS, AcpAgent, @@ -652,11 +653,14 @@ describe("ACP agent", () => { const session = harness.findSession(created.sessionId)!; await harness.agent.setSessionMode({ sessionId: created.sessionId, modeId: "plan" }); - const artifactsDir = session.sessionManager.getArtifactsDir(); - expect(artifactsDir).not.toBeNull(); - // The agent writes to its chosen `local://-plan.md` and resolves with - // the matching slug — the file is never renamed. - const planPath = path.join(artifactsDir!, "local", "words-counter-plan.md"); + const localOptions = { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }; + cleanupRoots.push(resolveLocalUrlToPath("local://", localOptions)); + // On Windows, long artifact roots are shortened by the local:// resolver to + // avoid MAX_PATH. Write through the same resolver the ACP handler reads from. + const planPath = resolveLocalUrlToPath("local://words-counter-plan.md", localOptions); await Bun.write(planPath, "# Words Counter\n\nFile contents."); const updatesBefore = harness.updates.length; @@ -722,8 +726,12 @@ describe("ACP agent", () => { const session = harness.findSession(created.sessionId)!; await harness.agent.setSessionMode({ sessionId: created.sessionId, modeId: "plan" }); - const artifactsDir = session.sessionManager.getArtifactsDir(); - const planPath = path.join(artifactsDir!, "local", "PLAN.md"); + const localOptions = { + getArtifactsDir: () => session.sessionManager.getArtifactsDir(), + getSessionId: () => session.sessionManager.getSessionId(), + }; + cleanupRoots.push(resolveLocalUrlToPath("local://", localOptions)); + const planPath = resolveLocalUrlToPath("local://PLAN.md", localOptions); await Bun.write(planPath, "# Words Counter\n\nFile contents."); const updatesBefore = harness.updates.length; @@ -737,7 +745,7 @@ describe("ACP agent", () => { expect(result.content[0]?.text).toMatch(/refinement requested/i); // Plan file stays put; no rename, no write-access grant. expect(await Bun.file(planPath).exists()).toBe(true); - expect(await Bun.file(path.join(artifactsDir!, "local", "words-counter.md")).exists()).toBe(false); + expect(await Bun.file(resolveLocalUrlToPath("local://words-counter.md", localOptions)).exists()).toBe(false); // Plan mode + standing handler stay active so the agent can iterate. expect(session.planModeState?.enabled).toBe(true); expect(typeof session.standingResolveHandler).toBe("function"); diff --git a/packages/coding-agent/test/acp-mcp-isolation.test.ts b/packages/coding-agent/test/acp-mcp-isolation.test.ts index abef4c4f2..1b1d68b20 100644 --- a/packages/coding-agent/test/acp-mcp-isolation.test.ts +++ b/packages/coding-agent/test/acp-mcp-isolation.test.ts @@ -13,23 +13,20 @@ */ import { describe, expect, it } from "bun:test"; -import * as fs from "node:fs"; -import * as os from "node:os"; -import * as path from "node:path"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { createAcpSessionFactory } from "@oh-my-pi/pi-coding-agent/main"; import type { CreateAgentSessionOptions, CreateAgentSessionResult } from "@oh-my-pi/pi-coding-agent/sdk"; import type { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; -import { Snowflake } from "@oh-my-pi/pi-utils"; +import { TempDir } from "@oh-my-pi/pi-utils"; describe("createAcpSessionFactory MCP isolation (issue #1234)", () => { it("forces enableMCP=false even when baseOptions opts in", async () => { - const tempDir = path.join(os.tmpdir(), `pi-acp-mcp-isolation-${Snowflake.next()}`); - fs.mkdirSync(tempDir, { recursive: true }); - const authStorage = await AuthStorage.create(path.join(tempDir, "auth.db")); + const tempDir = TempDir.createSync("@pi-acp-mcp-isolation-"); + let authStorage: AuthStorage | undefined; try { + authStorage = await AuthStorage.create(tempDir.join("auth.db")); const modelRegistry = new ModelRegistry(authStorage); const settings = Settings.isolated({}); const fakeSession = {} as AgentSession; @@ -56,7 +53,7 @@ describe("createAcpSessionFactory MCP isolation (issue #1234)", () => { const factory = createAcpSessionFactory({ baseOptions: { enableMCP: true } as CreateAgentSessionOptions, settings, - sessionDir: path.join(tempDir, "sessions"), + sessionDir: tempDir.join("sessions"), authStorage, modelRegistry, parsedArgs: {}, @@ -64,13 +61,17 @@ describe("createAcpSessionFactory MCP isolation (issue #1234)", () => { createSession, }); - const result = await factory(tempDir); + const result = await factory(tempDir.path()); expect(result).toBe(fakeSession); expect(captured).toHaveLength(1); expect(captured[0].enableMCP).toBe(false); } finally { - authStorage.close(); - fs.rmSync(tempDir, { recursive: true, force: true }); + try { + authStorage?.close(); + } finally { + await Bun.sleep(0); + await tempDir.remove(); + } } }); }); diff --git a/packages/coding-agent/test/advisor-watchdog.test.ts b/packages/coding-agent/test/advisor-watchdog.test.ts index 6977023f1..5e1e0291b 100644 --- a/packages/coding-agent/test/advisor-watchdog.test.ts +++ b/packages/coding-agent/test/advisor-watchdog.test.ts @@ -1,64 +1,66 @@ import { afterEach, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; -import * as os from "node:os"; import * as path from "node:path"; import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { createAgentSession } from "@oh-my-pi/pi-coding-agent/sdk"; +import type { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; -import { Snowflake } from "@oh-my-pi/pi-utils"; +import { TempDir } from "@oh-my-pi/pi-utils"; describe("advisor watchdog prompt discovery", () => { - const tempDirs: string[] = []; + const tempDirs: TempDir[] = []; - afterEach(() => { + afterEach(async () => { + await Bun.sleep(0); for (const tempDir of tempDirs.splice(0)) { - fs.rmSync(tempDir, { recursive: true, force: true }); + await tempDir.remove(); } }); it("discovers and appends WATCHDOG.md to the advisor prompt", async () => { - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-advisor-watchdog-${Snowflake.next()}-`)); + const tempDir = TempDir.createSync("@pi-advisor-watchdog-"); tempDirs.push(tempDir); - const cwd = path.join(tempDir, "project-root"); + const cwd = tempDir.join("project-root"); fs.mkdirSync(cwd, { recursive: true }); // Write a WATCHDOG.md file const watchdogContent = "Watchdog rule: Watch out for cheating on edits."; fs.writeFileSync(path.join(cwd, "WATCHDOG.md"), watchdogContent, "utf8"); - const authStorage = await AuthStorage.create(path.join(tempDir, "testauth.db")); - authStorage.setRuntimeApiKey("openai", "test-key"); - const modelRegistry = new ModelRegistry(authStorage); - - const sessionManager = SessionManager.create(cwd, path.join(tempDir, "sessions")); - const { session } = await createAgentSession({ - cwd, - agentDir: tempDir, - sessionManager, - authStorage, - modelRegistry, - settings: (() => { - const s = Settings.isolated({ - "async.enabled": false, - "advisor.enabled": true, - }); - s.setModelRole("advisor", "openai/gpt-4o-mini"); - return s; - })(), - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, - }); - + const authStorage = await AuthStorage.create(tempDir.join("testauth.db")); + let session: AgentSession | undefined; try { + authStorage.setRuntimeApiKey("openai", "test-key"); + const modelRegistry = new ModelRegistry(authStorage); + const sessionManager = SessionManager.create(cwd, tempDir.join("sessions")); + const result = await createAgentSession({ + cwd, + agentDir: tempDir.path(), + sessionManager, + authStorage, + modelRegistry, + settings: (() => { + const s = Settings.isolated({ + "async.enabled": false, + "advisor.enabled": true, + }); + s.setModelRole("advisor", "openai/gpt-4o-mini"); + return s; + })(), + model: getBundledModel("openai", "gpt-4o-mini"), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + }); + session = result.session; + expect(session.isAdvisorActive()).toBe(true); const dump = session.formatAdvisorHistoryAsText(); expect(dump).not.toBeNull(); @@ -67,14 +69,18 @@ describe("advisor watchdog prompt discovery", () => { expect(dump).toContain(watchdogContent); expect(dump).toContain(""); } finally { - await session.dispose(); + try { + await session?.dispose(); + } finally { + authStorage.close(); + } } }); it("resolves nested folders and sorts by depth", async () => { - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-advisor-watchdog-${Snowflake.next()}-`)); + const tempDir = TempDir.createSync("@pi-advisor-watchdog-"); tempDirs.push(tempDir); - const parentCwd = path.join(tempDir, "project-root"); + const parentCwd = tempDir.join("project-root"); const childCwd = path.join(parentCwd, "subfolder"); fs.mkdirSync(childCwd, { recursive: true }); @@ -84,36 +90,37 @@ describe("advisor watchdog prompt discovery", () => { fs.writeFileSync(path.join(parentCwd, "WATCHDOG.md"), parentWatchdogContent, "utf8"); fs.writeFileSync(path.join(childCwd, "WATCHDOG.md"), childWatchdogContent, "utf8"); - const authStorage = await AuthStorage.create(path.join(tempDir, "testauth.db")); - authStorage.setRuntimeApiKey("openai", "test-key"); - const modelRegistry = new ModelRegistry(authStorage); - - const sessionManager = SessionManager.create(childCwd, path.join(tempDir, "sessions")); - const { session } = await createAgentSession({ - cwd: childCwd, - agentDir: tempDir, - sessionManager, - authStorage, - modelRegistry, - settings: (() => { - const s = Settings.isolated({ - "async.enabled": false, - "advisor.enabled": true, - }); - s.setModelRole("advisor", "openai/gpt-4o-mini"); - return s; - })(), - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, - }); - + const authStorage = await AuthStorage.create(tempDir.join("testauth.db")); + let session: AgentSession | undefined; try { + authStorage.setRuntimeApiKey("openai", "test-key"); + const modelRegistry = new ModelRegistry(authStorage); + const sessionManager = SessionManager.create(childCwd, tempDir.join("sessions")); + const result = await createAgentSession({ + cwd: childCwd, + agentDir: tempDir.path(), + sessionManager, + authStorage, + modelRegistry, + settings: (() => { + const s = Settings.isolated({ + "async.enabled": false, + "advisor.enabled": true, + }); + s.setModelRole("advisor", "openai/gpt-4o-mini"); + return s; + })(), + model: getBundledModel("openai", "gpt-4o-mini"), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + }); + session = result.session; + expect(session.isAdvisorActive()).toBe(true); const dump = session.formatAdvisorHistoryAsText(); expect(dump).not.toBeNull(); @@ -130,16 +137,20 @@ describe("advisor watchdog prompt discovery", () => { expect(childIndex).toBeGreaterThan(-1); expect(parentIndex).toBeLessThan(childIndex); } finally { - await session.dispose(); + try { + await session?.dispose(); + } finally { + authStorage.close(); + } } }); it("discovers user-level and native project-level watchdog files", async () => { - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-advisor-watchdog-${Snowflake.next()}-`)); + const tempDir = TempDir.createSync("@pi-advisor-watchdog-"); tempDirs.push(tempDir); - const cwd = path.join(tempDir, "project-root"); + const cwd = tempDir.join("project-root"); const ompDir = path.join(cwd, ".omp"); - const userAgentDir = path.join(tempDir, "user-agent"); + const userAgentDir = tempDir.join("user-agent"); fs.mkdirSync(cwd, { recursive: true }); fs.mkdirSync(ompDir, { recursive: true }); fs.mkdirSync(userAgentDir, { recursive: true }); @@ -152,36 +163,37 @@ describe("advisor watchdog prompt discovery", () => { fs.writeFileSync(path.join(ompDir, "WATCHDOG.md"), nativeWatchdogContent, "utf8"); fs.writeFileSync(path.join(cwd, "WATCHDOG.md"), standaloneWatchdogContent, "utf8"); - const authStorage = await AuthStorage.create(path.join(tempDir, "testauth.db")); - authStorage.setRuntimeApiKey("openai", "test-key"); - const modelRegistry = new ModelRegistry(authStorage); - - const sessionManager = SessionManager.create(cwd, path.join(tempDir, "sessions")); - const { session } = await createAgentSession({ - cwd, - agentDir: userAgentDir, - sessionManager, - authStorage, - modelRegistry, - settings: (() => { - const s = Settings.isolated({ - "async.enabled": false, - "advisor.enabled": true, - }); - s.setModelRole("advisor", "openai/gpt-4o-mini"); - return s; - })(), - model: getBundledModel("openai", "gpt-4o-mini"), - disableExtensionDiscovery: true, - skills: [], - contextFiles: [], - promptTemplates: [], - slashCommands: [], - enableMCP: false, - enableLsp: false, - }); - + const authStorage = await AuthStorage.create(tempDir.join("testauth.db")); + let session: AgentSession | undefined; try { + authStorage.setRuntimeApiKey("openai", "test-key"); + const modelRegistry = new ModelRegistry(authStorage); + const sessionManager = SessionManager.create(cwd, tempDir.join("sessions")); + const result = await createAgentSession({ + cwd, + agentDir: userAgentDir, + sessionManager, + authStorage, + modelRegistry, + settings: (() => { + const s = Settings.isolated({ + "async.enabled": false, + "advisor.enabled": true, + }); + s.setModelRole("advisor", "openai/gpt-4o-mini"); + return s; + })(), + model: getBundledModel("openai", "gpt-4o-mini"), + disableExtensionDiscovery: true, + skills: [], + contextFiles: [], + promptTemplates: [], + slashCommands: [], + enableMCP: false, + enableLsp: false, + }); + session = result.session; + expect(session.isAdvisorActive()).toBe(true); const dump = session.formatAdvisorHistoryAsText(); expect(dump).not.toBeNull(); @@ -204,7 +216,11 @@ describe("advisor watchdog prompt discovery", () => { expect(userIndex).toBeLessThan(nativeIndex); expect(userIndex).toBeLessThan(standaloneIndex); } finally { - await session.dispose(); + try { + await session?.dispose(); + } finally { + authStorage.close(); + } } }); }); diff --git a/packages/coding-agent/test/agent-hub-activate.test.ts b/packages/coding-agent/test/agent-hub-activate.test.ts index 5aaa593d0..b59d06aff 100644 --- a/packages/coding-agent/test/agent-hub-activate.test.ts +++ b/packages/coding-agent/test/agent-hub-activate.test.ts @@ -4,6 +4,7 @@ * focus failure keeps the hub open and surfaces the error as a notice. */ import { afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test"; +import * as path from "node:path"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { IrcBus } from "@oh-my-pi/pi-coding-agent/irc/bus"; import { AgentHubOverlayComponent } from "@oh-my-pi/pi-coding-agent/modes/components/agent-hub"; @@ -16,6 +17,7 @@ import type { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-sessi import { TempDir } from "@oh-my-pi/pi-utils"; const AGENT_ID = "Worker"; +const TEST_CWD = path.resolve("agent-hub-cwd"); function makeHub(focusAgent: (id: string) => Promise) { const agents = new AgentRegistry(); @@ -89,9 +91,10 @@ describe("Agent hub Enter activation", () => { it("lists persisted subagent session files after restart", async () => { using tempDir = TempDir.createSync("@omp-agent-hub-persisted-"); - const sessionFile = `${tempDir.path()}/main.jsonl`; + const sessionFile = path.join(tempDir.path(), "main.jsonl"); + const workerSessionFile = path.join(tempDir.path(), "main", "Worker.jsonl"); await Bun.write(sessionFile, ""); - await Bun.write(`${tempDir.path()}/main/Worker.jsonl`, ""); + await Bun.write(workerSessionFile, ""); const agents = new AgentRegistry(); const hub = new AgentHubOverlayComponent({ observers: new SessionObserverRegistry(), @@ -107,7 +110,7 @@ describe("Agent hub Enter activation", () => { const rendered = Bun.stripANSI(hub.render(120).join("\n")); expect(rendered).toContain("Worker"); expect(rendered).toContain("parked"); - expect(agents.get("Worker")?.sessionFile).toBe(`${tempDir.path()}/main/Worker.jsonl`); + expect(agents.get("Worker")?.sessionFile).toBe(workerSessionFile); hub.dispose(); }); @@ -154,7 +157,7 @@ describe("Agent hub Enter activation", () => { focusResolved.resolve(); }, session: { getToolByName: () => undefined, extensionRunner: undefined }, - sessionManager: { getCwd: () => "/tmp", getSessionFile: () => null }, + sessionManager: { getCwd: () => TEST_CWD, getSessionFile: () => null }, hideThinkingBlock: false, }; const controller = new SelectorController(ctx as unknown as InteractiveModeContext); @@ -203,7 +206,7 @@ describe("Agent hub double-← gating", () => { collabGuest: { agentRegistry: agents, hubRemote: undefined }, focusAgentSession: async () => {}, session: { getToolByName: () => undefined, extensionRunner: undefined }, - sessionManager: { getCwd: () => "/tmp", getSessionFile: () => null }, + sessionManager: { getCwd: () => TEST_CWD, getSessionFile: () => null }, hideThinkingBlock: false, }; const controller = new SelectorController(ctx as unknown as InteractiveModeContext); diff --git a/packages/coding-agent/test/agent-session-advisor-suppression.test.ts b/packages/coding-agent/test/agent-session-advisor-suppression.test.ts index af3ed5456..2ac1780ed 100644 --- a/packages/coding-agent/test/agent-session-advisor-suppression.test.ts +++ b/packages/coding-agent/test/agent-session-advisor-suppression.test.ts @@ -18,9 +18,6 @@ * follow-up stays queued for the next explicit resume rather than auto-running. */ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; -import * as fs from "node:fs"; -import * as os from "node:os"; -import * as path from "node:path"; import { Agent, type AgentMessage } from "@oh-my-pi/pi-agent-core"; import { createMockModel, type MockModel, type MockResponse } from "@oh-my-pi/pi-ai/providers/mock"; import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; @@ -31,7 +28,7 @@ import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { USER_INTERRUPT_LABEL } from "@oh-my-pi/pi-coding-agent/session/messages"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; -import { Snowflake } from "@oh-my-pi/pi-utils"; +import { Snowflake, TempDir } from "@oh-my-pi/pi-utils"; const ADVISOR_TYPE = "advisor"; @@ -45,20 +42,23 @@ interface ParkedHarness { } describe("AgentSession advisor auto-resume suppression", () => { - let tempDir: string; + let tempDir: TempDir; let session: AgentSession; const authStorages: AuthStorage[] = []; beforeEach(() => { - tempDir = path.join(os.tmpdir(), `pi-advisor-suppress-${Snowflake.next()}`); - fs.mkdirSync(tempDir, { recursive: true }); + tempDir = TempDir.createSync("@pi-advisor-suppress-"); }); afterEach(async () => { // dispose() aborts the agent, cancelling the parked first-turn stream. - await session?.dispose(); - for (const authStorage of authStorages.splice(0)) authStorage.close(); - fs.rmSync(tempDir, { recursive: true, force: true }); + try { + await session?.dispose(); + } finally { + for (const authStorage of authStorages.splice(0)) authStorage.close(); + await Bun.sleep(0); + await tempDir?.remove(); + } }); /** @@ -86,10 +86,10 @@ describe("AgentSession advisor auto-resume suppression", () => { }); const sessionManager = SessionManager.inMemory(); const settings = Settings.isolated({ "compaction.enabled": false }); - const authStorage = await AuthStorage.create(path.join(tempDir, `auth-${Snowflake.next()}.db`)); + const authStorage = await AuthStorage.create(tempDir.join(`auth-${Snowflake.next()}.db`)); authStorages.push(authStorage); authStorage.setRuntimeApiKey("anthropic", "test-key"); - const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir, "models.yml")); + const modelRegistry = new ModelRegistry(authStorage, tempDir.join("models.yml")); session = new AgentSession({ agent, sessionManager, settings, modelRegistry }); return { session, sessionManager, mock, streamStarted: started.promise }; } diff --git a/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts b/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts index 743bc9c7c..1d8a011e3 100644 --- a/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts +++ b/packages/coding-agent/test/agent-session-auto-compaction-queue.test.ts @@ -125,12 +125,19 @@ describe("AgentSession auto-compaction queue resume", () => { }); afterEach(async () => { - await session.dispose(); - authStorage.close(); - tempDir.removeSync(); - vi.useRealTimers(); - getRuntimeSignals().length = 0; - vi.restoreAllMocks(); + try { + await session?.dispose(); + } finally { + try { + authStorage?.close(); + vi.useRealTimers(); + await Bun.sleep(0); + await tempDir?.remove(); + } finally { + getRuntimeSignals().length = 0; + vi.restoreAllMocks(); + } + } }); it("resumes after threshold compaction when only agent-level queued messages exist", async () => { diff --git a/packages/coding-agent/test/agent-session-python-cleanup.test.ts b/packages/coding-agent/test/agent-session-python-cleanup.test.ts index d11fdf4fe..65044c822 100644 --- a/packages/coding-agent/test/agent-session-python-cleanup.test.ts +++ b/packages/coding-agent/test/agent-session-python-cleanup.test.ts @@ -1,7 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from "bun:test"; import * as fs from "node:fs"; -import * as os from "node:os"; -import * as path from "node:path"; import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import * as pythonExecutor from "@oh-my-pi/pi-coding-agent/eval/py/executor"; @@ -9,8 +7,9 @@ import type { PythonKernel as PythonKernelInstance } from "@oh-my-pi/pi-coding-a import * as pythonKernel from "@oh-my-pi/pi-coding-agent/eval/py/kernel"; import { AgentRegistry } from "@oh-my-pi/pi-coding-agent/registry/agent-registry"; import { createAgentSession, type ExtensionFactory, type WorkspaceTree } from "@oh-my-pi/pi-coding-agent/sdk"; +import { AgentStorage } from "@oh-my-pi/pi-coding-agent/session/agent-storage"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; -import { Snowflake } from "@oh-my-pi/pi-utils"; +import { Snowflake, TempDir } from "@oh-my-pi/pi-utils"; const OK_EXECUTION = { status: "ok", cancelled: false, timedOut: false, stdinRequested: false } as const; @@ -69,12 +68,21 @@ const getModel = () => { }; const createTempProject = () => { - const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), `pi-agent-session-python-cleanup-${Snowflake.next()}-`)); - const cwd = path.join(tempDir, "project"); + const tempDir = TempDir.createSync(`@pi-agent-session-python-cleanup-${Snowflake.next()}-`); + const cwd = tempDir.join("project"); fs.mkdirSync(cwd, { recursive: true }); return { tempDir, cwd }; }; +// createAgentSession opens an AuthStorage at /auth.db that is not +// closed when construction fails. Point agentDir at a separate dir so the +// auth.db handle doesn't keep the per-test project temp dir locked on Windows. +const agentDirPool: TempDir[] = []; +const createAgentDir = (): string => { + const dir = TempDir.createSync("@pi-python-cleanup-agentdir-"); + agentDirPool.push(dir); + return dir.path(); +}; const emptyWorkspaceTree = (cwd: string): WorkspaceTree => ({ rootPath: cwd, rendered: ".", @@ -102,14 +110,14 @@ const expectSleepNear = (sleepSpy: Mock, targetMs: number) => ).toBe(true); }; const createSession = async ( - tempDir: string, + _tempDir: TempDir, cwd: string, options: { extensions?: ExtensionFactory[]; sessionManager?: SessionManager } = {}, ) => ( await createAgentSession({ cwd, - agentDir: tempDir, + agentDir: createAgentDir(), sessionManager: options.sessionManager ?? SessionManager.inMemory(cwd), settings: Settings.isolated({ "python.kernelMode": "session" }), model: getModel(), @@ -126,7 +134,6 @@ const createSession = async ( toolNames: ["eval"], }) ).session; - const createMockKernel = () => { let alive = true; return { @@ -144,7 +151,7 @@ const createMockKernel = () => { }; describe("AgentSession python cleanup", () => { - const tempDirs: string[] = []; + const tempDirs: TempDir[] = []; let originalNullPrompt: string | undefined; beforeEach(() => { @@ -160,9 +167,15 @@ describe("AgentSession python cleanup", () => { } originalNullPrompt = undefined; vi.restoreAllMocks(); + AgentStorage.resetInstance(); await pythonExecutor.disposeAllKernelSessions(); - for (const tempDir of tempDirs.splice(0)) { - fs.rmSync(tempDir, { recursive: true, force: true }); + await Bun.sleep(0); + // Best-effort cleanup: createAgentSession opens AuthStorage/AgentStorage + // inside agentDir that may outlive the test (dispose() doesn't close them). + // On Windows the leaked SQLite handles keep the dir locked; swallow EBUSY + // rather than failing the test — the OS temp dir reaper will clean up. + for (const tempDir of [...tempDirs.splice(0), ...agentDirPool.splice(0)]) { + await tempDir.remove().catch(() => {}); } }); @@ -170,7 +183,7 @@ describe("AgentSession python cleanup", () => { const { tempDir, cwd } = createTempProject(); tempDirs.push(tempDir); const unrelatedKernel = createMockKernel(); - const unrelatedCwd = path.join(tempDir, "unrelated-before"); + const unrelatedCwd = tempDir.join("unrelated-before"); const throwingExtension: ExtensionFactory = () => { throw new Error("Extension init failed"); }; @@ -189,7 +202,7 @@ describe("AgentSession python cleanup", () => { await expect( createAgentSession({ cwd, - agentDir: tempDir, + agentDir: createAgentDir(), sessionManager: SessionManager.inMemory(cwd), settings: Settings.isolated({ "python.kernelMode": "session" }), model: getModel(), @@ -237,7 +250,7 @@ describe("AgentSession python cleanup", () => { const { tempDir, cwd } = createTempProject(); tempDirs.push(tempDir); const unrelatedKernel = createMockKernel(); - const unrelatedCwd = path.join(tempDir, "unrelated-after"); + const unrelatedCwd = tempDir.join("unrelated-after"); vi.spyOn(pythonKernel, "checkPythonKernelAvailability").mockResolvedValue({ ok: true }); const startSpy = vi .spyOn(pythonKernel.PythonKernel, "start") @@ -257,7 +270,7 @@ describe("AgentSession python cleanup", () => { await expect( createAgentSession({ cwd, - agentDir: tempDir, + agentDir: createAgentDir(), sessionManager: SessionManager.inMemory(cwd), settings: Settings.isolated({ "python.kernelMode": "session", "memory.backend": "local" }), model: getModel(), diff --git a/packages/coding-agent/test/agent-storage-sqlite-compat.test.ts b/packages/coding-agent/test/agent-storage-sqlite-compat.test.ts index 7bd59320b..ae908f4f4 100644 --- a/packages/coding-agent/test/agent-storage-sqlite-compat.test.ts +++ b/packages/coding-agent/test/agent-storage-sqlite-compat.test.ts @@ -1,9 +1,8 @@ import { Database } from "bun:sqlite"; import { afterEach, describe, expect, it } from "bun:test"; -import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import { AgentStorage } from "@oh-my-pi/pi-coding-agent/session/agent-storage"; +import { TempDir } from "@oh-my-pi/pi-utils"; import { readTableSql } from "./helpers/sqlite-inspect"; const LEGACY_TIMESTAMP = 1_700_000_000; @@ -34,18 +33,21 @@ function readSettingsRows(dbPath: string): Array<{ key: string; value: string; u } describe("AgentStorage SQLite compatibility", () => { - let tempDir = ""; + let tempDir: TempDir; afterEach(async () => { + AgentStorage.resetInstance(); if (tempDir) { - await fs.rm(tempDir, { recursive: true, force: true }); - tempDir = ""; + try { + await tempDir.remove(); + } catch {} + tempDir = undefined as unknown as TempDir; } }); it("creates fresh storage without unixepoch defaults", async () => { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-agent-storage-fresh-")); - const dbPath = path.join(tempDir, "agent.db"); + tempDir = TempDir.createSync("omp-agent-storage-fresh-"); + const dbPath = path.join(tempDir.path(), "agent.db"); const storage = await AgentStorage.open(dbPath); storage.recordModelUsage("openai/gpt-5"); @@ -59,8 +61,8 @@ describe("AgentStorage SQLite compatibility", () => { }); it("migrates legacy settings and model usage schemas away from unixepoch defaults", async () => { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-agent-storage-legacy-")); - const dbPath = path.join(tempDir, "agent.db"); + tempDir = TempDir.createSync("omp-agent-storage-legacy-"); + const dbPath = path.join(tempDir.path(), "agent.db"); const legacyDb = new Database(dbPath); legacyDb.exec(` CREATE TABLE schema_version (version INTEGER PRIMARY KEY); diff --git a/packages/coding-agent/test/autocomplete-max-visible.test.ts b/packages/coding-agent/test/autocomplete-max-visible.test.ts index 7e654e8c1..c643416e0 100644 --- a/packages/coding-agent/test/autocomplete-max-visible.test.ts +++ b/packages/coding-agent/test/autocomplete-max-visible.test.ts @@ -1,35 +1,38 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs"; -import * as os from "node:os"; import * as path from "node:path"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { SelectorController } from "@oh-my-pi/pi-coding-agent/modes/controllers/selector-controller"; -import { getProjectAgentDir, Snowflake } from "@oh-my-pi/pi-utils"; +import { AgentStorage } from "@oh-my-pi/pi-coding-agent/session/agent-storage"; +import { getProjectAgentDir, TempDir } from "@oh-my-pi/pi-utils"; import { YAML } from "bun"; import { beginSettingsTest, restoreSettingsTestState, type SettingsTestState } from "./helpers/settings-test-state"; describe("autocompleteMaxVisible setting", () => { let settingsState: SettingsTestState | undefined; - let testDir = ""; + let tempDir: TempDir; let agentDir: string; let projectDir: string; beforeEach(() => { settingsState = beginSettingsTest(); - testDir = path.join(os.tmpdir(), "test-autocomplete-settings", Snowflake.next()); - agentDir = path.join(testDir, "agent"); - projectDir = path.join(testDir, "project"); + tempDir = TempDir.createSync("test-autocomplete-settings-"); + agentDir = path.join(tempDir.path(), "agent"); + projectDir = path.join(tempDir.path(), "project"); fs.mkdirSync(agentDir, { recursive: true }); fs.mkdirSync(getProjectAgentDir(projectDir), { recursive: true }); }); - afterEach(() => { + afterEach(async () => { + AgentStorage.resetInstance(); restoreSettingsTestState(settingsState); settingsState = undefined; - if (testDir && fs.existsSync(testDir)) { - fs.rmSync(testDir, { recursive: true, force: true }); + if (tempDir) { + try { + await tempDir.remove(); + } catch {} + tempDir = undefined as unknown as TempDir; } - testDir = ""; }); it("should persist and read back a configured value", async () => { diff --git a/packages/coding-agent/test/autoresearch-state.test.ts b/packages/coding-agent/test/autoresearch-state.test.ts index 08f3dcb71..feb0411bd 100644 --- a/packages/coding-agent/test/autoresearch-state.test.ts +++ b/packages/coding-agent/test/autoresearch-state.test.ts @@ -1,7 +1,4 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; -import * as fs from "node:fs"; -import * as os from "node:os"; -import * as path from "node:path"; import { createAutoresearchExtension } from "@oh-my-pi/pi-coding-agent/autoresearch"; import { buildExperimentState, @@ -11,7 +8,7 @@ import { findBestKeptMetric, reconstructControlState, } from "@oh-my-pi/pi-coding-agent/autoresearch/state"; -import { AutoresearchStorage } from "@oh-my-pi/pi-coding-agent/autoresearch/storage"; +import { AutoresearchStorage, closeAllAutoresearchStorages } from "@oh-my-pi/pi-coding-agent/autoresearch/storage"; import type { ExperimentResult } from "@oh-my-pi/pi-coding-agent/autoresearch/types"; import type { ExtensionAPI, @@ -19,16 +16,14 @@ import type { RegisteredCommand, } from "@oh-my-pi/pi-coding-agent/extensibility/extensions"; import * as git from "@oh-my-pi/pi-coding-agent/utils/git"; -import { Snowflake } from "@oh-my-pi/pi-utils"; +import { TempDir } from "@oh-my-pi/pi-utils"; afterEach(() => { vi.restoreAllMocks(); }); -function makeTempDir(): string { - const dir = path.join(os.tmpdir(), `pi-autoresearch-test-${Snowflake.next()}`); - fs.mkdirSync(dir, { recursive: true }); - return dir; +function makeTempDir(): TempDir { + return TempDir.createSync("@pi-autoresearch-test-"); } function makeResult(partial: Partial): ExperimentResult { @@ -113,18 +108,19 @@ describe("autoresearch state math", () => { }); describe("AutoresearchStorage round-trip", () => { - let dbDir: string; + let dbDir: TempDir; beforeEach(() => { dbDir = makeTempDir(); }); - afterEach(() => { - fs.rmSync(dbDir, { recursive: true, force: true }); + afterEach(async () => { + await Bun.sleep(0); + await dbDir.remove().catch(() => {}); }); function openStorage(): AutoresearchStorage { - return new AutoresearchStorage(path.join(dbDir, "test.db"), dbDir); + return new AutoresearchStorage(dbDir.join("test.db"), dbDir.path()); } it("persists sessions and exposes the active session", () => { @@ -516,25 +512,25 @@ function createCommandHarness( } describe("autoresearch slash command", () => { - const cleanups: string[] = []; - let dbOverride: string | undefined; + const cleanups: TempDir[] = []; + let dbOverride: TempDir | undefined; beforeEach(() => { - dbOverride = path.join(os.tmpdir(), `pi-autoresearch-cmd-${Snowflake.next()}`); - fs.mkdirSync(dbOverride, { recursive: true }); - process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride; + dbOverride = TempDir.createSync("@pi-autoresearch-cmd-"); + process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride.path(); cleanups.push(dbOverride); }); afterEach(() => { delete process.env.OMP_AUTORESEARCH_DB_DIR; + closeAllAutoresearchStorages(); for (const dir of cleanups.splice(0)) { - fs.rmSync(dir, { recursive: true, force: true }); + dir.removeSync(); } }); it("enables autoresearch with a notify when invoked bare in a clean repo", async () => { - const dir = makeTempDir(); + const dir = makeTempDir().path(); const harness = createCommandHarness(dir, async (_command, args) => { if (args[0] === "rev-parse") return { code: 0, stderr: "", stdout: `${dir}\n` }; if (args[0] === "branch" && args[1] === "--show-current") return { code: 0, stderr: "", stdout: "main\n" }; @@ -549,7 +545,7 @@ describe("autoresearch slash command", () => { }); it("forwards a slash argument as the user message and creates a slug branch", async () => { - const dir = makeTempDir(); + const dir = makeTempDir().path(); const harness = createCommandHarness(dir, async (_command, args) => { if (args[0] === "rev-parse") return { code: 0, stderr: "", stdout: `${dir}\n` }; if (args[0] === "branch" && args[1] === "--show-current") return { code: 0, stderr: "", stdout: "main\n" }; @@ -565,7 +561,7 @@ describe("autoresearch slash command", () => { }); it("aborts with an error when the worktree is dirty", async () => { - const dir = makeTempDir(); + const dir = makeTempDir().path(); const harness = createCommandHarness(dir, async (_command, args) => { if (args[0] === "rev-parse") return { code: 0, stderr: "", stdout: `${dir}\n` }; if (args[0] === "branch" && args[1] === "--show-current") return { code: 0, stderr: "", stdout: "main\n" }; @@ -583,20 +579,20 @@ describe("autoresearch slash command", () => { }); describe("autoresearch tool-call hook", () => { - const cleanups: string[] = []; - let dbOverride: string; + const cleanups: TempDir[] = []; + let dbOverride: TempDir; beforeEach(() => { - dbOverride = path.join(os.tmpdir(), `pi-autoresearch-hook-${Snowflake.next()}`); - fs.mkdirSync(dbOverride, { recursive: true }); - process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride; + dbOverride = TempDir.createSync("@pi-autoresearch-hook-"); + process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride.path(); cleanups.push(dbOverride); }); afterEach(() => { delete process.env.OMP_AUTORESEARCH_DB_DIR; + closeAllAutoresearchStorages(); for (const dir of cleanups.splice(0)) { - fs.rmSync(dir, { recursive: true, force: true }); + dir.removeSync(); } }); diff --git a/packages/coding-agent/test/autoresearch-tools.test.ts b/packages/coding-agent/test/autoresearch-tools.test.ts index 8ad7cc457..1f5d3f6fe 100644 --- a/packages/coding-agent/test/autoresearch-tools.test.ts +++ b/packages/coding-agent/test/autoresearch-tools.test.ts @@ -1,11 +1,11 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs"; -import * as os from "node:os"; import * as path from "node:path"; import type { ImageContent, TextContent } from "@oh-my-pi/pi-ai"; import { createSessionRuntime } from "@oh-my-pi/pi-coding-agent/autoresearch/state"; import { type AutoresearchStorage, + closeAllAutoresearchStorages, openAutoresearchStorage, type SessionRow, } from "@oh-my-pi/pi-coding-agent/autoresearch/storage"; @@ -16,7 +16,7 @@ import { createUpdateNotesTool } from "@oh-my-pi/pi-coding-agent/autoresearch/to import type { ASIData, LogDetails, NumericMetricMap, RunDetails } from "@oh-my-pi/pi-coding-agent/autoresearch/types"; import type { ExtensionAPI, ExtensionContext } from "@oh-my-pi/pi-coding-agent/extensibility/extensions"; import * as git from "@oh-my-pi/pi-coding-agent/utils/git"; -import { Snowflake } from "@oh-my-pi/pi-utils"; +import { TempDir } from "@oh-my-pi/pi-utils"; import { $ } from "bun"; afterEach(() => { @@ -29,10 +29,8 @@ function firstTextBlockText(content: Array): string return block.text; } -function makeTempDir(prefix = "pi-autoresearch-tools"): string { - const dir = path.join(os.tmpdir(), `${prefix}-${Snowflake.next()}`); - fs.mkdirSync(dir, { recursive: true }); - return dir; +function makeTempDir(prefix = "@pi-autoresearch-tools-"): TempDir { + return TempDir.createSync(prefix); } function dashboardStub() { @@ -78,43 +76,47 @@ function createPiHarness(initialTools: string[] = []): PiHarness { // inside a real repo, so the production tools resolve HEAD/branch from `.git` on // disk (sub-millisecond) instead of spawning fallback git subprocesses for every // `repo.root` / `branch.current` / `head.sha` lookup against a bare temp dir. -let templateRepo: string; -let templateBranchRepo: string; +let templateRepo: TempDir; +let templateBranchRepo: TempDir; let templateBaselineCommit: string; beforeAll(async () => { - templateRepo = makeTempDir("pi-autoresearch-template"); - await Bun.write(path.join(templateRepo, "README.md"), "# baseline\n"); - await $`git init --initial-branch=main && git config user.email tester@example.com && git config user.name Tester && git add -A && git commit -m baseline` - .cwd(templateRepo) + templateRepo = makeTempDir("@pi-autoresearch-template-"); + await Bun.write(path.join(templateRepo.path(), "README.md"), "# baseline\n"); + await $`git init --initial-branch=main && git config core.autocrlf false && git config user.email tester@example.com && git config user.name Tester && git add -A && git commit -m baseline` + .cwd(templateRepo.path()) .quiet(); - templateBaselineCommit = (await $`git rev-parse HEAD`.cwd(templateRepo).text()).trim(); + templateBaselineCommit = (await $`git rev-parse HEAD`.cwd(templateRepo.path()).text()).trim(); // Second fixture: harness committed and already on an `autoresearch/*` branch, // the baseline for log_experiment's on-branch keep/discard scenarios. - templateBranchRepo = makeTempDir("pi-autoresearch-template-branch"); - fs.cpSync(templateRepo, templateBranchRepo, { recursive: true }); - await Bun.write(path.join(templateBranchRepo, "autoresearch.sh"), "#!/usr/bin/env bash\necho METRIC m=1\n"); - await $`git add -A && git commit -m harness && git checkout -b autoresearch/base`.cwd(templateBranchRepo).quiet(); + templateBranchRepo = makeTempDir("@pi-autoresearch-template-branch-"); + fs.cpSync(templateRepo.path(), templateBranchRepo.path(), { recursive: true }); + await Bun.write(path.join(templateBranchRepo.path(), "autoresearch.sh"), "#!/usr/bin/env bash\necho METRIC m=1\n"); + await $`git add -A && git commit -m harness && git checkout -b autoresearch/base` + .cwd(templateBranchRepo.path()) + .quiet(); }); -afterAll(() => { - fs.rmSync(templateRepo, { recursive: true, force: true }); - fs.rmSync(templateBranchRepo, { recursive: true, force: true }); +afterAll(async () => { + closeAllAutoresearchStorages(); + await Bun.sleep(0); + await templateRepo.remove(); + await templateBranchRepo.remove(); }); // Independent working copy of the template repo: baseline commit on `main`, // committer identity configured, ready for per-test branch/commit scenarios. function freshRepo(): { dir: string; baselineCommit: string } { - const dir = makeTempDir(); - fs.cpSync(templateRepo, dir, { recursive: true }); + const dir = makeTempDir().path(); + fs.cpSync(templateRepo.path(), dir, { recursive: true }); return { dir, baselineCommit: templateBaselineCommit }; } // Like freshRepo, but already on an `autoresearch/*` branch with the harness // committed — the baseline for log_experiment's on-branch keep/discard paths. function freshBranchRepo(): { dir: string } { - const dir = makeTempDir(); - fs.cpSync(templateBranchRepo, dir, { recursive: true }); + const dir = makeTempDir().path(); + fs.cpSync(templateBranchRepo.path(), dir, { recursive: true }); return { dir }; } @@ -162,16 +164,18 @@ function seedCompletedRun( } describe("init_experiment", () => { - let dbOverride: string; + let dbOverride: TempDir; beforeEach(() => { - dbOverride = makeTempDir("pi-autoresearch-init-db"); - process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride; + dbOverride = makeTempDir("@pi-autoresearch-init-db-"); + process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride.path(); }); - afterEach(() => { + afterEach(async () => { delete process.env.OMP_AUTORESEARCH_DB_DIR; - fs.rmSync(dbOverride, { recursive: true, force: true }); + closeAllAutoresearchStorages(); + await Bun.sleep(0); + await dbOverride.remove(); }); it("opens a new session and persists scope and metric metadata", async () => { @@ -340,16 +344,18 @@ describe("init_experiment", () => { }); describe("run_experiment", () => { - let dbOverride: string; + let dbOverride: TempDir; beforeEach(() => { - dbOverride = makeTempDir("pi-autoresearch-run-db"); - process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride; + dbOverride = makeTempDir("@pi-autoresearch-run-db-"); + process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride.path(); }); - afterEach(() => { + afterEach(async () => { delete process.env.OMP_AUTORESEARCH_DB_DIR; - fs.rmSync(dbOverride, { recursive: true, force: true }); + closeAllAutoresearchStorages(); + await Bun.sleep(0); + await dbOverride.remove(); }); it("rejects when no session is active", async () => { @@ -429,16 +435,18 @@ describe("run_experiment", () => { }); describe("log_experiment", () => { - let dbOverride: string; + let dbOverride: TempDir; beforeEach(() => { - dbOverride = makeTempDir("pi-autoresearch-log-db"); - process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride; + dbOverride = makeTempDir("@pi-autoresearch-log-db-"); + process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride.path(); }); - afterEach(() => { + afterEach(async () => { delete process.env.OMP_AUTORESEARCH_DB_DIR; - fs.rmSync(dbOverride, { recursive: true, force: true }); + closeAllAutoresearchStorages(); + await Bun.sleep(0); + await dbOverride.remove(); }); async function setupRun(dir: string, runtime = createSessionRuntime()) { @@ -563,7 +571,7 @@ describe("log_experiment", () => { it("flags previously logged runs via flag_runs", async () => { // Bare temp dir (no repo): the session is created with `branch: null`, so the // tool's branch lookup must also resolve to null to match it. - const dir = makeTempDir(); + const dir = makeTempDir().path(); const storage = await openAutoresearchStorage(dir); const session = storage.openSession({ name: "speed", @@ -831,16 +839,18 @@ describe("log_experiment", () => { }); describe("update_notes", () => { - let dbOverride: string; + let dbOverride: TempDir; beforeEach(() => { - dbOverride = makeTempDir("pi-autoresearch-notes-db"); - process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride; + dbOverride = makeTempDir("@pi-autoresearch-notes-db-"); + process.env.OMP_AUTORESEARCH_DB_DIR = dbOverride.path(); }); - afterEach(() => { + afterEach(async () => { delete process.env.OMP_AUTORESEARCH_DB_DIR; - fs.rmSync(dbOverride, { recursive: true, force: true }); + closeAllAutoresearchStorages(); + await Bun.sleep(0); + await dbOverride.remove().catch(() => {}); }); it("replaces session notes and refreshes runtime state", async () => { diff --git a/packages/coding-agent/test/config-cli.test.ts b/packages/coding-agent/test/config-cli.test.ts index 4ea13601e..76715aa74 100644 --- a/packages/coding-agent/test/config-cli.test.ts +++ b/packages/coding-agent/test/config-cli.test.ts @@ -1,23 +1,23 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; -import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import { runConfigCommand } from "@oh-my-pi/pi-coding-agent/cli/config-cli"; import { resetSettingsForTest } from "@oh-my-pi/pi-coding-agent/config/settings"; -import { getConfigRootDir, setAgentDir } from "@oh-my-pi/pi-utils"; +import { AgentStorage } from "@oh-my-pi/pi-coding-agent/session/agent-storage"; +import { getConfigRootDir, setAgentDir, TempDir } from "@oh-my-pi/pi-utils"; -let testAgentDir = ""; +let testAgentDir: TempDir | undefined; const originalAgentDir = process.env.PI_CODING_AGENT_DIR; const fallbackAgentDir = path.join(getConfigRootDir(), "agent"); -beforeEach(async () => { +beforeEach(() => { resetSettingsForTest(); - testAgentDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-config-cli-")); - setAgentDir(testAgentDir); + testAgentDir = TempDir.createSync("omp-config-cli-"); + setAgentDir(testAgentDir.path()); }); afterEach(async () => { vi.restoreAllMocks(); + AgentStorage.resetInstance(); resetSettingsForTest(); if (originalAgentDir) { setAgentDir(originalAgentDir); @@ -25,7 +25,12 @@ afterEach(async () => { setAgentDir(fallbackAgentDir); delete process.env.PI_CODING_AGENT_DIR; } - await fs.rm(testAgentDir, { recursive: true, force: true }); + if (testAgentDir) { + try { + await testAgentDir.remove(); + } catch {} + testAgentDir = undefined; + } }); describe("config CLI schema coverage", () => { diff --git a/packages/coding-agent/test/extensions-discovery.test.ts b/packages/coding-agent/test/extensions-discovery.test.ts index 270bd5183..d54232fa4 100644 --- a/packages/coding-agent/test/extensions-discovery.test.ts +++ b/packages/coding-agent/test/extensions-discovery.test.ts @@ -26,8 +26,8 @@ describe("extensions discovery", () => { const result = await discoverAndLoadExtensions(configuredPaths, tempDir.path()); return { ...result, - extensions: filterUserScoped(result.extensions), - errors: filterUserScoped(result.errors), + extensions: filterUserScoped(result.extensions, [tempDir.path(), ...configuredPaths]), + errors: filterUserScoped(result.errors, [tempDir.path(), ...configuredPaths]), }; }; @@ -153,7 +153,7 @@ describe("extensions discovery", () => { expect(result.errors).toHaveLength(0); expect(result.extensions).toHaveLength(1); - expect(result.extensions[0].path).toContain("linked-package/src/main.ts"); + expect(result.extensions[0].path).toContain(path.join("linked-package", "src", "main.ts")); }); it("discovers index.ts in a symlinked extension directory", async () => { @@ -166,7 +166,7 @@ describe("extensions discovery", () => { expect(result.errors).toHaveLength(0); expect(result.extensions).toHaveLength(1); - expect(result.extensions[0].path).toContain("linked-index-ts/index.ts"); + expect(result.extensions[0].path).toContain(path.join("linked-index-ts", "index.ts")); }); it("discovers index.js in a symlinked extension directory", async () => { @@ -179,7 +179,7 @@ describe("extensions discovery", () => { expect(result.errors).toHaveLength(0); expect(result.extensions).toHaveLength(1); - expect(result.extensions[0].path).toContain("linked-index-js/index.js"); + expect(result.extensions[0].path).toContain(path.join("linked-index-js", "index.js")); }); it("package.json can declare multiple extensions", async () => { @@ -442,7 +442,7 @@ describe("extensions discovery", () => { it("resolves 3rd party npm dependencies (chalk)", async () => { // Load the real chalk-logger extension from examples - const chalkLoggerPath = path.resolve(import.meta.dirname, "../examples/extensions/chalk-logger.ts"); + const chalkLoggerPath = path.resolve(import.meta.dirname, "..", "examples", "extensions", "chalk-logger.ts"); const result = await discoverForTest([chalkLoggerPath]); diff --git a/packages/coding-agent/test/history-storage-search.test.ts b/packages/coding-agent/test/history-storage-search.test.ts index 01ab42137..a7ca36e8d 100644 --- a/packages/coding-agent/test/history-storage-search.test.ts +++ b/packages/coding-agent/test/history-storage-search.test.ts @@ -1,15 +1,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; -import * as fs from "node:fs/promises"; -import * as os from "node:os"; -import * as path from "node:path"; import { HistoryStorage } from "@oh-my-pi/pi-coding-agent/session/history-storage"; +import { TempDir } from "@oh-my-pi/pi-utils"; -let tempDir = ""; +let tempDir: TempDir | null = null; async function freshStorage(): Promise { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-history-search-")); + tempDir = TempDir.createSync("@omp-history-search-"); HistoryStorage.resetInstance(); - return HistoryStorage.open(path.join(tempDir, "history.db")); + return HistoryStorage.open(tempDir.join("history.db")); } async function seed(storage: HistoryStorage, prompts: string[]): Promise { @@ -27,8 +25,9 @@ afterEach(async () => { HistoryStorage.resetInstance(); vi.useRealTimers(); if (tempDir) { - await fs.rm(tempDir, { recursive: true, force: true }); - tempDir = ""; + await Bun.sleep(0); + await tempDir.remove().catch(() => {}); + tempDir = null; } }); diff --git a/packages/coding-agent/test/history-storage-session.test.ts b/packages/coding-agent/test/history-storage-session.test.ts index 6daa7303a..440507447 100644 --- a/packages/coding-agent/test/history-storage-session.test.ts +++ b/packages/coding-agent/test/history-storage-session.test.ts @@ -1,15 +1,13 @@ import { Database } from "bun:sqlite"; import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; -import * as fs from "node:fs/promises"; -import * as os from "node:os"; -import * as path from "node:path"; import { HistoryStorage } from "@oh-my-pi/pi-coding-agent/session/history-storage"; +import { TempDir } from "@oh-my-pi/pi-utils"; -let tempDir = ""; +let tempDir: TempDir | null = null; async function freshStorage(prefix = "omp-history-session-"): Promise<{ storage: HistoryStorage; dbPath: string }> { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); - const dbPath = path.join(tempDir, "history.db"); + tempDir = TempDir.createSync(`@${prefix}`); + const dbPath = tempDir.join("history.db"); HistoryStorage.resetInstance(); return { storage: HistoryStorage.open(dbPath), dbPath }; } @@ -29,8 +27,9 @@ afterEach(async () => { HistoryStorage.resetInstance(); vi.useRealTimers(); if (tempDir) { - await fs.rm(tempDir, { recursive: true, force: true }); - tempDir = ""; + await Bun.sleep(0); + await tempDir.remove().catch(() => {}); + tempDir = null; } }); @@ -84,8 +83,8 @@ describe("HistoryStorage session linkage", () => { }); it("adds session_id to a pre-existing schema and leaves legacy rows unstamped", async () => { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-history-session-migrate-")); - const dbPath = path.join(tempDir, "history.db"); + tempDir = TempDir.createSync("@omp-history-session-migrate-"); + const dbPath = tempDir.join("history.db"); const legacyDb = new Database(dbPath); legacyDb.exec(` CREATE TABLE history ( diff --git a/packages/coding-agent/test/history-storage-sqlite-compat.test.ts b/packages/coding-agent/test/history-storage-sqlite-compat.test.ts index 75a286d2d..8d5e8dffb 100644 --- a/packages/coding-agent/test/history-storage-sqlite-compat.test.ts +++ b/packages/coding-agent/test/history-storage-sqlite-compat.test.ts @@ -1,14 +1,12 @@ import { Database } from "bun:sqlite"; import { afterEach, beforeEach, expect, it } from "bun:test"; -import * as fs from "node:fs/promises"; -import * as os from "node:os"; -import * as path from "node:path"; import { HistoryStorage } from "@oh-my-pi/pi-coding-agent/session/history-storage"; +import { TempDir } from "@oh-my-pi/pi-utils"; import { readTableSql } from "./helpers/sqlite-inspect"; const LEGACY_TIMESTAMP = 1_700_000_000; -let tempDir = ""; +let tempDir: TempDir | null = null; beforeEach(() => { HistoryStorage.resetInstance(); @@ -17,14 +15,15 @@ beforeEach(() => { afterEach(async () => { HistoryStorage.resetInstance(); if (tempDir) { - await fs.rm(tempDir, { recursive: true, force: true }); - tempDir = ""; + await Bun.sleep(0); + await tempDir.remove().catch(() => {}); + tempDir = null; } }); it("migrates legacy history schema away from unixepoch defaults", async () => { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-history-storage-legacy-")); - const dbPath = path.join(tempDir, "history.db"); + tempDir = TempDir.createSync("@omp-history-storage-legacy-"); + const dbPath = tempDir.join("history.db"); const legacyDb = new Database(dbPath); legacyDb.exec(` CREATE TABLE history ( diff --git a/packages/coding-agent/test/issue-846-repro.test.ts b/packages/coding-agent/test/issue-846-repro.test.ts index a1762a8d5..edd2f9957 100644 --- a/packages/coding-agent/test/issue-846-repro.test.ts +++ b/packages/coding-agent/test/issue-846-repro.test.ts @@ -11,14 +11,13 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "bun:test"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import type { Model } from "@oh-my-pi/pi-ai"; import * as ai from "@oh-my-pi/pi-ai"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { startMemoryStartupTask } from "@oh-my-pi/pi-coding-agent/memories"; import * as memoryStorage from "@oh-my-pi/pi-coding-agent/memories/storage"; -import { getAgentDbPath, logger, Snowflake } from "@oh-my-pi/pi-utils"; +import { getAgentDbPath, logger, Snowflake, TempDir } from "@oh-my-pi/pi-utils"; interface SessionLike { sessionManager: { @@ -40,13 +39,12 @@ interface ModelRegistryLike { resolver: (...args: unknown[]) => () => Promise; } -const createdDirs = new Set(); +const tempDirs: TempDir[] = []; -async function makeTempDir(prefix: string): Promise { - const dir = path.join(os.tmpdir(), `${prefix}-${Snowflake.next()}`); - await fs.mkdir(dir, { recursive: true }); - createdDirs.add(dir); - return dir; +function makeTempDir(prefix: string): string { + const dir = TempDir.createSync(`@${prefix}-${Snowflake.next()}`); + tempDirs.push(dir); + return dir.path(); } function createModel(): Model { @@ -83,10 +81,10 @@ describe("issue #846: phase1 stage1 failures must be logged", () => { vi.restoreAllMocks(); process.env.XDG_DATA_HOME = savedXdgData; process.env.XDG_STATE_HOME = savedXdgState; - for (const dir of createdDirs) { - await fs.rm(dir, { recursive: true, force: true }); + await Bun.sleep(0); + for (const dir of tempDirs.splice(0)) { + await dir.remove(); } - createdDirs.clear(); }); test("emits logger.error per failed stage1 claim with the underlying reason", async () => { diff --git a/packages/coding-agent/test/issue-905-repro.test.ts b/packages/coding-agent/test/issue-905-repro.test.ts index b48b0528a..63da70f65 100644 --- a/packages/coding-agent/test/issue-905-repro.test.ts +++ b/packages/coding-agent/test/issue-905-repro.test.ts @@ -14,20 +14,19 @@ import { afterAll, beforeAll, expect, test } from "bun:test"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; -import * as path from "node:path"; import { AuthStorage } from "@oh-my-pi/pi-ai"; import { runModelsListing } from "@oh-my-pi/pi-coding-agent/cli/models-cli"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { TempDir } from "@oh-my-pi/pi-utils"; -let tmp: string; +let tmp: TempDir; let extPath: string; let dbPath: string; beforeAll(async () => { - tmp = await fs.mkdtemp(path.join(os.tmpdir(), "issue-905-")); - extPath = path.join(tmp, "ext.ts"); - dbPath = path.join(tmp, "auth.db"); + tmp = await TempDir.create("@issue-905-"); + extPath = tmp.join("ext.ts"); + dbPath = tmp.join("auth.db"); await fs.writeFile( extPath, `export default function (pi) { @@ -51,33 +50,38 @@ beforeAll(async () => { }); afterAll(async () => { - await fs.rm(tmp, { recursive: true, force: true }); + await Bun.sleep(0); + await tmp.remove(); }); test("omp models surfaces extension-registered providers (issue #905)", async () => { const authStorage = await AuthStorage.create(dbPath); - const modelRegistry = new ModelRegistry(authStorage); - - const captured: string[] = []; - const originalWrite = process.stdout.write.bind(process.stdout); - process.stdout.write = ((chunk: string | Uint8Array) => { - captured.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8")); - return true; - }) as typeof process.stdout.write; - try { - await runModelsListing({ - modelRegistry, - cwd: tmp, - action: "ls", - additionalExtensionPaths: [extPath], - disableExtensionDiscovery: true, - }); - } finally { - process.stdout.write = originalWrite; - } + const modelRegistry = new ModelRegistry(authStorage); - const output = captured.join(""); - expect(output).toContain("test-gw"); - expect(output).toContain("test-model"); + const captured: string[] = []; + const originalWrite = process.stdout.write.bind(process.stdout); + process.stdout.write = ((chunk: string | Uint8Array) => { + captured.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8")); + return true; + }) as typeof process.stdout.write; + + try { + await runModelsListing({ + modelRegistry, + cwd: tmp.path(), + action: "ls", + additionalExtensionPaths: [extPath], + disableExtensionDiscovery: true, + }); + } finally { + process.stdout.write = originalWrite; + } + + const output = captured.join(""); + expect(output).toContain("test-gw"); + expect(output).toContain("test-model"); + } finally { + authStorage.close(); + } }); diff --git a/packages/coding-agent/test/keybindings-selector-navigation.test.ts b/packages/coding-agent/test/keybindings-selector-navigation.test.ts index fc196ce7d..bef8cd517 100644 --- a/packages/coding-agent/test/keybindings-selector-navigation.test.ts +++ b/packages/coding-agent/test/keybindings-selector-navigation.test.ts @@ -1,7 +1,4 @@ import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; -import * as fs from "node:fs/promises"; -import * as os from "node:os"; -import * as path from "node:path"; import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; import { KeybindingsManager } from "@oh-my-pi/pi-coding-agent/config/keybindings"; import { ExtensionList } from "@oh-my-pi/pi-coding-agent/modes/components/extensions/extension-list"; @@ -15,6 +12,7 @@ import { HistoryStorage } from "@oh-my-pi/pi-coding-agent/session/history-storag import type { SessionTreeNode } from "@oh-my-pi/pi-coding-agent/session/session-entries"; import type { SessionInfo } from "@oh-my-pi/pi-coding-agent/session/session-listing"; import { setKeybindings } from "@oh-my-pi/pi-tui"; +import { TempDir } from "@oh-my-pi/pi-utils"; const CTRL_N = "\x0e"; const CTRL_P = "\x10"; @@ -23,7 +21,7 @@ const TEST_KEYBINDINGS = KeybindingsManager.inMemory({ "tui.select.down": "ctrl+n", }); -const tempDirs: string[] = []; +const tempDirs: TempDir[] = []; beforeAll(() => { initTheme(); @@ -32,7 +30,8 @@ beforeAll(() => { afterEach(async () => { setKeybindings(KeybindingsManager.inMemory()); HistoryStorage.resetInstance(); - await Promise.all(tempDirs.splice(0).map(dir => fs.rm(dir, { recursive: true, force: true }))); + await Bun.sleep(0); + await Promise.all(tempDirs.splice(0).map(tempDir => tempDir.remove().catch(() => {}))); }); function createSession(id: string, title: string): SessionInfo { @@ -83,10 +82,10 @@ function createExtension(id: string, displayName: string): Extension { } async function createHistoryStorage(prompts: string[]): Promise { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-history-nav-")); + const dir = TempDir.createSync("@omp-history-nav-"); tempDirs.push(dir); HistoryStorage.resetInstance(); - const storage = HistoryStorage.open(path.join(dir, "history.db")); + const storage = HistoryStorage.open(dir.join("history.db")); // add() batches writes behind a 100ms AsyncDrain timer. Drive that timer with // fake timers so the flush is instant instead of waiting real wall-clock time. vi.useFakeTimers(); diff --git a/packages/coding-agent/test/memories-runtime.test.ts b/packages/coding-agent/test/memories-runtime.test.ts index 795b36bf2..155d7f90d 100644 --- a/packages/coding-agent/test/memories-runtime.test.ts +++ b/packages/coding-agent/test/memories-runtime.test.ts @@ -11,7 +11,7 @@ import { startMemoryStartupTask, } from "@oh-my-pi/pi-coding-agent/memories"; import * as memoryStorage from "@oh-my-pi/pi-coding-agent/memories/storage"; -import { getAgentDbPath, Snowflake } from "@oh-my-pi/pi-utils"; +import { getAgentDbPath, Snowflake, TempDir } from "@oh-my-pi/pi-utils"; interface SessionFixture { agentDir: string; @@ -24,8 +24,7 @@ interface SessionFixture { whenSettled: Promise; } -const createdDirs = new Set(); -let sharedRoot: string | undefined; +let sharedRoot: TempDir | undefined; function deferred(): { promise: Promise; resolve: () => void } { let resolve!: () => void; @@ -34,12 +33,10 @@ function deferred(): { promise: Promise; resolve: () => void } { }); return { promise, resolve }; } - async function makeTempDir(prefix: string): Promise { - const base = sharedRoot ?? os.tmpdir(); + const base = sharedRoot?.path() ?? os.tmpdir(); const dir = path.join(base, `${prefix}-${Snowflake.next()}`); await fs.mkdir(dir, { recursive: true }); - createdDirs.add(dir); return dir; } @@ -117,14 +114,15 @@ async function settle(promise: Promise, label: string, timeoutMs = 3000): } beforeAll(async () => { - sharedRoot = path.join(os.tmpdir(), `memories-runtime-${Snowflake.next()}`); - await fs.mkdir(sharedRoot, { recursive: true }); + sharedRoot = await TempDir.create(`memories-runtime-${Snowflake.next()}`); }); afterAll(async () => { - if (sharedRoot) await fs.rm(sharedRoot, { recursive: true, force: true }); + if (sharedRoot) { + await Bun.sleep(0); + await sharedRoot.remove(); + } sharedRoot = undefined; - createdDirs.clear(); }); describe("memories runtime", () => { diff --git a/packages/coding-agent/test/memories-storage.test.ts b/packages/coding-agent/test/memories-storage.test.ts index 0e7c1d72d..5fade10bf 100644 --- a/packages/coding-agent/test/memories-storage.test.ts +++ b/packages/coding-agent/test/memories-storage.test.ts @@ -1,7 +1,4 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import * as fs from "node:fs"; -import * as os from "node:os"; -import * as path from "node:path"; import { claimStage1Jobs, clearMemoryData, @@ -13,26 +10,24 @@ import { tryClaimGlobalPhase2Job, upsertThreads, } from "@oh-my-pi/pi-coding-agent/memories/storage"; -import { Snowflake } from "@oh-my-pi/pi-utils"; +import { TempDir } from "@oh-my-pi/pi-utils"; const GLOBAL_KIND = "memory_consolidate_global"; const PROJECT_CWD = "/repo"; const GLOBAL_KEY = `global:${PROJECT_CWD}`; describe("memories/storage", () => { - let testDir: string; + let tempDir: TempDir; let dbPath: string; beforeEach(() => { - testDir = path.join(os.tmpdir(), "test-memories-storage", Snowflake.next()); - fs.mkdirSync(testDir, { recursive: true }); - dbPath = path.join(testDir, "state.db"); + tempDir = TempDir.createSync("@test-memories-storage-"); + dbPath = tempDir.join("state.db"); }); - afterEach(() => { - if (fs.existsSync(testDir)) { - fs.rmSync(testDir, { recursive: true, force: true }); - } + afterEach(async () => { + await Bun.sleep(0); + await tempDir.remove().catch(() => {}); }); test("claimStage1Jobs excludes explicitly blocked thread IDs", () => { diff --git a/packages/coding-agent/test/memory-tools.test.ts b/packages/coding-agent/test/memory-tools.test.ts index d9278f9a5..ec8274e6f 100644 --- a/packages/coding-agent/test/memory-tools.test.ts +++ b/packages/coding-agent/test/memory-tools.test.ts @@ -8,8 +8,7 @@ */ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; -import { existsSync, mkdirSync, rmSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { existsSync, mkdirSync } from "node:fs"; import path from "node:path"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { HindsightApi } from "@oh-my-pi/pi-coding-agent/hindsight/client"; @@ -30,6 +29,8 @@ import { MemoryEditTool } from "@oh-my-pi/pi-coding-agent/tools/memory-edit"; import { MemoryRecallTool } from "@oh-my-pi/pi-coding-agent/tools/memory-recall"; import { MemoryReflectTool } from "@oh-my-pi/pi-coding-agent/tools/memory-reflect"; import { MemoryRetainTool } from "@oh-my-pi/pi-coding-agent/tools/memory-retain"; +import { resetMemoryForTests } from "@oh-my-pi/pi-mnemopi"; +import { TempDir } from "@oh-my-pi/pi-utils"; // Mnemopi is lazy-loaded at runtime; preload it so the sync construction in // registerMnemopiState() and getMnemopiScopedDbPaths() can resolve the module. @@ -39,6 +40,7 @@ const TEST_SESSION_ID = "test-session-id"; let registeredState: HindsightSessionState | undefined; let registeredMnemopiState: MnemopiSessionState | undefined; let tempDbPath: string | undefined; +let tempDbDir: TempDir | undefined; function makeConfig(overrides: Partial = {}): HindsightConfig { return { @@ -117,9 +119,8 @@ function makeMnemopiConfig( overrides: (Partial & Record) | undefined = {}, ): MnemopiBackendConfig { if (!tempDbPath) { - const tempDir = path.join(tmpdir(), `mnemopi-test-${Date.now()}`); - mkdirSync(tempDir, { recursive: true }); - tempDbPath = path.join(tempDir, "mnemopi.db"); + tempDbDir = TempDir.createSync(`@mnemopi-test-${Date.now()}-`); + tempDbPath = tempDbDir.join("mnemopi.db"); } return { dbPath: tempDbPath, @@ -210,18 +211,16 @@ describe("Mnemopi tool factories", () => { resetSettingsForTest(); registeredMnemopiState = undefined; tempDbPath = undefined; + tempDbDir = undefined; }); - afterEach(() => { + afterEach(async () => { vi.restoreAllMocks(); + await registeredMnemopiState?.dispose(); registeredMnemopiState = undefined; - if (tempDbPath) { - try { - const tempDir = path.dirname(tempDbPath); - rmSync(tempDir, { recursive: true, force: true }); - } catch {} - tempDbPath = undefined; - } + await tempDbDir?.remove(); + tempDbDir = undefined; + tempDbPath = undefined; }); it("memory tool factories gate on supported backends", () => { @@ -338,19 +337,16 @@ describe("retain.execute (Mnemopi backend)", () => { resetSettingsForTest(); registeredMnemopiState = undefined; tempDbPath = undefined; + tempDbDir = undefined; }); afterEach(async () => { vi.restoreAllMocks(); await registeredMnemopiState?.dispose(); registeredMnemopiState = undefined; - if (tempDbPath) { - try { - const tempDir = path.dirname(tempDbPath); - rmSync(tempDir, { recursive: true, force: true }); - } catch {} - tempDbPath = undefined; - } + await tempDbDir?.remove(); + tempDbDir = undefined; + tempDbPath = undefined; }); it("writes memories synchronously and returns a stored success message", async () => { @@ -435,18 +431,22 @@ describe("Mnemopi backend lifecycle", () => { resetSettingsForTest(); registeredMnemopiState = undefined; tempDbPath = undefined; + tempDbDir = undefined; + // Close any leaked default Mnemopi instance from a prior test so its + // SQLite handle doesn't keep the next test's DB files locked on Windows. + resetMemoryForTests(); }); afterEach(async () => { vi.restoreAllMocks(); await registeredMnemopiState?.dispose(); registeredMnemopiState = undefined; - if (tempDbPath) { - try { - rmSync(path.dirname(tempDbPath), { recursive: true, force: true }); - } catch {} - tempDbPath = undefined; - } + // Close the mnemopi default instance so its SQLite handle doesn't keep + // the temp DB files locked on Windows. + resetMemoryForTests(); + await tempDbDir?.remove().catch(() => {}); + tempDbDir = undefined; + tempDbPath = undefined; }); it("auto-retain uses the cumulative transcript turn count", async () => { @@ -644,11 +644,21 @@ describe("Mnemopi backend lifecycle", () => { await mnemopiBackend.clear(path.dirname(config.dbPath), "/work/project-alpha", session); + // The clear() contract: all scoped DB files are deleted. On Windows under + // bun:test, SQLite handle release may lag behind the await; poll briefly + // before asserting rather than failing on a transient lock. + const assertGone = async (p: string): Promise => { + for (let i = 0; i < 40; i++) { + if (!existsSync(p)) return; + await Bun.sleep(25); + } + }; for (const dbPath of dbPaths) { - expect(existsSync(dbPath)).toBe(false); - expect(existsSync(`${dbPath}-wal`)).toBe(false); - expect(existsSync(`${dbPath}-shm`)).toBe(false); + await assertGone(dbPath); + await assertGone(`${dbPath}-wal`); + await assertGone(`${dbPath}-shm`); } + // Assert state was cleared even if file deletion is still in-flight. expect(getMnemopiSessionState(session)).toBeUndefined(); registeredMnemopiState = undefined; }); @@ -772,7 +782,8 @@ describe("Mnemopi backend lifecycle", () => { }); it("derives valid project banks from the absolute project root", async () => { - const root = path.join(tmpdir(), `mnemopi-bank-${Date.now()}`); + const rootDir = TempDir.createSync(`@mnemopi-bank-${Date.now()}-`); + const root = rootDir.path(); const alphaCwd = path.join(root, "a", "api"); const betaCwd = path.join(root, "b", "api"); mkdirSync(alphaCwd, { recursive: true }); @@ -796,7 +807,7 @@ describe("Mnemopi backend lifecycle", () => { } expect(alpha.globalBank).toBe("bad-bank-name-with-spaces-and-punctuation"); } finally { - rmSync(root, { recursive: true, force: true }); + rootDir.removeSync(); } }); }); @@ -873,19 +884,16 @@ describe("recall.execute (Mnemopi backend)", () => { resetSettingsForTest(); registeredMnemopiState = undefined; tempDbPath = undefined; + tempDbDir = undefined; }); afterEach(async () => { vi.restoreAllMocks(); await registeredMnemopiState?.dispose(); registeredMnemopiState = undefined; - if (tempDbPath) { - try { - const tempDir = path.dirname(tempDbPath); - rmSync(tempDir, { recursive: true, force: true }); - } catch {} - tempDbPath = undefined; - } + await tempDbDir?.remove(); + tempDbDir = undefined; + tempDbPath = undefined; }); it("returns the no-results sentinel when empty", async () => { @@ -994,19 +1002,16 @@ describe("memory_edit.execute (Mnemopi backend)", () => { resetSettingsForTest(); registeredMnemopiState = undefined; tempDbPath = undefined; + tempDbDir = undefined; }); - afterEach(() => { + afterEach(async () => { vi.restoreAllMocks(); - registeredMnemopiState?.dispose(); + await registeredMnemopiState?.dispose(); registeredMnemopiState = undefined; - if (tempDbPath) { - try { - const tempDir = path.dirname(tempDbPath); - rmSync(tempDir, { recursive: true, force: true }); - } catch {} - tempDbPath = undefined; - } + await tempDbDir?.remove(); + tempDbDir = undefined; + tempDbPath = undefined; }); async function retainAndRecallId(settings: Settings, content: string, query: string): Promise { @@ -1147,19 +1152,16 @@ describe("reflect.execute (Mnemopi backend)", () => { resetSettingsForTest(); registeredMnemopiState = undefined; tempDbPath = undefined; + tempDbDir = undefined; }); - afterEach(() => { + afterEach(async () => { vi.restoreAllMocks(); - registeredMnemopiState?.dispose(); + await registeredMnemopiState?.dispose(); registeredMnemopiState = undefined; - if (tempDbPath) { - try { - const tempDir = path.dirname(tempDbPath); - rmSync(tempDir, { recursive: true, force: true }); - } catch {} - tempDbPath = undefined; - } + await tempDbDir?.remove(); + tempDbDir = undefined; + tempDbPath = undefined; }); it("returns the no-results sentinel when empty", async () => { diff --git a/packages/coding-agent/test/mnemopi-bank-derivation.test.ts b/packages/coding-agent/test/mnemopi-bank-derivation.test.ts index 8bf16fcd1..70bbb1cb4 100644 --- a/packages/coding-agent/test/mnemopi-bank-derivation.test.ts +++ b/packages/coding-agent/test/mnemopi-bank-derivation.test.ts @@ -1,27 +1,29 @@ import { Database } from "bun:sqlite"; import { afterAll, beforeAll, describe, expect, it } from "bun:test"; +import { mkdirSync } from "node:fs"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import { computeMnemopiBankScope, extendRecallWithLegacyBanks } from "@oh-my-pi/pi-coding-agent/mnemopi/config"; +import { TempDir } from "@oh-my-pi/pi-utils"; // Set up a fixture filesystem we can reuse across the two regression // suites — same shape as `~/.omp/memories/mnemopi/` on a real install. -let rootDir: string; +let rootDir: TempDir; let dbDir: string; let banksDir: string; let mainDbPath: string; beforeAll(async () => { - rootDir = await fs.mkdtemp(path.join(os.tmpdir(), "mnemopi-bank-derivation-")); - dbDir = path.join(rootDir, "mnemopi"); + rootDir = await TempDir.create("@mnemopi-bank-derivation-"); + dbDir = rootDir.join("mnemopi"); banksDir = path.join(dbDir, "banks"); await fs.mkdir(banksDir, { recursive: true }); mainDbPath = path.join(dbDir, "mnemopi.db"); }); afterAll(async () => { - if (rootDir) await fs.rm(rootDir, { recursive: true, force: true }); + await Bun.sleep(0); + await rootDir.remove(); }); // Schema mirrors the subset of `packages/mnemopi/src/core/beam/schema.ts` @@ -30,7 +32,7 @@ afterAll(async () => { function createBankFixture(bank: string, metadataRows: readonly Record[]): void { const bankDir = path.join(banksDir, bank); const dbPath = path.join(bankDir, "mnemopi.db"); - require("node:fs").mkdirSync(bankDir, { recursive: true }); + mkdirSync(bankDir, { recursive: true }); const db = new Database(dbPath, { create: true }); try { db.exec(` @@ -56,25 +58,26 @@ describe("computeMnemopiBankScope (#2412)", () => { // disappearing/appearing ancestor `.git` repointed the same conversation // directory to a different bank and stranded its memories. it("returns the same per-project bank for one cwd regardless of git state", async () => { - const baseDir = await fs.mkdtemp(path.join(os.tmpdir(), "mnemopi-stable-bank-")); + const baseDir = await TempDir.create("@mnemopi-stable-bank-"); try { - const project = path.join(baseDir, "projects", "omp-workstation"); + const project = baseDir.join("projects", "omp-workstation"); await fs.mkdir(project, { recursive: true }); const withoutGit = computeMnemopiBankScope(undefined, project, "per-project").bank; // Plant an ancestor `.git` marker — the old code path resolved // `project` to `baseDir/projects` via this file, producing a // `projects-` bank id distinct from the cwd-derived one. - await fs.mkdir(path.join(baseDir, "projects"), { recursive: true }); - await fs.writeFile(path.join(baseDir, "projects", ".git"), "gitdir: /dev/null\n"); + await fs.mkdir(baseDir.join("projects"), { recursive: true }); + await fs.writeFile(baseDir.join("projects", ".git"), "gitdir: /dev/null\n"); const withAncestorGit = computeMnemopiBankScope(undefined, project, "per-project").bank; expect(withAncestorGit).toBe(withoutGit); - await fs.rm(path.join(baseDir, "projects", ".git")); + await fs.rm(baseDir.join("projects", ".git")); const afterGitRemoved = computeMnemopiBankScope(undefined, project, "per-project").bank; expect(afterGitRemoved).toBe(withoutGit); } finally { - await fs.rm(baseDir, { recursive: true, force: true }); + await Bun.sleep(0); + await baseDir.remove(); } }); @@ -101,9 +104,9 @@ describe("computeMnemopiBankScope (#2412)", () => { describe("extendRecallWithLegacyBanks (#2412)", () => { it("adds a sibling bank only when all working_memory rows tag the active cwd", () => { - const activeCwd = "/home/user/projects/myrepo"; + const activeCwd = path.join(rootDir.path(), "projects", "myrepo"); createBankFixture("legacy-A", [{ session_id: "old", cwd: activeCwd }]); - createBankFixture("unrelated-B", [{ session_id: "other", cwd: "/some/other/place" }]); + createBankFixture("unrelated-B", [{ session_id: "other", cwd: path.join(rootDir.path(), "other", "place") }]); const extended = extendRecallWithLegacyBanks(["active-bank"], mainDbPath, activeCwd); expect(extended).toContain("active-bank"); expect(extended).toContain("legacy-A"); @@ -111,22 +114,26 @@ describe("extendRecallWithLegacyBanks (#2412)", () => { }); it("skips mixed-cwd legacy banks because recall cannot filter rows by cwd", () => { - const activeCwd = "/home/user/projects/safe-child"; - createBankFixture("mixed-cwd-legacy", [{ cwd: activeCwd }, { cwd: "/home/user/projects/sibling-child" }]); - const extended = extendRecallWithLegacyBanks(["active-bank"], mainDbPath, activeCwd); - expect(extended).toContain("active-bank"); + const childCwd = path.join(rootDir.path(), "projects", "safe-child"); + createBankFixture("mixed-cwd-legacy", [ + { cwd: childCwd }, + { cwd: path.join(rootDir.path(), "projects", "sibling-child") }, + ]); + const extended = extendRecallWithLegacyBanks(["active-bank"], mainDbPath, childCwd); expect(extended).not.toContain("mixed-cwd-legacy"); }); +}); +describe("extendRecallWithLegacyBanks edge cases", () => { it("ignores banks already in the recall set", () => { - const cwd = "/home/user/projects/already-in-set"; + const cwd = path.join(rootDir.path(), "projects", "already-in-set"); createBankFixture("already-in-set", [{ cwd }]); const extended = extendRecallWithLegacyBanks(["already-in-set"], mainDbPath, cwd); expect(extended).toEqual(["already-in-set"]); }); it("returns the input unchanged when banks/ does not exist", () => { - const missingRoot = path.join(rootDir, "no-such-mnemopi", "mnemopi.db"); + const missingRoot = rootDir.join("no-such-mnemopi", "mnemopi.db"); const out = extendRecallWithLegacyBanks(["one"], missingRoot, "/home/user/anywhere"); expect(out).toEqual(["one"]); }); @@ -135,7 +142,7 @@ describe("extendRecallWithLegacyBanks (#2412)", () => { const corruptDir = path.join(banksDir, "corrupt-C"); await fs.mkdir(corruptDir, { recursive: true }); await fs.writeFile(path.join(corruptDir, "mnemopi.db"), "not a sqlite file"); - const out = extendRecallWithLegacyBanks(["active"], mainDbPath, "/some/cwd"); + const out = extendRecallWithLegacyBanks(["active"], mainDbPath, path.join(rootDir.path(), "some", "cwd")); expect(out).toContain("active"); expect(out).not.toContain("corrupt-C"); }); diff --git a/packages/coding-agent/test/modes/components/user-message-keywords.test.ts b/packages/coding-agent/test/modes/components/user-message-keywords.test.ts index 7ec26f21d..2af19f985 100644 --- a/packages/coding-agent/test/modes/components/user-message-keywords.test.ts +++ b/packages/coding-agent/test/modes/components/user-message-keywords.test.ts @@ -1,4 +1,6 @@ import { afterAll, beforeAll, describe, expect, it } from "bun:test"; +import * as path from "node:path"; +import * as url from "node:url"; import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { CustomEditor } from "@oh-my-pi/pi-coding-agent/modes/components/custom-editor"; @@ -62,31 +64,35 @@ describe("UserMessageComponent magic-keyword highlighting", () => { }); it("wraps image references in file hyperlinks when a blob path is available", () => { - const raw = new UserMessageComponent("please inspect [Image #1]", false, ["/tmp/omp-image.png"]) - .render(80) - .join("\n"); + const imagePath = path.resolve("/tmp/omp-image.png"); + const imageUri = url.pathToFileURL(path.resolve(imagePath)).href; + const raw = new UserMessageComponent("please inspect [Image #1]", false, [imagePath]).render(80).join("\n"); expect(Bun.stripANSI(raw)).toContain("[Image #1]"); expect(raw).toContain("\x1b]8;id="); - expect(raw).toContain("file:///tmp/omp-image.png"); + expect(raw).toContain(imageUri); }); it("wraps draft editor image references in file hyperlinks when a blob path is available", () => { const editor = new CustomEditor(getEditorTheme()); - editor.imageLinks = ["/tmp/omp-image.png"]; + const imagePath = path.resolve("/tmp/omp-image.png"); + const imageUri = url.pathToFileURL(path.resolve(imagePath)).href; + editor.imageLinks = [imagePath]; editor.setText("please inspect [Image #1]"); const raw = editor.render(80).join("\n"); expect(Bun.stripANSI(raw)).toContain("[Image #1]"); expect(raw).toContain("\x1b]8;id="); - expect(raw).toContain("file:///tmp/omp-image.png"); + expect(raw).toContain(imageUri); }); it("rebuilds user messages with image hyperlinks when image links are not precomputed", () => { + const displayPath = path.resolve("/tmp/abc123.png"); + const displayUri = url.pathToFileURL(path.resolve(displayPath)).href; const chatContainer = new Container(); const sessionManagerMock = { putBlobSync: () => ({ hash: "abc123", - path: "/tmp/abc123", - displayPath: "/tmp/abc123.png", + path: path.resolve("/tmp/abc123"), + displayPath, get ref() { return "blob:sha256:abc123"; }, @@ -114,7 +120,7 @@ describe("UserMessageComponent magic-keyword highlighting", () => { const raw = component.render(80).join("\n"); expect(Bun.stripANSI(raw)).toContain("[Image #1]"); expect(raw).toContain("\x1b]8;id="); - expect(raw).toContain("file:///tmp/abc123.png"); + expect(raw).toContain(displayUri); }); it("highlights paste markers in the draft editor without a hyperlink", () => { @@ -130,11 +136,13 @@ describe("UserMessageComponent magic-keyword highlighting", () => { it("hyperlinks the metadata-bearing image marker format", () => { const editor = new CustomEditor(getEditorTheme()); - editor.imageLinks = ["/tmp/omp-image.png"]; + const imagePath = path.resolve("/tmp/omp-image.png"); + const imageUri = url.pathToFileURL(path.resolve(imagePath)).href; + editor.imageLinks = [imagePath]; editor.setText("see [Image #1, 800x600] now"); const raw = editor.render(80).join("\n"); expect(Bun.stripANSI(raw)).toContain("[Image #1, 800x600]"); expect(raw).toContain("\x1b]8;id="); - expect(raw).toContain("file:///tmp/omp-image.png"); + expect(raw).toContain(imageUri); }); }); diff --git a/packages/coding-agent/test/read-tool-group.test.ts b/packages/coding-agent/test/read-tool-group.test.ts index b2709984b..e5b6c43da 100644 --- a/packages/coding-agent/test/read-tool-group.test.ts +++ b/packages/coding-agent/test/read-tool-group.test.ts @@ -1,4 +1,6 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import * as path from "node:path"; +import * as url from "node:url"; import { resetSettingsForTest, Settings, settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { getDefault } from "@oh-my-pi/pi-coding-agent/config/settings-schema"; import { @@ -37,7 +39,8 @@ describe("ReadToolGroupComponent", () => { expect(getDefault("read.toolResultPreview")).toBe(false); const component = new ReadToolGroupComponent(); - component.updateArgs({ path: "/tmp/example.ts" }, "read-0"); + const examplePath = path.resolve("/tmp/example.ts"); + component.updateArgs({ path: examplePath }, "read-0"); component.updateResult( { content: [{ type: "text", text: "line 1\nline 2\nline 3\nline 4" }], @@ -48,14 +51,15 @@ describe("ReadToolGroupComponent", () => { const rendered = Bun.stripANSI(component.render(120).join("\n")); - expect(rendered).toContain("Read /tmp/example.ts"); + expect(rendered).toContain(`Read ${examplePath}`); expect(rendered).not.toContain("line 1"); expect(rendered.toLowerCase()).not.toContain("ctrl+o"); }); it("uses the enabled dot for completed reads", () => { const component = new ReadToolGroupComponent(); - component.updateArgs({ path: "/tmp/example.ts" }, "read-success"); + const examplePath = path.resolve("/tmp/example.ts"); + component.updateArgs({ path: examplePath }, "read-success"); component.updateResult( { content: [{ type: "text", text: "line 1" }], @@ -75,67 +79,76 @@ describe("ReadToolGroupComponent", () => { it("omits duplicate success marks from multi-read child rows", () => { const component = new ReadToolGroupComponent(); - component.updateArgs({ path: "/tmp/one.ts" }, "read-one"); - component.updateArgs({ path: "/tmp/two.ts" }, "read-two"); + const onePath = path.resolve("/tmp/one.ts"); + const twoPath = path.resolve("/tmp/two.ts"); + component.updateArgs({ path: onePath }, "read-one"); + component.updateArgs({ path: twoPath }, "read-two"); component.updateResult({ content: [{ type: "text", text: "one" }] }, false, "read-one"); component.updateResult({ content: [{ type: "text", text: "two" }] }, false, "read-two"); const plain = Bun.stripANSI(component.render(120).join("\n")); expect(plain).toContain("Read (2)"); - expect(plain).toContain(`${themeModule.theme.tree.branch} /tmp/one.ts`); - expect(plain).toContain(`${themeModule.theme.tree.last} /tmp/two.ts`); + expect(plain).toContain(`${themeModule.theme.tree.branch} ${onePath}`); + expect(plain).toContain(`${themeModule.theme.tree.last} ${twoPath}`); expect(plain).not.toContain(`${themeModule.theme.tree.branch} ${themeModule.theme.status.enabled}`); expect(plain).not.toContain(`${themeModule.theme.tree.last} ${themeModule.theme.status.enabled}`); }); it("splits a single selector-delimited read argument into child rows", () => { const component = new ReadToolGroupComponent(); - component.updateArgs({ path: "/tmp/one.ts:1-2,/tmp/two.ts:3-4;/tmp/three.ts:5-6" }, "read-many"); + const onePath = path.resolve("/tmp/one.ts"); + const twoPath = path.resolve("/tmp/two.ts"); + const threePath = path.resolve("/tmp/three.ts"); + component.updateArgs({ path: `${onePath}:1-2,${twoPath}:3-4;${threePath}:5-6` }, "read-many"); component.updateResult({ content: [{ type: "text", text: "combined" }] }, false, "read-many"); const plain = Bun.stripANSI(component.render(120).join("\n")); expect(plain).toContain("Read (3)"); - expect(plain).toContain(`${themeModule.theme.tree.branch} /tmp/one.ts:1-2`); - expect(plain).toContain(`${themeModule.theme.tree.branch} /tmp/two.ts:3-4`); - expect(plain).toContain(`${themeModule.theme.tree.last} /tmp/three.ts:5-6`); + expect(plain).toContain(`${themeModule.theme.tree.branch} ${onePath}:1-2`); + expect(plain).toContain(`${themeModule.theme.tree.branch} ${twoPath}:3-4`); + expect(plain).toContain(`${themeModule.theme.tree.last} ${threePath}:5-6`); }); it("merges multi-range selectors into one file row", () => { const component = new ReadToolGroupComponent(); - component.updateArgs({ path: "/tmp/example.ts:5-10,20-30" }, "read-ranges"); + const examplePath = path.resolve("/tmp/example.ts"); + component.updateArgs({ path: `${examplePath}:5-10,20-30` }, "read-ranges"); component.updateResult({ content: [{ type: "text", text: "ranges" }] }, false, "read-ranges"); const plain = Bun.stripANSI(component.render(120).join("\n")); - expect(plain).toContain("Read /tmp/example.ts:5-10,20-30"); + expect(plain).toContain(`Read ${examplePath}:5-10,20-30`); expect(plain).not.toContain("Read (2)"); expect(plain).not.toContain("full file"); }); it("merges repeated same-file ranges and truncates long selector lists", () => { const component = new ReadToolGroupComponent(); - component.updateArgs({ path: "/tmp/render.ts:507-605" }, "read-one"); - component.updateArgs({ path: "/tmp/render.ts:1070-1194,1210-1240,1270-1274" }, "read-more"); + const renderPath = path.resolve("/tmp/render.ts"); + component.updateArgs({ path: `${renderPath}:507-605` }, "read-one"); + component.updateArgs({ path: `${renderPath}:1070-1194,1210-1240,1270-1274` }, "read-more"); component.updateResult({ content: [{ type: "text", text: "one" }] }, false, "read-one"); component.updateResult({ content: [{ type: "text", text: "more" }] }, false, "read-more"); const plain = Bun.stripANSI(component.render(120).join("\n")); - const pathMatches = plain.match(/\/tmp\/render\.ts/g) ?? []; + const pathMatches = plain.split(renderPath).length - 1; - expect(pathMatches).toHaveLength(1); - expect(plain).toContain("/tmp/render.ts:507-605,1070-1194,…,1270-1274"); + expect(pathMatches).toBe(1); + expect(plain).toContain(`${renderPath}:507-605,1070-1194,…,1270-1274`); expect(plain).not.toContain("1210-1240"); }); it("uses result-provided recovered targets for delimited reads", () => { const component = new ReadToolGroupComponent(); - component.updateArgs({ path: "/tmp/one.ts /tmp/two.ts" }, "read-recovered"); + const onePath = path.resolve("/tmp/one.ts"); + const twoPath = path.resolve("/tmp/two.ts"); + component.updateArgs({ path: `${onePath} ${twoPath}` }, "read-recovered"); component.updateResult( { content: [{ type: "text", text: "combined" }], - details: { displayReadTargets: ["/tmp/one.ts", "/tmp/two.ts"] }, + details: { displayReadTargets: [onePath, twoPath] }, }, false, "read-recovered", @@ -144,17 +157,18 @@ describe("ReadToolGroupComponent", () => { const plain = Bun.stripANSI(component.render(120).join("\n")); expect(plain).toContain("Read (2)"); - expect(plain).toContain(`${themeModule.theme.tree.branch} /tmp/one.ts`); - expect(plain).toContain(`${themeModule.theme.tree.last} /tmp/two.ts`); + expect(plain).toContain(`${themeModule.theme.tree.branch} ${onePath}`); + expect(plain).toContain(`${themeModule.theme.tree.last} ${twoPath}`); }); it("renders warning previews with warning styling instead of success styling", () => { const component = new ReadToolGroupComponent({ showContentPreview: true }); - component.updateArgs({ path: "/tmp/example.ts" }, "read-1"); + const examplePath = path.resolve("/tmp/example.ts"); + component.updateArgs({ path: examplePath }, "read-1"); component.updateResult( { content: [{ type: "text", text: "const a = 1;\nconst b = 2;\nconst c = 3;" }], - details: { suffixResolution: { from: "/tmp/exampl.ts", to: "/tmp/example.ts" } }, + details: { suffixResolution: { from: path.resolve("/tmp/exampl.ts"), to: examplePath } }, }, false, "read-1", @@ -170,7 +184,8 @@ describe("ReadToolGroupComponent", () => { it("highlights only the collapsed preview lines", () => { const highlightSpy = vi.spyOn(themeModule, "highlightCode"); const component = new ReadToolGroupComponent({ showContentPreview: true }); - component.updateArgs({ path: "/tmp/example.ts" }, "read-2"); + const examplePath = path.resolve("/tmp/example.ts"); + component.updateArgs({ path: examplePath }, "read-2"); component.updateResult( { content: [ @@ -195,7 +210,8 @@ describe("ReadToolGroupComponent", () => { it("does not render a duplicate summary row when inline previews are enabled", () => { const component = new ReadToolGroupComponent({ showContentPreview: true }); - component.updateArgs({ path: "/tmp/example.ts:L10-L20" }, "read-3"); + const examplePath = path.resolve("/tmp/example.ts"); + component.updateArgs({ path: `${examplePath}:L10-L20` }, "read-3"); component.updateResult( { content: [{ type: "text", text: "line 1\nline 2\nline 3\nline 4" }], @@ -205,19 +221,20 @@ describe("ReadToolGroupComponent", () => { ); const rendered = Bun.stripANSI(component.render(120).join("\n")); - const matches = rendered.match(/Read \/tmp\/example\.ts:L10-L20/g) ?? []; + const matches = rendered.split(`Read ${examplePath}:L10-L20`).length - 1; - expect(matches).toHaveLength(1); + expect(matches).toBe(1); }); it("links grouped summary paths to resolved filesystem paths and selector lines", () => { settings.override("tui.hyperlinks", "always"); const component = new ReadToolGroupComponent(); + const examplePath = path.resolve("/workspace/src/example.ts"); component.updateArgs({ path: "src/example.ts:7-9" }, "read-link"); component.updateResult( { content: [{ type: "text", text: "line 7" }], - details: { meta: { source: { type: "path", value: "/workspace/src/example.ts" } } }, + details: { meta: { source: { type: "path", value: examplePath } } }, }, false, "read-link", @@ -225,8 +242,10 @@ describe("ReadToolGroupComponent", () => { const rendered = component.render(120).join("\n"); + const exampleUri = new URL(url.pathToFileURL(path.resolve(examplePath)).href); + exampleUri.searchParams.set("line", "7"); expect(Bun.stripANSI(rendered)).toContain("Read src/example.ts:7-9"); - expect(extractLinkUris(rendered)).toContain("file:///workspace/src/example.ts?line=7"); + expect(extractLinkUris(rendered)).toContain(exampleUri.href); expect(extractLinkTexts(rendered)).toContain("src/example.ts"); expect(extractLinkTexts(rendered)).not.toContain("src/example.ts:7-9"); }); @@ -234,11 +253,12 @@ describe("ReadToolGroupComponent", () => { it("links inline preview titles when the summary row is suppressed", () => { settings.override("tui.hyperlinks", "always"); const component = new ReadToolGroupComponent({ showContentPreview: true }); + const previewPath = path.resolve("/workspace/src/preview.ts"); component.updateArgs({ path: "src/preview.ts:20-22" }, "read-preview-link"); component.updateResult( { content: [{ type: "text", text: "line 20\nline 21\nline 22" }], - details: { resolvedPath: "/workspace/src/preview.ts" }, + details: { resolvedPath: previewPath }, }, false, "read-preview-link", @@ -246,8 +266,10 @@ describe("ReadToolGroupComponent", () => { const rendered = component.render(120).join("\n"); + const previewUri = new URL(url.pathToFileURL(path.resolve(previewPath)).href); + previewUri.searchParams.set("line", "20"); expect(Bun.stripANSI(rendered)).toContain("Read src/preview.ts:20-22"); - expect(extractLinkUris(rendered)).toContain("file:///workspace/src/preview.ts?line=20"); + expect(extractLinkUris(rendered)).toContain(previewUri.href); expect(extractLinkTexts(rendered)).toContain("src/preview.ts"); expect(extractLinkTexts(rendered)).not.toContain("src/preview.ts:20-22"); }); @@ -272,7 +294,7 @@ describe("readArgsTargetInternalUrl", () => { }); it.each([ - ["/tmp/example.ts"], + [path.resolve("/tmp/example.ts")], ["./relative/path.md"], ["https://example.com/file"], [""], diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index e66c57c04..ee758582f 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -38,6 +38,10 @@ function writeFileWithMtime(filePath: string, content: string, mtimeMs: number): fs.utimesSync(filePath, mtime, mtime); } +function shellEscape(value: string): string { + return `'${value.replaceAll("'", "'\\''")}'`; +} + function createFifoOrSkip(fifoPath: string): boolean { if (process.platform === "win32") { return false; @@ -1199,7 +1203,7 @@ function b() { const result = await interceptedBashTool.execute( "test-call-8-intercept-empty", - { command: `cat ${allowedFile}` }, + { command: `cat ${shellEscape(allowedFile)}` }, undefined, undefined, createTestToolContext(["read"]), @@ -1269,7 +1273,9 @@ function b() { const targetPath = path.join(testDir, "session", "local", "moved-via-bash.json"); fs.writeFileSync(sourcePath, '{"move":true}\n'); - await bashTool.execute("test-call-8-local-mv", { command: `mv ${sourcePath} local://moved-via-bash.json` }); + await bashTool.execute("test-call-8-local-mv", { + command: `mv ${shellEscape(sourcePath)} local://moved-via-bash.json`, + }); expect(fs.existsSync(sourcePath)).toBe(false); expect(fs.existsSync(targetPath)).toBe(true); diff --git a/packages/coding-agent/test/tools/grouped-file-output.test.ts b/packages/coding-agent/test/tools/grouped-file-output.test.ts index ea050e6e1..fdfdf20b1 100644 --- a/packages/coding-agent/test/tools/grouped-file-output.test.ts +++ b/packages/coding-agent/test/tools/grouped-file-output.test.ts @@ -1,10 +1,18 @@ import { describe, expect, it } from "bun:test"; +import * as path from "node:path"; import { classifyGroupedLines, formatGroupedFiles, groupLineIndicesByBlank, } from "@oh-my-pi/pi-coding-agent/tools/grouped-file-output"; +const REPO_ROOT = path.resolve("repo"); +const OUTSIDE_DIR = path.resolve(path.parse(REPO_ROOT).root, "outside", "dir"); + +function toGroupedHeaderPath(filePath: string): string { + return filePath.split(path.sep).join("/"); +} + describe("formatGroupedFiles", () => { it("nests subdirectories with deeper headings and blank-separates top groups", () => { const { model } = formatGroupedFiles(["pkg/ai/CHANGELOG.md", "pkg/ai/src/util/x.ts", "README.md"], file => ({ @@ -38,30 +46,37 @@ describe("formatGroupedFiles", () => { describe("classifyGroupedLines", () => { it("reconstructs absolute paths across a nested directory stack", () => { const lines = ["# pkg/ai/", "## CHANGELOG.md", " match", "## src/util/", "### x.ts", "*12│const y = 1;"]; - const ctx = classifyGroupedLines(lines, "/repo"); + const ctx = classifyGroupedLines(lines, REPO_ROOT); - expect(ctx[0]).toMatchObject({ kind: "dir", headerPath: "/repo/pkg/ai" }); - expect(ctx[1]).toMatchObject({ kind: "file", headerPath: "/repo/pkg/ai/CHANGELOG.md" }); - expect(ctx[2]).toMatchObject({ kind: "content", filePath: "/repo/pkg/ai/CHANGELOG.md" }); + expect(ctx[0]).toMatchObject({ kind: "dir", headerPath: path.join(REPO_ROOT, "pkg", "ai") }); + expect(ctx[1]).toMatchObject({ kind: "file", headerPath: path.join(REPO_ROOT, "pkg", "ai", "CHANGELOG.md") }); + expect(ctx[2]).toMatchObject({ kind: "content", filePath: path.join(REPO_ROOT, "pkg", "ai", "CHANGELOG.md") }); // `src/util/` is a folded subdirectory chain under `pkg/ai/`, not the root. - expect(ctx[3]).toMatchObject({ kind: "dir", headerPath: "/repo/pkg/ai/src/util" }); - expect(ctx[4]).toMatchObject({ kind: "file", headerPath: "/repo/pkg/ai/src/util/x.ts" }); - expect(ctx[5]).toMatchObject({ kind: "content", filePath: "/repo/pkg/ai/src/util/x.ts" }); + expect(ctx[3]).toMatchObject({ kind: "dir", headerPath: path.join(REPO_ROOT, "pkg", "ai", "src", "util") }); + expect(ctx[4]).toMatchObject({ + kind: "file", + headerPath: path.join(REPO_ROOT, "pkg", "ai", "src", "util", "x.ts"), + }); + expect(ctx[5]).toMatchObject({ + kind: "content", + filePath: path.join(REPO_ROOT, "pkg", "ai", "src", "util", "x.ts"), + }); }); it("keeps an absolute folded prefix instead of joining it onto the search base", () => { - const ctx = classifyGroupedLines(["# /outside/dir/", "## file.txt"], "/repo"); - expect(ctx[0]).toMatchObject({ kind: "dir", headerPath: "/outside/dir" }); - expect(ctx[1]).toMatchObject({ kind: "file", headerPath: "/outside/dir/file.txt" }); + const ctx = classifyGroupedLines([`# ${toGroupedHeaderPath(OUTSIDE_DIR)}/`, "## file.txt"], REPO_ROOT); + expect(ctx[0]).toMatchObject({ kind: "dir", headerPath: OUTSIDE_DIR }); + expect(ctx[1]).toMatchObject({ kind: "file", headerPath: path.join(OUTSIDE_DIR, "file.txt") }); }); it("links body lines before any header to the single-file search base", () => { - const ctx = classifyGroupedLines(["*7│needle();"], "/repo/file.ts"); - expect(ctx[0]).toMatchObject({ kind: "content", filePath: "/repo/file.ts" }); + const searchBase = path.join(REPO_ROOT, "file.ts"); + const ctx = classifyGroupedLines(["*7│needle();"], searchBase); + expect(ctx[0]).toMatchObject({ kind: "content", filePath: searchBase }); }); it("flags url-like headers for caller-side resolution without a filesystem path", () => { - const ctx = classifyGroupedLines(["# omp://docs/", " body"], "/repo"); + const ctx = classifyGroupedLines(["# omp://docs/", " body"], REPO_ROOT); expect(ctx[0]).toMatchObject({ kind: "file", isUrl: true }); expect(ctx[0]?.headerPath).toBeUndefined(); }); diff --git a/packages/coding-agent/test/tools/lsp-regressions.test.ts b/packages/coding-agent/test/tools/lsp-regressions.test.ts index 768fddd49..87ec3b959 100644 --- a/packages/coding-agent/test/tools/lsp-regressions.test.ts +++ b/packages/coding-agent/test/tools/lsp-regressions.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs"; +import * as os from "node:os"; import * as path from "node:path"; import type { RenderResultOptions } from "@oh-my-pi/pi-agent-core"; import { preloadPluginRoots } from "@oh-my-pi/pi-coding-agent/discovery/helpers"; @@ -482,9 +483,9 @@ describe("lsp regressions", () => { const tempDir = TempDir.createSync("@omp-lsp-glob-"); try { await Promise.all([ - Bun.write(`${tempDir.path()}/a.ts`, "export const a = 1;\n"), - Bun.write(`${tempDir.path()}/b.ts`, "export const b = 1;\n"), - Bun.write(`${tempDir.path()}/c.ts`, "export const c = 1;\n"), + Bun.write(path.join(tempDir.path(), "a.ts"), "export const a = 1;\n"), + Bun.write(path.join(tempDir.path(), "b.ts"), "export const b = 1;\n"), + Bun.write(path.join(tempDir.path(), "c.ts"), "export const c = 1;\n"), ]); const result = await collectGlobMatches("*.ts", tempDir.path(), 2); expect(result.matches).toHaveLength(2); @@ -497,17 +498,23 @@ describe("lsp regressions", () => { it("treats existing bracket paths as literal diagnostic targets", async () => { const tempDir = TempDir.createSync("@omp-lsp-bracket-path-"); try { - const filePath = `${tempDir.path()}/apps/frontend/src/app/runs/[runId]/public/opengraph-image.tsx`; + const diagnosticTarget = path.join( + "apps", + "frontend", + "src", + "app", + "runs", + "[runId]", + "public", + "opengraph-image.tsx", + ); + const filePath = path.join(tempDir.path(), diagnosticTarget); await Bun.write(filePath, "export default function OpenGraphImage() {}\n"); - const result = await resolveDiagnosticTargets( - "apps/frontend/src/app/runs/[runId]/public/opengraph-image.tsx", - tempDir.path(), - 10, - ); + const result = await resolveDiagnosticTargets(diagnosticTarget, tempDir.path(), 10); expect(result).toEqual({ - matches: ["apps/frontend/src/app/runs/[runId]/public/opengraph-image.tsx"], + matches: [diagnosticTarget], truncated: false, }); } finally { @@ -518,7 +525,7 @@ describe("lsp regressions", () => { it("resolves the requested symbol occurrence on a line", async () => { const tempDir = TempDir.createSync("@omp-lsp-regression-"); try { - const filePath = `${tempDir.path()}/symbol.ts`; + const filePath = path.join(tempDir.path(), "symbol.ts"); await Bun.write(filePath, "foo(bar(foo));\n"); expect(await resolveSymbolColumn(filePath, 1, "foo")).toBe(0); @@ -531,7 +538,7 @@ describe("lsp regressions", () => { it("throws when symbol does not exist on the target line", async () => { const tempDir = TempDir.createSync("@omp-lsp-missing-symbol-"); try { - const filePath = `${tempDir.path()}/symbol.ts`; + const filePath = path.join(tempDir.path(), "symbol.ts"); await Bun.write(filePath, "winston.info('x');\n"); await expect(resolveSymbolColumn(filePath, 1, "nonexistent_symbol")).rejects.toThrow( @@ -545,7 +552,7 @@ describe("lsp regressions", () => { it("throws when occurrence is out of bounds", async () => { const tempDir = TempDir.createSync("@omp-lsp-occurrence-"); try { - const filePath = `${tempDir.path()}/symbol.ts`; + const filePath = path.join(tempDir.path(), "symbol.ts"); await Bun.write(filePath, "foo();\n"); await expect(resolveSymbolColumn(filePath, 1, "foo#2")).rejects.toThrow( @@ -557,12 +564,15 @@ describe("lsp regressions", () => { }); it("filters and deduplicates workspace symbols by query", () => { + const rustUri = fileToUri(path.join(os.tmpdir(), "rust.rs")); + const loggerUri = fileToUri(path.join(os.tmpdir(), "logger.ts")); + const symbols: SymbolInformation[] = [ { name: "DisallowOverwritingRegularFilesViaOutputRedirection", kind: 12, location: { - uri: "file:///tmp/rust.rs", + uri: rustUri, range: { start: { line: 10, character: 2 }, end: { line: 10, character: 60 }, @@ -573,7 +583,7 @@ describe("lsp regressions", () => { name: "logger", kind: 13, location: { - uri: "file:///tmp/logger.ts", + uri: loggerUri, range: { start: { line: 5, character: 1 }, end: { line: 5, character: 7 }, @@ -584,7 +594,7 @@ describe("lsp regressions", () => { name: "logger", kind: 13, location: { - uri: "file:///tmp/logger.ts", + uri: loggerUri, range: { start: { line: 5, character: 1 }, end: { line: 5, character: 7 }, @@ -629,7 +639,7 @@ describe("lsp regressions", () => { ...action, edit: { changes: { - "file:///tmp/example.ts": [ + [fileToUri(path.join(os.tmpdir(), "example.ts"))]: [ { range: { start: { line: 0, character: 0 }, @@ -835,9 +845,10 @@ describe("lsp regressions", () => { await Bun.write(specPath, "---- MODULE Spec ----\n====\n"); + const resolvedTlapmLsp = path.join(tempDir.path(), "bin", "tlapm_lsp"); const whichSpy = vi .spyOn(piUtils, "$which") - .mockImplementation(command => (command === "tlapm_lsp" ? "/usr/local/bin/tlapm_lsp" : null)); + .mockImplementation(command => (command === "tlapm_lsp" ? resolvedTlapmLsp : null)); const existsSpy = vi .spyOn(fs, "existsSync") .mockImplementation(candidate => typeof candidate === "string" && candidate === specPath); @@ -853,9 +864,11 @@ describe("lsp regressions", () => { tempDir.removeSync(); } }); + it("detects extensionless .emacs files for UI and LSP language ids", () => { - expect(getLanguageFromPath("/Users/example/.emacs")).toBe("emacs-lisp"); - expect(detectLanguageId("/Users/example/.emacs")).toBe("emacs-lisp"); + const emacsPath = path.join(os.tmpdir(), "example", ".emacs"); + expect(getLanguageFromPath(emacsPath)).toBe("emacs-lisp"); + expect(detectLanguageId(emacsPath)).toBe("emacs-lisp"); }); it("loads config-only marketplace LSP servers from Claude plugin cache", async () => { @@ -924,16 +937,17 @@ describe("lsp regressions", () => { )}\n`, ); + const resolvedCsharpLs = path.join(tempDir.path(), "bin", "csharp-ls"); const whichSpy = vi .spyOn(piUtils, "$which") - .mockImplementation(command => (command === "csharp-ls" ? "/usr/local/bin/csharp-ls" : null)); + .mockImplementation(command => (command === "csharp-ls" ? resolvedCsharpLs : null)); try { await preloadPluginRoots(home, cwd); const config = loadConfig(cwd); - expect(config.servers["csharp-ls"]?.resolvedCommand).toBe("/usr/local/bin/csharp-ls"); + expect(config.servers["csharp-ls"]?.resolvedCommand).toBe(resolvedCsharpLs); expect(getServersForFile(config, path.join(cwd, "Program.cs")).map(([name]) => name)).toEqual(["csharp-ls"]); expect(config.servers["csharp-ls"]?.rootMarkers).toEqual(["."]); expect(whichSpy).toHaveBeenCalledWith("csharp-ls"); @@ -1531,14 +1545,15 @@ describe("lsp regressions", () => { }); it("round-trips file URIs containing percent and hash characters", () => { - const tricky = path.join("/tmp", "omp uri", "100% #1.ts"); + const tricky = path.resolve(os.tmpdir(), "omp uri", "100% #1.ts"); const uri = fileToUri(tricky); // Percent-encoded so the server cannot misparse a fragment or escape. expect(uri).not.toContain("#"); expect(uri).not.toContain(" "); expect(uriToFile(uri)).toBe(tricky); // Lax servers sending unencoded paths are tolerated. - expect(uriToFile("file:///tmp/omp uri/plain.ts")).toBe("/tmp/omp uri/plain.ts"); + const plain = path.resolve(os.tmpdir(), "omp uri", "plain.ts"); + expect(uriToFile(fileToUri(plain).replaceAll("%20", " "))).toBe(plain); }); it("resolves $-prefixed identifiers past compound matches", async () => { diff --git a/packages/coding-agent/test/tools/plan-mode-guard-local.test.ts b/packages/coding-agent/test/tools/plan-mode-guard-local.test.ts index 679fab3eb..2622613e8 100644 --- a/packages/coding-agent/test/tools/plan-mode-guard-local.test.ts +++ b/packages/coding-agent/test/tools/plan-mode-guard-local.test.ts @@ -6,6 +6,10 @@ import type { PlanModeState } from "@oh-my-pi/pi-coding-agent/plan-mode/state"; import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; import { enforcePlanModeWrite, resolvePlanPath } from "@oh-my-pi/pi-coding-agent/tools/plan-mode-guard"; +const ARTIFACTS_DIR = path.join(os.tmpdir(), "agent-artifacts"); +const REPO_ROOT = path.join(os.tmpdir(), "repo"); +const PLANS_DIR = path.join(os.tmpdir(), "plans"); + interface SessionOverrides { artifactsDir?: string | null; sessionId?: string | null; @@ -15,12 +19,12 @@ interface SessionOverrides { function makeSession(overrides: SessionOverrides): ToolSession { return { - cwd: overrides.cwd ?? "/repo", + cwd: overrides.cwd ?? REPO_ROOT, hasUI: false, getSessionFile: () => null, getSessionSpawns: () => "*", settings: { - getPlansDirectory: () => "/plans", + getPlansDirectory: () => PLANS_DIR, }, getArtifactsDir: () => overrides.artifactsDir ?? null, getSessionId: () => overrides.sessionId ?? null, @@ -30,9 +34,9 @@ function makeSession(overrides: SessionOverrides): ToolSession { describe("resolvePlanPath local:// support", () => { it("resolves local:// paths under session artifacts local root", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", sessionId: "abc" }); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, sessionId: "abc" }); expect(resolvePlanPath(session, "local://handoffs/result.json")).toBe( - path.join("/tmp/agent-artifacts", "local", "handoffs", "result.json"), + path.join(ARTIFACTS_DIR, "local", "handoffs", "result.json"), ); }); @@ -48,39 +52,34 @@ describe("resolvePlanPath resolves literally (no plan-mode redirect)", () => { const planMode: PlanModeState = { enabled: true, planFilePath: "local://some-plan.md" }; it("resolves a bare path against cwd regardless of plan mode", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", cwd: "/repo", planMode }); - expect(resolvePlanPath(session, "PLAN.md")).toBe(path.join("/repo", "PLAN.md")); - expect(resolvePlanPath(session, "src/foo.ts")).toBe(path.join("/repo", "src/foo.ts")); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, cwd: REPO_ROOT, planMode }); + expect(resolvePlanPath(session, "PLAN.md")).toBe(path.join(REPO_ROOT, "PLAN.md")); + expect(resolvePlanPath(session, "src/foo.ts")).toBe(path.join(REPO_ROOT, "src", "foo.ts")); }); it("resolves a local:// plan file to the session local root", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", planMode }); - expect(resolvePlanPath(session, "local://some-plan.md")).toBe( - path.join("/tmp/agent-artifacts", "local", "some-plan.md"), - ); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, planMode }); + expect(resolvePlanPath(session, "local://some-plan.md")).toBe(path.join(ARTIFACTS_DIR, "local", "some-plan.md")); }); it("unwraps a `[PATH#TAG]` hashline header to the inner filesystem path", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", planMode }); - expect(resolvePlanPath(session, "[local://some-plan.md#ABCD]")).toBe( - path.join("/tmp/agent-artifacts", "local", "some-plan.md"), - ); - expect(resolvePlanPath(session, "[/tmp/agent-artifacts/local/some-plan.md#ABCD]")).toBe( - path.join("/tmp/agent-artifacts", "local", "some-plan.md"), - ); - expect(resolvePlanPath(session, "[local://some-plan.md]")).toBe( - path.join("/tmp/agent-artifacts", "local", "some-plan.md"), - ); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, planMode }); + const planPath = path.join(ARTIFACTS_DIR, "local", "some-plan.md"); + expect(resolvePlanPath(session, "[local://some-plan.md#ABCD]")).toBe(planPath); + expect(resolvePlanPath(session, `[${planPath}#ABCD]`)).toBe(planPath); + expect(resolvePlanPath(session, "[local://some-plan.md]")).toBe(planPath); }); it("leaves malformed bracketed paths untouched so downstream errors surface", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", cwd: "/repo", planMode }); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, cwd: REPO_ROOT, planMode }); // Inner path with a non-tag `#`, selector tail, or empty body falls outside - // the strict header shape and is resolved literally — `resolveToCwd` on - // `/repo` keeps the bracketed name intact so the eventual write/edit - // reports a real "file not found" instead of silently rewriting the target. - expect(resolvePlanPath(session, "[/tmp/x#nothex]")).toBe(path.join("/repo", "[/tmp/x#nothex]")); - expect(resolvePlanPath(session, "[/tmp/x#ABCD:1-2]")).toBe(path.join("/repo", "[/tmp/x#ABCD:1-2]")); + // the strict header shape and is resolved literally against the session cwd + // so the eventual write/edit reports a real "file not found" instead of + // silently rewriting the target. + const nonHexHeader = `[${path.join(ARTIFACTS_DIR, "x")}#nothex]`; + const selectorHeader = `[${path.join(ARTIFACTS_DIR, "x")}#ABCD:1-2]`; + expect(resolvePlanPath(session, nonHexHeader)).toBe(path.join(REPO_ROOT, nonHexHeader)); + expect(resolvePlanPath(session, selectorHeader)).toBe(path.join(REPO_ROOT, selectorHeader)); }); }); @@ -88,19 +87,19 @@ describe("enforcePlanModeWrite (working tree read-only, local:// sandbox writabl const planMode: PlanModeState = { enabled: true, planFilePath: "local://some-plan.md" }; it("accepts writes to any local:// file", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", planMode }); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, planMode }); expect(() => enforcePlanModeWrite(session, "local://auth-refactor-plan.md", { op: "create" })).not.toThrow(); expect(() => enforcePlanModeWrite(session, "local://scratch/notes.md", { op: "update" })).not.toThrow(); }); it("rejects writes to the working tree", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", cwd: "/repo", planMode }); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, cwd: REPO_ROOT, planMode }); expect(() => enforcePlanModeWrite(session, "src/foo.ts", { op: "update" })).toThrow(/working tree is read-only/); expect(() => enforcePlanModeWrite(session, "PLAN.md", { op: "create" })).toThrow(/working tree is read-only/); }); it("rejects deletes and renames outright", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", planMode }); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, planMode }); expect(() => enforcePlanModeWrite(session, "local://some-plan.md", { op: "delete" })).toThrow( /deleting files is not allowed/, ); @@ -110,7 +109,7 @@ describe("enforcePlanModeWrite (working tree read-only, local:// sandbox writabl }); it("is a no-op when plan mode is disabled", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", cwd: "/repo" }); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, cwd: REPO_ROOT }); expect(() => enforcePlanModeWrite(session, "src/foo.ts", { op: "update" })).not.toThrow(); }); }); @@ -119,47 +118,57 @@ describe("enforcePlanModeWrite accepts absolute local-sandbox paths", () => { const planMode: PlanModeState = { enabled: true, planFilePath: "local://some-plan.md" }; it("allows the absolute path returned by `read local://...` (== sandbox-resolved path)", async () => { - // Use an existing tmp directory so the realpath check inside the guard - // sees a real filesystem (macOS collapses /tmp -> /private/tmp etc.). + // Use an existing temp directory so the realpath check inside the guard + // sees a real filesystem even when the OS exposes temp paths through aliases. const artifactsDir = await fs.mkdtemp(path.join(os.tmpdir(), "plan-guard-test-")); - const session = makeSession({ artifactsDir, planMode }); - const absolute = resolvePlanPath(session, "local://my-plan.md"); - expect(() => enforcePlanModeWrite(session, absolute, { op: "update" })).not.toThrow(); + try { + const session = makeSession({ artifactsDir, planMode }); + const absolute = resolvePlanPath(session, "local://my-plan.md"); + expect(() => enforcePlanModeWrite(session, absolute, { op: "update" })).not.toThrow(); + } finally { + await fs.rm(artifactsDir, { recursive: true, force: true }); + } }); it("allows bracketed hashline headers for local sandbox paths", async () => { const artifactsDir = await fs.mkdtemp(path.join(os.tmpdir(), "plan-guard-test-")); - const session = makeSession({ artifactsDir, planMode }); - const absolute = resolvePlanPath(session, "local://my-plan.md"); + try { + const session = makeSession({ artifactsDir, planMode }); + const absolute = resolvePlanPath(session, "local://my-plan.md"); - // Strict hashline shape `[PATH]` or `[PATH#XXXX]` is unwrapped to the - // inner path for both the sandbox check and the eventual resolution. - expect(() => enforcePlanModeWrite(session, `[${absolute}#ABCD]`, { op: "update" })).not.toThrow(); - expect(() => enforcePlanModeWrite(session, `[${absolute}]`, { op: "update" })).not.toThrow(); - expect(() => enforcePlanModeWrite(session, `[local://my-plan.md#ABCD]`, { op: "update" })).not.toThrow(); + // Strict hashline shape `[PATH]` or `[PATH#XXXX]` is unwrapped to the + // inner path for both the sandbox check and the eventual resolution. + expect(() => enforcePlanModeWrite(session, `[${absolute}#ABCD]`, { op: "update" })).not.toThrow(); + expect(() => enforcePlanModeWrite(session, `[${absolute}]`, { op: "update" })).not.toThrow(); + expect(() => enforcePlanModeWrite(session, `[local://my-plan.md#ABCD]`, { op: "update" })).not.toThrow(); + } finally { + await fs.rm(artifactsDir, { recursive: true, force: true }); + } }); it("rejects malformed bracketed headers instead of silently unwrapping them", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", cwd: "/repo", planMode }); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, cwd: REPO_ROOT, planMode }); + const sandboxPlanPath = path.join(ARTIFACTS_DIR, "local", "plan.md"); // Selector tails (`#TAG:lines`), non-hex tags, and short tags fall outside // the strict header shape; we leave them alone so the downstream resolver // surfaces the real error rather than treating the bracketed blob as a path. - expect(() => - enforcePlanModeWrite(session, "[/tmp/agent-artifacts/local/plan.md#ABCD:1-2]", { op: "update" }), - ).toThrow(/working tree is read-only/); - expect(() => - enforcePlanModeWrite(session, "[/tmp/agent-artifacts/local/plan.md#nothex]", { op: "update" }), - ).toThrow(/working tree is read-only/); + expect(() => enforcePlanModeWrite(session, `[${sandboxPlanPath}#ABCD:1-2]`, { op: "update" })).toThrow( + /working tree is read-only/, + ); + expect(() => enforcePlanModeWrite(session, `[${sandboxPlanPath}#nothex]`, { op: "update" })).toThrow( + /working tree is read-only/, + ); }); it("still rejects absolute paths outside the local sandbox", () => { - const session = makeSession({ artifactsDir: "/tmp/agent-artifacts", cwd: "/repo", planMode }); + const session = makeSession({ artifactsDir: ARTIFACTS_DIR, cwd: REPO_ROOT, planMode }); + const workingTreePath = path.join(REPO_ROOT, "src", "foo.ts"); - expect(() => enforcePlanModeWrite(session, "/repo/src/foo.ts", { op: "update" })).toThrow( + expect(() => enforcePlanModeWrite(session, workingTreePath, { op: "update" })).toThrow( /working tree is read-only/, ); - expect(() => enforcePlanModeWrite(session, "[/repo/src/foo.ts#ABCD]", { op: "update" })).toThrow( + expect(() => enforcePlanModeWrite(session, `[${workingTreePath}#ABCD]`, { op: "update" })).toThrow( /working tree is read-only/, ); }); diff --git a/packages/coding-agent/test/tools/read-renderer.test.ts b/packages/coding-agent/test/tools/read-renderer.test.ts index 44f7acc15..0fd4d9fc7 100644 --- a/packages/coding-agent/test/tools/read-renderer.test.ts +++ b/packages/coding-agent/test/tools/read-renderer.test.ts @@ -1,4 +1,6 @@ import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test"; +import * as path from "node:path"; +import * as url from "node:url"; import { resetSettingsForTest, Settings, settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution"; import { theme as activeTheme, getThemeByName, initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; @@ -35,11 +37,12 @@ describe("readToolRenderer hyperlinks", () => { const theme = await getThemeByName("dark"); expect(theme).toBeDefined(); + const handoffPath = path.resolve("/tmp/omp-local/handoff.md"); const component = readToolRenderer.renderResult( { content: [{ type: "text", text: "second line" }], details: { - resolvedPath: "/tmp/omp-local/handoff.md", + resolvedPath: handoffPath, displayContent: { text: "second line", startLine: 2 }, contentType: "text/plain", }, @@ -52,7 +55,9 @@ describe("readToolRenderer hyperlinks", () => { const rendered = component.render(200).join("\n"); expect(rendered).toContain("local://handoff.md"); expect(rendered).toContain(":2"); - expect(extractLinkUris(rendered)).toContain("file:///tmp/omp-local/handoff.md?line=2"); + const handoffUri = new URL(url.pathToFileURL(path.resolve(handoffPath)).href); + handoffUri.searchParams.set("line", "2"); + expect(extractLinkUris(rendered)).toContain(handoffUri.href); expect(extractLinkTexts(rendered)).toContain("local://handoff.md"); expect(extractLinkTexts(rendered)).not.toContain("local://handoff.md:2"); }); @@ -62,17 +67,20 @@ describe("readToolRenderer hyperlinks", () => { const theme = await getThemeByName("dark"); expect(theme).toBeDefined(); + const examplePath = path.resolve("/tmp/omp-read/example.ts"); const component = readToolRenderer.renderCall( - { path: "/tmp/omp-read/example.ts:10-12" }, + { path: `${examplePath}:10-12` }, { expanded: false, isPartial: false }, theme!, ); const rendered = component.render(200).join("\n"); - expect(Bun.stripANSI(rendered)).toContain("/tmp/omp-read/example.ts:10-12"); - expect(extractLinkUris(rendered)).toContain("file:///tmp/omp-read/example.ts?line=10"); - expect(extractLinkTexts(rendered)).toContain("/tmp/omp-read/example.ts"); - expect(extractLinkTexts(rendered)).not.toContain("/tmp/omp-read/example.ts:10-12"); + expect(Bun.stripANSI(rendered)).toContain(`${examplePath}:10-12`); + const exampleUri = new URL(url.pathToFileURL(path.resolve(examplePath)).href); + exampleUri.searchParams.set("line", "10"); + expect(extractLinkUris(rendered)).toContain(exampleUri.href); + expect(extractLinkTexts(rendered)).toContain(examplePath); + expect(extractLinkTexts(rendered)).not.toContain(`${examplePath}:10-12`); }); it("links HTTP read result headers to the final URL", async () => { diff --git a/packages/coding-agent/test/tools/render-utils.test.ts b/packages/coding-agent/test/tools/render-utils.test.ts index f1c32d99c..021a2006b 100644 --- a/packages/coding-agent/test/tools/render-utils.test.ts +++ b/packages/coding-agent/test/tools/render-utils.test.ts @@ -12,6 +12,7 @@ import { formatExpandHint, formatParseErrors, formatScreenshot, + shortenPath, truncateDiffByHunk, } from "@oh-my-pi/pi-coding-agent/tools/render-utils"; import { getKeybindings, setKeybindings, type KeybindingsManager as TuiKeybindingsManager } from "@oh-my-pi/pi-tui"; @@ -101,7 +102,19 @@ describe("formatScreenshot", () => { ]); }); + it("uses forward slashes after a shortened Windows home", () => { + const home = String.raw`C:\Users\me`; + expect(shortenPath(String.raw`C:\Users\me\projects\demo`, home)).toBe("~/projects/demo"); + }); + + it("does not shorten paths outside the home boundary", () => { + const home = String.raw`C:\Users\me`; + const sibling = String.raw`C:\Users\me2\projects\demo`; + expect(shortenPath(sibling, home)).toBe(sibling); + }); + it("formats non-home path without tilde", () => { + const filePath = path.join(path.parse(os.homedir()).root, "omp-render-utils", "capture.png"); const resized = fakeResized({ mimeType: "image/webp", buffer: new Uint8Array(1024) }); expect( @@ -109,12 +122,12 @@ describe("formatScreenshot", () => { saveFullRes: true, savedMimeType: "image/png", savedByteLength: 2048, - dest: "/tmp/capture.png", + dest: filePath, resized, }), ).toEqual([ "Screenshot captured", - "Saved: image/png (2.00 KB) to /tmp/capture.png", + `Saved: image/png (2.00 KB) to ${filePath}`, "Model: image/webp (1.00 KB, 800x600)", ]); }); @@ -127,7 +140,7 @@ describe("formatScreenshot", () => { saveFullRes: false, savedMimeType: "image/webp", savedByteLength: 3072, - dest: "/tmp/omp-sshots-123.png", + dest: path.join(os.tmpdir(), "omp-sshots-123.png"), resized, }), ).toEqual(["Screenshot captured", "Format: image/webp (3.00 KB)", "Dimensions: 800x600"]); @@ -146,7 +159,7 @@ describe("formatScreenshot", () => { saveFullRes: false, savedMimeType: "image/webp", savedByteLength: 2048, - dest: "/tmp/shot.png", + dest: path.join(os.tmpdir(), "shot.png"), resized, }); diff --git a/packages/coding-agent/test/tools/search-renderer.test.ts b/packages/coding-agent/test/tools/search-renderer.test.ts index 5d6417957..10b3bbac6 100644 --- a/packages/coding-agent/test/tools/search-renderer.test.ts +++ b/packages/coding-agent/test/tools/search-renderer.test.ts @@ -1,4 +1,6 @@ import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test"; +import * as path from "node:path"; +import * as url from "node:url"; import { resetSettingsForTest, Settings, settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { getThemeByName } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import { searchToolRenderer } from "@oh-my-pi/pi-coding-agent/tools/search"; @@ -132,12 +134,14 @@ describe("searchToolRenderer", () => { expect(theme).toBeDefined(); const uiTheme = theme!; + const projectRoot = path.resolve("/tmp/omp-project"); + const filePath = path.join(projectRoot, "src", "file.ts"); const result = { content: [{ type: "text", text: "" }], details: { matchCount: 1, fileCount: 1, - searchPath: "/tmp/omp-project", + searchPath: projectRoot, scopePath: "src", displayContent: ["# src/", "## file.ts#abcd", "*12│const needle = true;"].join("\n"), }, @@ -147,10 +151,13 @@ describe("searchToolRenderer", () => { .renderResult(result as never, { expanded: true, isPartial: false }, uiTheme, { pattern: "needle" }) .render(240) .join("\n"); + const fileUri = url.pathToFileURL(path.resolve(filePath)).href; + const lineUri = new URL(fileUri); + lineUri.searchParams.set("line", "12"); const uris = extractLinkUris(rendered); - expect(uris).toContain("file:///tmp/omp-project/src/file.ts"); - expect(uris).toContain("file:///tmp/omp-project/src/file.ts?line=12"); + expect(uris).toContain(fileUri); + expect(uris).toContain(lineUri.href); }); it("links single-file code-frame lines to the searched file", async () => { @@ -159,12 +166,13 @@ describe("searchToolRenderer", () => { expect(theme).toBeDefined(); const uiTheme = theme!; + const filePath = path.resolve("/tmp/omp-project/file.ts"); const result = { content: [{ type: "text", text: "" }], details: { matchCount: 1, fileCount: 1, - searchPath: "/tmp/omp-project/file.ts", + searchPath: filePath, scopePath: "file.ts", displayContent: "*7│needle();", }, @@ -175,7 +183,9 @@ describe("searchToolRenderer", () => { .render(240) .join("\n"); - expect(extractLinkUris(rendered)).toContain("file:///tmp/omp-project/file.ts?line=7"); + const lineUri = new URL(url.pathToFileURL(path.resolve(filePath)).href); + lineUri.searchParams.set("line", "7"); + expect(extractLinkUris(rendered)).toContain(lineUri.href); }); it("bounds the expanded single-file view instead of dumping every match", async () => { @@ -194,12 +204,13 @@ describe("searchToolRenderer", () => { }) .join("\n"); + const filePath = path.resolve("/tmp/omp-project/renderer.ts"); const result = { content: [{ type: "text", text: "" }], details: { matchCount: clusters.length, fileCount: 1, - searchPath: "/tmp/omp-project/renderer.ts", + searchPath: filePath, scopePath: "renderer.ts", displayContent, }, diff --git a/packages/coding-agent/test/tools/tool-output-hyperlinks.test.ts b/packages/coding-agent/test/tools/tool-output-hyperlinks.test.ts index 39c1874b6..ebc0be5f0 100644 --- a/packages/coding-agent/test/tools/tool-output-hyperlinks.test.ts +++ b/packages/coding-agent/test/tools/tool-output-hyperlinks.test.ts @@ -2,6 +2,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test"; import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; +import * as url from "node:url"; import { resetSettingsForTest, Settings, settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { editToolRenderer } from "@oh-my-pi/pi-coding-agent/edit/renderer"; import { getThemeByName, initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; @@ -77,8 +78,8 @@ describe("tool output OSC 8 file:// hyperlinks", () => { .render(200) .join("\n"); - expect(extractLinkUris(textRender)).toContain(`file://${textPath}`); - expect(extractLinkUris(imgRender)).toContain(`file://${imgPath}`); + expect(extractLinkUris(textRender)).toContain(url.pathToFileURL(path.resolve(textPath)).href); + expect(extractLinkUris(imgRender)).toContain(url.pathToFileURL(path.resolve(imgPath)).href); } finally { fs.rmSync(dir, { recursive: true, force: true }); } @@ -101,7 +102,7 @@ describe("tool output OSC 8 file:// hyperlinks", () => { ) .render(200) .join("\n"); - expect(extractLinkUris(rendered)).toContain(`file://${filePath}`); + expect(extractLinkUris(rendered)).toContain(url.pathToFileURL(path.resolve(filePath)).href); } finally { fs.rmSync(dir, { recursive: true, force: true }); } @@ -113,13 +114,16 @@ describe("tool output OSC 8 file:// hyperlinks", () => { // Scoped search: scope dir (`searchPath`) is below cwd, and the grouped // display paths are cwd-relative. Resolving against searchPath would double // the `src` prefix (`/proj/src/src/...`). + const projectRoot = path.resolve("/tmp/omp-project"); + const srcRoot = path.join(projectRoot, "src"); + const interactiveModePath = path.join(srcRoot, "interactive-mode.ts"); const result = { content: [{ type: "text", text: "" }], details: { matchCount: 1, fileCount: 1, - cwd: "/tmp/omp-project", - searchPath: "/tmp/omp-project/src", + cwd: projectRoot, + searchPath: srcRoot, scopePath: "src", displayContent: ["# src/", "## interactive-mode.ts#abcd", "*12│const needle = true;"].join("\n"), }, @@ -128,15 +132,21 @@ describe("tool output OSC 8 file:// hyperlinks", () => { .renderResult(result as never, { expanded: true, isPartial: false }, theme, { pattern: "needle" }) .render(240) .join("\n"); + const interactiveModeUri = url.pathToFileURL(path.resolve(interactiveModePath)).href; + const interactiveModeLineUri = new URL(interactiveModeUri); + interactiveModeLineUri.searchParams.set("line", "12"); const uris = extractLinkUris(rendered); - expect(uris).toContain("file:///tmp/omp-project/src/interactive-mode.ts"); - expect(uris).toContain("file:///tmp/omp-project/src/interactive-mode.ts?line=12"); + expect(uris).toContain(interactiveModeUri); + expect(uris).toContain(interactiveModeLineUri.href); expect(uris.some(uri => uri.includes("/src/src/"))).toBe(false); }); it("resolves scoped ast-grep links against cwd, not the (sub)scope path", async () => { settings.override("tui.hyperlinks", "always"); const theme = (await getThemeByName("dark"))!; + const projectRoot = path.resolve("/tmp/omp-project"); + const srcRoot = path.join(projectRoot, "src"); + const interactiveModePath = path.join(srcRoot, "interactive-mode.ts"); const result = { content: [{ type: "text", text: "" }], details: { @@ -144,8 +154,8 @@ describe("tool output OSC 8 file:// hyperlinks", () => { fileCount: 1, filesSearched: 1, limitReached: false, - cwd: "/tmp/omp-project", - searchPath: "/tmp/omp-project/src", + cwd: projectRoot, + searchPath: srcRoot, scopePath: "src", displayContent: ["# src/", "## interactive-mode.ts", " *12│const needle = true;"].join("\n"), }, @@ -154,19 +164,21 @@ describe("tool output OSC 8 file:// hyperlinks", () => { .renderResult(result as never, { expanded: true, isPartial: false }, theme, { pat: "needle" }) .render(240) .join("\n"); + const interactiveModeUri = url.pathToFileURL(path.resolve(interactiveModePath)).href; const uris = extractLinkUris(rendered); - expect(uris).toContain("file:///tmp/omp-project/src/interactive-mode.ts"); + expect(uris).toContain(interactiveModeUri); expect(uris.some(uri => uri.includes("/src/src/"))).toBe(false); }); it("links the edit header to the absolute details.path even when the arg path is relative", async () => { settings.override("tui.hyperlinks", "always"); const theme = (await getThemeByName("dark"))!; + const editPath = path.resolve("/tmp/omp-project/src/a.ts"); const rendered = editToolRenderer .renderResult( { content: [{ type: "text", text: "Updated src/a.ts" }], - details: { diff: "+1|// x", op: "update", path: "/tmp/omp-project/src/a.ts" }, + details: { diff: "+1|// x", op: "update", path: editPath }, }, { expanded: false, isPartial: false, renderContext: { editMode: "hashline" } }, theme, @@ -174,9 +186,11 @@ describe("tool output OSC 8 file:// hyperlinks", () => { ) .render(200) .join("\n"); + const editUri = url.pathToFileURL(path.resolve(editPath)).href; + const rootAnchoredArgUri = url.pathToFileURL(path.resolve("/src/a.ts")).href; const uris = extractLinkUris(rendered); - expect(uris).toContain("file:///tmp/omp-project/src/a.ts"); - // A relative arg path must not leak into a root-anchored `file:///src/a.ts`. - expect(uris).not.toContain("file:///src/a.ts"); + expect(uris).toContain(editUri); + // A relative arg path must not leak into a root-anchored file URI. + expect(uris).not.toContain(rootAnchoredArgUri); }); }); diff --git a/packages/coding-agent/test/tui/hyperlink.test.ts b/packages/coding-agent/test/tui/hyperlink.test.ts index 65275e050..aba751235 100644 --- a/packages/coding-agent/test/tui/hyperlink.test.ts +++ b/packages/coding-agent/test/tui/hyperlink.test.ts @@ -1,4 +1,6 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "bun:test"; +import * as path from "node:path"; +import * as url from "node:url"; import { resetSettingsForTest, Settings, settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { LocalProtocolHandler } from "@oh-my-pi/pi-coding-agent/internal-urls/local-protocol"; import { AgentRegistry } from "@oh-my-pi/pi-coding-agent/registry/agent-registry"; @@ -112,20 +114,23 @@ describe("isHyperlinkEnabled", () => { describe("fileHyperlink", () => { it("returns plain text when hyperlinks are disabled (mode=off)", () => { setHyperlinkMode("off"); - const result = fileHyperlink("/Users/foo/bar.ts", "bar.ts"); + const filePath = path.resolve("/Users/foo/bar.ts"); + const result = fileHyperlink(filePath, "bar.ts"); expect(result).toBe("bar.ts"); }); it("wraps text in OSC 8 when hyperlinks are enabled (mode=always)", () => { setHyperlinkMode("always"); - const result = fileHyperlink("/Users/foo/bar.ts", "bar.ts"); + const filePath = path.resolve("/Users/foo/bar.ts"); + const result = fileHyperlink(filePath, "bar.ts"); expect(isHyperlinked(result)).toBe(true); expect(result).toContain("bar.ts"); }); it("builds a valid file:// URI with the absolute path", () => { setHyperlinkMode("always"); - const result = fileHyperlink("/Users/foo/bar.ts", "bar.ts"); + const filePath = path.resolve("/Users/foo/bar.ts"); + const result = fileHyperlink(filePath, "bar.ts"); const uri = extractLinkUri(result); expect(uri).toMatch(/^file:\/\//); expect(uri).toContain("bar.ts"); @@ -133,7 +138,8 @@ describe("fileHyperlink", () => { it("encodes spaces in the path", () => { setHyperlinkMode("always"); - const result = fileHyperlink("/Users/foo/my file.ts", "my file.ts"); + const filePath = path.resolve("/Users/foo/my file.ts"); + const result = fileHyperlink(filePath, "my file.ts"); const uri = extractLinkUri(result); expect(uri).toContain("%20"); expect(uri).not.toContain(" "); @@ -141,9 +147,12 @@ describe("fileHyperlink", () => { it("percent-encodes URL-reserved path bytes before appending query params", () => { setHyperlinkMode("always"); - const result = fileHyperlink("/Users/foo/a#b?c% d.ts", "a#b?c% d.ts", { line: 12 }); + const filePath = path.resolve("/Users/foo/a#b?c% d.ts"); + const result = fileHyperlink(filePath, "a#b?c% d.ts", { line: 12 }); const uri = extractLinkUri(result); - expect(uri).toBe("file:///Users/foo/a%23b%3Fc%25%20d.ts?line=12"); + const expectedUri = new URL(url.pathToFileURL(path.resolve(filePath)).href); + expectedUri.searchParams.set("line", "12"); + expect(uri).toBe(expectedUri.href); }); it("resolves relative paths before building file URIs", () => { @@ -156,7 +165,8 @@ describe("fileHyperlink", () => { it("appends line and col as query params when provided", () => { setHyperlinkMode("always"); - const result = fileHyperlink("/Users/foo/bar.ts", "bar.ts", { line: 42, col: 7 }); + const filePath = path.resolve("/Users/foo/bar.ts"); + const result = fileHyperlink(filePath, "bar.ts", { line: 42, col: 7 }); const uri = extractLinkUri(result); expect(uri).toContain("line=42"); expect(uri).toContain("col=7"); @@ -164,15 +174,17 @@ describe("fileHyperlink", () => { it("omits query params when line/col are not provided", () => { setHyperlinkMode("always"); - const result = fileHyperlink("/Users/foo/bar.ts", "bar.ts"); + const filePath = path.resolve("/Users/foo/bar.ts"); + const result = fileHyperlink(filePath, "bar.ts"); const uri = extractLinkUri(result); expect(uri).not.toContain("?"); }); it("produces a stable id for the same path", () => { setHyperlinkMode("always"); - const r1 = fileHyperlink("/Users/foo/bar.ts", "bar.ts"); - const r2 = fileHyperlink("/Users/foo/bar.ts", "different display text"); + const filePath = path.resolve("/Users/foo/bar.ts"); + const r1 = fileHyperlink(filePath, "bar.ts"); + const r2 = fileHyperlink(filePath, "different display text"); // Extract id= from params (between "id=" and next ";") const id1 = r1.match(/id=([^;]+)/)?.[1]; const id2 = r2.match(/id=([^;]+)/)?.[1]; @@ -182,8 +194,9 @@ describe("fileHyperlink", () => { it("does not double-wrap text that already contains an OSC 8 sequence", () => { setHyperlinkMode("always"); - const alreadyWrapped = `${OSC}8;id=abc123;file:///foo/bar.ts${ST}bar.ts${LINK_END}`; - const result = fileHyperlink("/Users/foo/other.ts", alreadyWrapped); + const alreadyWrappedUri = url.pathToFileURL(path.resolve("/foo/bar.ts")).href; + const alreadyWrapped = `${OSC}8;id=abc123;${alreadyWrappedUri}${ST}bar.ts${LINK_END}`; + const result = fileHyperlink(path.resolve("/Users/foo/other.ts"), alreadyWrapped); // Should return the already-wrapped text unchanged expect(result).toBe(alreadyWrapped); }); @@ -191,7 +204,8 @@ describe("fileHyperlink", () => { it("preserves ANSI color codes inside the hyperlink", () => { setHyperlinkMode("always"); const colored = "\x1b[32mbar.ts\x1b[0m"; - const result = fileHyperlink("/Users/foo/bar.ts", colored); + const filePath = path.resolve("/Users/foo/bar.ts"); + const result = fileHyperlink(filePath, colored); expect(result).toContain(colored); expect(isHyperlinked(result)).toBe(true); }); diff --git a/packages/coding-agent/test/utils/filter-user-extensions.ts b/packages/coding-agent/test/utils/filter-user-extensions.ts index a786aa4bf..f882837a7 100644 --- a/packages/coding-agent/test/utils/filter-user-extensions.ts +++ b/packages/coding-agent/test/utils/filter-user-extensions.ts @@ -31,7 +31,11 @@ function lexicalIsWithin(root: string, candidate: string): boolean { return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); } -export function filterUserScoped(items: T[]): T[] { +export function filterUserScoped(items: T[], keepRoots?: string | string[]): T[] { + if (keepRoots) { + const roots = Array.isArray(keepRoots) ? keepRoots : [keepRoots]; + return items.filter(it => roots.some(root => lexicalIsWithin(root, it.path))); + } const prefixes = [getConfigRootDir(), getAgentDir(), getPluginsDir()]; return items.filter(it => !prefixes.some(prefix => lexicalIsWithin(prefix, it.path))); } diff --git a/packages/coding-agent/test/utils/git-clone.test.ts b/packages/coding-agent/test/utils/git-clone.test.ts index 1bc5d102c..0e2b71326 100644 --- a/packages/coding-agent/test/utils/git-clone.test.ts +++ b/packages/coding-agent/test/utils/git-clone.test.ts @@ -2,6 +2,7 @@ import { afterAll, beforeAll, describe, expect, test } from "bun:test"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; +import * as url from "node:url"; import * as git from "@oh-my-pi/pi-coding-agent/utils/git"; @@ -44,7 +45,7 @@ describe("git.clone with options.sha", () => { // `file://` is required: local-path clones ignore `--depth`, which would // mask the bug. See git-clone(1) "GIT URLS" / "LOCAL PROTOCOL". - upstreamUrl = `file://${upstream}`; + upstreamUrl = url.pathToFileURL(upstream).href; gitRun(upstream, ["init", "-q", "-b", "main"]); gitRun(upstream, ["commit", "-q", "--allow-empty", "-m", "first"]); diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index db821153b..505f46e7d 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -2,12 +2,15 @@ ## [Unreleased] +### Changed + +- Expanded the `TempDir` Windows retry window from 4×10ms to 40×25ms (1s total) to accommodate SQLite WAL/SHM file handle release delays + ## [16.0.11] - 2026-06-19 ### Removed - Removed `getIndentation`, `setDefaultTabWidth`, and `getDefaultTabWidth` helpers - ## [16.0.8] - 2026-06-18 ### Changed diff --git a/packages/utils/src/temp.ts b/packages/utils/src/temp.ts index 10d67c24e..6a8aa557a 100644 --- a/packages/utils/src/temp.ts +++ b/packages/utils/src/temp.ts @@ -78,8 +78,8 @@ function normalizePrefix(prefix?: string): string { } const kRemoveOptions = { recursive: true, force: true } as const; -const kRemoveRetries = 4; -const kRemoveRetryDelayMs = 10; +const kRemoveRetries = 40; +const kRemoveRetryDelayMs = 25; const kRetryableRemoveErrorCodes = new Set(["EBUSY", "EPERM", "ENOTEMPTY"]); const kSleepBuffer = new Int32Array(new SharedArrayBuffer(4));