mirror of
https://github.com/Kilo-Org/kilocode.git
synced 2026-10-30 19:32:39 +08:00
test(storage): make the storage root injectable and fail closed on disk databases
Closes #12997. Part of #12986. - Storage.layerFromDir(dir) roots the service at an explicit directory; the default layer still resolves Global.Path.data/storage lazily, so production composition (app-runtime) is unchanged. - storage.test.ts injects a tmp root directly instead of remapping six FSUtil methods under Global.Path.data - the remap hack and its keep-in-sync burden are gone. - Both test preloads now assert the resolved database path is ":memory:" after all preload imports. Previously a dropped env write or import-order regression silently pointed unit tests at the real ~/.local/share/kilo database (locking errors, Windows EBUSY teardown crashes); now the run fails immediately with a clear message.
This commit is contained in:
Yury Zialionka
@@ -3,3 +3,13 @@ import path from "path"
|
||||
process.env.KILO_DB = ":memory:"
|
||||
process.env.KILO_MODELS_PATH = path.join(import.meta.dir, "plugin", "fixtures", "models-dev.json")
|
||||
process.env.KILO_DISABLE_MODELS_FETCH = "true"
|
||||
|
||||
// kilocode_change start - fail closed: core unit tests do not redirect XDG dirs, so KILO_DB
|
||||
// is the only thing keeping them off the real ~/.local/share/kilo database. Verify the
|
||||
// resolved path (env is read at flag import time, so this must stay after the env writes).
|
||||
const { Database } = await import("../src/database/database")
|
||||
const resolved = Database.path()
|
||||
if (resolved !== ":memory:") {
|
||||
throw new Error(`unit test preload: database path must resolve to ":memory:", got "${resolved}"`)
|
||||
}
|
||||
// kilocode_change end
|
||||
|
||||
@@ -210,117 +210,130 @@ const MIGRATIONS: Migration[] = [
|
||||
}),
|
||||
]
|
||||
|
||||
const layer = Layer.effect(
|
||||
Service,
|
||||
Effect.gen(function* () {
|
||||
const fs = yield* FSUtil.Service
|
||||
const git = yield* Git.Service
|
||||
const locks = yield* RcMap.make({
|
||||
lookup: () => TxReentrantLock.make(),
|
||||
idleTimeToLive: 0,
|
||||
})
|
||||
const state = yield* Effect.cached(
|
||||
Effect.gen(function* () {
|
||||
const dir = path.join(Global.Path.data, "storage")
|
||||
const marker = path.join(dir, "migration")
|
||||
const migration = yield* fs.readFileString(marker).pipe(
|
||||
Effect.map(parseMigration),
|
||||
Effect.catchIf(missing, () => Effect.succeed(0)),
|
||||
Effect.orElseSucceed(() => 0),
|
||||
)
|
||||
for (let i = migration; i < MIGRATIONS.length; i++) {
|
||||
yield* Effect.logInfo("running migration", { index: i })
|
||||
const step = MIGRATIONS[i]!
|
||||
const exit = yield* Effect.exit(step(dir, fs, git))
|
||||
if (Exit.isFailure(exit)) {
|
||||
yield* Effect.logError("failed to run migration", { index: i, cause: exit.cause })
|
||||
break
|
||||
}
|
||||
yield* fs.writeWithDirs(marker, String(i + 1))
|
||||
}
|
||||
return { dir }
|
||||
}),
|
||||
)
|
||||
|
||||
const fail = (target: string): Effect.Effect<never, NotFoundError> =>
|
||||
Effect.fail(new NotFoundError({ message: `Resource not found: ${target}` }))
|
||||
|
||||
const wrap = <A>(target: string, body: Effect.Effect<A, FSUtil.Error>) =>
|
||||
body.pipe(Effect.catchIf(missing, () => fail(target)))
|
||||
|
||||
const writeJson = Effect.fnUntraced(function* (target: string, content: unknown) {
|
||||
yield* fs.writeWithDirs(target, JSON.stringify(content, null, 2))
|
||||
})
|
||||
|
||||
const withResolved = <A, E>(
|
||||
key: string[],
|
||||
fn: (target: string, rw: TxReentrantLock.TxReentrantLock) => Effect.Effect<A, E>,
|
||||
): Effect.Effect<A, E | FSUtil.Error> =>
|
||||
Effect.scoped(
|
||||
// kilocode_change start - the storage root is injectable so tests can point Storage at a
|
||||
// temporary directory instead of remapping FSUtil paths under Global.Path.data. The
|
||||
// default (production) root is unchanged and resolved lazily on first use. Note for
|
||||
// callers of layerFromDir: migration 1 walks `../project` relative to this directory,
|
||||
// so inject `<root>/storage` when legacy layouts must be reachable from the parent.
|
||||
const make = (root?: string) =>
|
||||
Layer.effect(
|
||||
Service,
|
||||
Effect.gen(function* () {
|
||||
const fs = yield* FSUtil.Service
|
||||
const git = yield* Git.Service
|
||||
const locks = yield* RcMap.make({
|
||||
lookup: () => TxReentrantLock.make(),
|
||||
idleTimeToLive: 0,
|
||||
})
|
||||
const state = yield* Effect.cached(
|
||||
Effect.gen(function* () {
|
||||
const target = file((yield* state).dir, key)
|
||||
return yield* fn(target, yield* RcMap.get(locks, target))
|
||||
const dir = root ?? path.join(Global.Path.data, "storage")
|
||||
const marker = path.join(dir, "migration")
|
||||
const migration = yield* fs.readFileString(marker).pipe(
|
||||
Effect.map(parseMigration),
|
||||
Effect.catchIf(missing, () => Effect.succeed(0)),
|
||||
Effect.orElseSucceed(() => 0),
|
||||
)
|
||||
for (let i = migration; i < MIGRATIONS.length; i++) {
|
||||
yield* Effect.logInfo("running migration", { index: i })
|
||||
const step = MIGRATIONS[i]!
|
||||
const exit = yield* Effect.exit(step(dir, fs, git))
|
||||
if (Exit.isFailure(exit)) {
|
||||
yield* Effect.logError("failed to run migration", { index: i, cause: exit.cause })
|
||||
break
|
||||
}
|
||||
yield* fs.writeWithDirs(marker, String(i + 1))
|
||||
}
|
||||
return { dir }
|
||||
}),
|
||||
)
|
||||
|
||||
const remove: Interface["remove"] = Effect.fn("Storage.remove")(function* (key: string[]) {
|
||||
yield* withResolved(key, (target, rw) =>
|
||||
TxReentrantLock.withWriteLock(rw, fs.remove(target).pipe(Effect.catchIf(missing, () => Effect.void))),
|
||||
)
|
||||
})
|
||||
const fail = (target: string): Effect.Effect<never, NotFoundError> =>
|
||||
Effect.fail(new NotFoundError({ message: `Resource not found: ${target}` }))
|
||||
|
||||
const read: Interface["read"] = <T>(key: string[]) =>
|
||||
Effect.gen(function* () {
|
||||
const value = yield* withResolved(key, (target, rw) =>
|
||||
TxReentrantLock.withReadLock(rw, wrap(target, fs.readJson(target))),
|
||||
const wrap = <A>(target: string, body: Effect.Effect<A, FSUtil.Error>) =>
|
||||
body.pipe(Effect.catchIf(missing, () => fail(target)))
|
||||
|
||||
const writeJson = Effect.fnUntraced(function* (target: string, content: unknown) {
|
||||
yield* fs.writeWithDirs(target, JSON.stringify(content, null, 2))
|
||||
})
|
||||
|
||||
const withResolved = <A, E>(
|
||||
key: string[],
|
||||
fn: (target: string, rw: TxReentrantLock.TxReentrantLock) => Effect.Effect<A, E>,
|
||||
): Effect.Effect<A, E | FSUtil.Error> =>
|
||||
Effect.scoped(
|
||||
Effect.gen(function* () {
|
||||
const target = file((yield* state).dir, key)
|
||||
return yield* fn(target, yield* RcMap.get(locks, target))
|
||||
}),
|
||||
)
|
||||
return value as T
|
||||
})
|
||||
|
||||
const update: Interface["update"] = <T>(key: string[], fn: (draft: T) => void) =>
|
||||
Effect.gen(function* () {
|
||||
const value = yield* withResolved(key, (target, rw) =>
|
||||
TxReentrantLock.withWriteLock(
|
||||
rw,
|
||||
Effect.gen(function* () {
|
||||
const content = yield* wrap(target, fs.readJson(target))
|
||||
fn(content as T)
|
||||
yield* writeJson(target, content)
|
||||
return content
|
||||
}),
|
||||
),
|
||||
const remove: Interface["remove"] = Effect.fn("Storage.remove")(function* (key: string[]) {
|
||||
yield* withResolved(key, (target, rw) =>
|
||||
TxReentrantLock.withWriteLock(rw, fs.remove(target).pipe(Effect.catchIf(missing, () => Effect.void))),
|
||||
)
|
||||
return value as T
|
||||
})
|
||||
|
||||
const write: Interface["write"] = (key: string[], content: unknown) =>
|
||||
Effect.gen(function* () {
|
||||
yield* withResolved(key, (target, rw) => TxReentrantLock.withWriteLock(rw, writeJson(target, content)))
|
||||
})
|
||||
|
||||
const list: Interface["list"] = Effect.fn("Storage.list")(function* (prefix: string[]) {
|
||||
const dir = (yield* state).dir
|
||||
const cwd = path.join(dir, ...prefix)
|
||||
const result = yield* fs
|
||||
.glob("**/*", {
|
||||
cwd,
|
||||
include: "file",
|
||||
const read: Interface["read"] = <T>(key: string[]) =>
|
||||
Effect.gen(function* () {
|
||||
const value = yield* withResolved(key, (target, rw) =>
|
||||
TxReentrantLock.withReadLock(rw, wrap(target, fs.readJson(target))),
|
||||
)
|
||||
return value as T
|
||||
})
|
||||
.pipe(Effect.catch(() => Effect.succeed<string[]>([])))
|
||||
return result
|
||||
.map((x) => [...prefix, ...x.slice(0, -5).split(path.sep)])
|
||||
.toSorted((a, b) => a.join("/").localeCompare(b.join("/")))
|
||||
})
|
||||
|
||||
return Service.of({
|
||||
remove,
|
||||
read,
|
||||
update,
|
||||
write,
|
||||
list,
|
||||
})
|
||||
}),
|
||||
)
|
||||
const update: Interface["update"] = <T>(key: string[], fn: (draft: T) => void) =>
|
||||
Effect.gen(function* () {
|
||||
const value = yield* withResolved(key, (target, rw) =>
|
||||
TxReentrantLock.withWriteLock(
|
||||
rw,
|
||||
Effect.gen(function* () {
|
||||
const content = yield* wrap(target, fs.readJson(target))
|
||||
fn(content as T)
|
||||
yield* writeJson(target, content)
|
||||
return content
|
||||
}),
|
||||
),
|
||||
)
|
||||
return value as T
|
||||
})
|
||||
|
||||
const write: Interface["write"] = (key: string[], content: unknown) =>
|
||||
Effect.gen(function* () {
|
||||
yield* withResolved(key, (target, rw) => TxReentrantLock.withWriteLock(rw, writeJson(target, content)))
|
||||
})
|
||||
|
||||
const list: Interface["list"] = Effect.fn("Storage.list")(function* (prefix: string[]) {
|
||||
const dir = (yield* state).dir
|
||||
const cwd = path.join(dir, ...prefix)
|
||||
const result = yield* fs
|
||||
.glob("**/*", {
|
||||
cwd,
|
||||
include: "file",
|
||||
})
|
||||
.pipe(Effect.catch(() => Effect.succeed<string[]>([])))
|
||||
return result
|
||||
.map((x) => [...prefix, ...x.slice(0, -5).split(path.sep)])
|
||||
.toSorted((a, b) => a.join("/").localeCompare(b.join("/")))
|
||||
})
|
||||
|
||||
return Service.of({
|
||||
remove,
|
||||
read,
|
||||
update,
|
||||
write,
|
||||
list,
|
||||
})
|
||||
}),
|
||||
)
|
||||
|
||||
const layer = make()
|
||||
|
||||
// kilocode_change start
|
||||
/** Storage rooted at an explicit directory — for tests and multi-instance isolation. */
|
||||
export const layerFromDir = (dir: string) => make(dir)
|
||||
// kilocode_change end
|
||||
|
||||
export const node = LayerNode.make({ service: Service, layer: layer, deps: [FSUtil.node, Git.node] })
|
||||
|
||||
|
||||
@@ -96,3 +96,20 @@ const { installMemoryRuntime } = await import("../src/kilocode/memory/runtime")
|
||||
|
||||
initProjectors()
|
||||
installMemoryRuntime() // kilocode_change
|
||||
|
||||
// kilocode_change start - fail closed: unit tests must never open a disk database. Both DB
|
||||
// path resolvers (core Database.path and the v1 client in src/storage/db.ts) honor KILO_DB
|
||||
// verbatim when it is ":memory:", so asserting the resolved core path after all preload
|
||||
// imports catches env mutations, import-order regressions, and channel/absolute fallbacks
|
||||
// that would silently point tests at ~/.local/share/kilo/kilo.db.
|
||||
if (process.env["KILO_DB"] !== ":memory:") {
|
||||
throw new Error(`unit test preload: KILO_DB must be ":memory:", got "${process.env["KILO_DB"]}"`)
|
||||
}
|
||||
{
|
||||
const { Database } = await import("@opencode-ai/core/database/database")
|
||||
const resolved = Database.path()
|
||||
if (resolved !== ":memory:") {
|
||||
throw new Error(`unit test preload: database path must resolve to ":memory:", got "${resolved}"`)
|
||||
}
|
||||
}
|
||||
// kilocode_change end
|
||||
|
||||
@@ -24,41 +24,17 @@ const scope = Effect.fnUntraced(function* () {
|
||||
return { root, svc }
|
||||
})
|
||||
|
||||
// remap(root) rewrites any path under Global.Path.data to live under `root` instead.
|
||||
// Used by remappedFs to build an FSUtil that Storage thinks is the real global
|
||||
// data dir but actually targets a tmp dir — letting migration tests stage legacy layouts.
|
||||
// NOTE: only the 6 methods below are intercepted. If Storage starts using a different
|
||||
// FSUtil method that touches Global.Path.data, add it here.
|
||||
function remap(root: string, file: string) {
|
||||
if (file === Global.Path.data) return root
|
||||
if (file.startsWith(Global.Path.data + path.sep)) return path.join(root, path.relative(Global.Path.data, file))
|
||||
return file
|
||||
}
|
||||
|
||||
function remappedFs(root: string) {
|
||||
return Layer.effect(
|
||||
FSUtil.Service,
|
||||
Effect.gen(function* () {
|
||||
const fs = yield* FSUtil.Service
|
||||
return FSUtil.Service.of({
|
||||
...fs,
|
||||
isDir: (file) => fs.isDir(remap(root, file)),
|
||||
readJson: (file) => fs.readJson(remap(root, file)),
|
||||
writeWithDirs: (file, content, mode) => fs.writeWithDirs(remap(root, file), content, mode),
|
||||
readFileString: (file) => fs.readFileString(remap(root, file)),
|
||||
remove: (file) => fs.remove(remap(root, file)),
|
||||
glob: (pattern, options) =>
|
||||
fs.glob(pattern, options?.cwd ? { ...options, cwd: remap(root, options.cwd) } : options),
|
||||
})
|
||||
}),
|
||||
).pipe(Layer.provide(LayerNode.compile(FSUtil.node)))
|
||||
}
|
||||
|
||||
// Layer.fresh forces a new Storage instance — without it, Effect's in-test layer cache
|
||||
// returns the outer testEffect's Storage (which uses the real FSUtil), not a new
|
||||
// one built on top of remappedFs.
|
||||
const remappedStorage = (root: string) =>
|
||||
Layer.fresh(LayerNode.compile(Storage.node, [[FSUtil.node, remappedFs(root)]]))
|
||||
// Storage rooted at `<root>/storage` via the injectable layer — migration tests stage
|
||||
// legacy layouts (e.g. `<root>/project`) as siblings, matching migration 1's `../project`
|
||||
// walk. Layer.fresh forces a new Storage instance — without it, Effect's in-test layer
|
||||
// cache returns the outer testEffect's Storage (rooted at the real data dir), not a new
|
||||
// one rooted at the tmp dir.
|
||||
const injectedStorage = (root: string) =>
|
||||
Layer.fresh(
|
||||
Storage.layerFromDir(path.join(root, "storage")).pipe(
|
||||
Layer.provide(LayerNode.compile(LayerNode.group([FSUtil.node, Git.node]))),
|
||||
),
|
||||
)
|
||||
|
||||
describe("Storage", () => {
|
||||
it.live("round-trips JSON content", () =>
|
||||
@@ -228,7 +204,7 @@ describe("Storage", () => {
|
||||
title: "legacy",
|
||||
summary: { additions: 5, deletions: 5 },
|
||||
})
|
||||
}).pipe(Effect.provide(remappedStorage(tmp)))
|
||||
}).pipe(Effect.provide(injectedStorage(tmp)))
|
||||
|
||||
expect(yield* fs.readFileString(path.join(storage, "migration"))).toBe("2")
|
||||
}),
|
||||
@@ -270,7 +246,7 @@ describe("Storage", () => {
|
||||
role: "user",
|
||||
text: "hello",
|
||||
})
|
||||
}).pipe(Effect.provide(remappedStorage(tmp)))
|
||||
}).pipe(Effect.provide(injectedStorage(tmp)))
|
||||
|
||||
expect(yield* fs.readFileString(path.join(storage, "migration"))).toBe("2")
|
||||
}),
|
||||
@@ -288,7 +264,7 @@ describe("Storage", () => {
|
||||
yield* Effect.gen(function* () {
|
||||
const svc = yield* Storage.Service
|
||||
expect(yield* svc.list(["project"])).toEqual([])
|
||||
}).pipe(Effect.provide(remappedStorage(tmp)))
|
||||
}).pipe(Effect.provide(injectedStorage(tmp)))
|
||||
|
||||
const exit = yield* fs.access(path.join(storage, "migration")).pipe(Effect.exit)
|
||||
expect(Exit.isFailure(exit)).toBe(true)
|
||||
|
||||
Reference in New Issue
Block a user