refactor(cli): remove legacy Snapshot facade

This commit is contained in:
marius-kilocode
2026-05-28 15:47:06 +02:00
parent 600d47a6d2
commit 3d3d68f37d
4 changed files with 106 additions and 93 deletions
@@ -1,12 +1,13 @@
import { test, expect } from "bun:test"
import { $ } from "bun"
import { Effect } from "effect"
import { Snapshot } from "../../src/snapshot"
import { WithInstance } from "../../src/project/with-instance"
import { Filesystem } from "../../src/util/filesystem"
import * as Log from "@opencode-ai/core/util/log"
import { tmpdir } from "../fixture/fixture"
Log.init({ print: false })
void Log.init({ print: false })
async function bootstrap() {
return tmpdir({
@@ -20,26 +21,33 @@ async function bootstrap() {
})
}
function run<A>(body: (snapshot: Snapshot.Interface) => Effect.Effect<A>) {
return Effect.runPromise(Snapshot.Service.use(body).pipe(Effect.provide(Snapshot.defaultLayer)))
}
test("diffFull returns cached result for same hash pair", async () => {
await using tmp = await bootstrap()
await WithInstance.provide({
directory: tmp.path,
fn: async () => {
const before = await Snapshot.track()
expect(before).toBeTruthy()
fn: () =>
run((snapshot) =>
Effect.gen(function* () {
const before = yield* snapshot.track()
expect(before).toBeTruthy()
await Filesystem.write(`${tmp.path}/a.txt`, "MODIFIED")
const after = await Snapshot.track()
expect(after).toBeTruthy()
expect(after).not.toBe(before)
yield* Effect.promise(() => Filesystem.write(`${tmp.path}/a.txt`, "MODIFIED"))
const after = yield* snapshot.track()
expect(after).toBeTruthy()
expect(after).not.toBe(before)
const first = await Snapshot.diffFull(before!, after!)
const second = await Snapshot.diffFull(before!, after!)
const first = yield* snapshot.diffFull(before!, after!)
const second = yield* snapshot.diffFull(before!, after!)
// Should be the exact same array reference (cached)
expect(second).toBe(first)
expect(first.length).toBeGreaterThan(0)
},
// Should be the exact same array reference (cached)
expect(second).toBe(first)
expect(first.length).toBeGreaterThan(0)
}),
),
})
})
@@ -47,13 +55,16 @@ test("diffFull returns empty array when from === to", async () => {
await using tmp = await bootstrap()
await WithInstance.provide({
directory: tmp.path,
fn: async () => {
const hash = await Snapshot.track()
expect(hash).toBeTruthy()
fn: () =>
run((snapshot) =>
Effect.gen(function* () {
const hash = yield* snapshot.track()
expect(hash).toBeTruthy()
const result = await Snapshot.diffFull(hash!, hash!)
expect(result).toEqual([])
},
const result = yield* snapshot.diffFull(hash!, hash!)
expect(result).toEqual([])
}),
),
})
})
@@ -61,24 +72,30 @@ test("diffFull concurrent calls for same pair share one result", async () => {
await using tmp = await bootstrap()
await WithInstance.provide({
directory: tmp.path,
fn: async () => {
const before = await Snapshot.track()
expect(before).toBeTruthy()
fn: () =>
run((snapshot) =>
Effect.gen(function* () {
const before = yield* snapshot.track()
expect(before).toBeTruthy()
await Filesystem.write(`${tmp.path}/a.txt`, "CONCURRENT")
const after = await Snapshot.track()
expect(after).toBeTruthy()
yield* Effect.promise(() => Filesystem.write(`${tmp.path}/a.txt`, "CONCURRENT"))
const after = yield* snapshot.track()
expect(after).toBeTruthy()
// Fire multiple concurrent calls they should all resolve to the same object
const results = await Promise.all([
Snapshot.diffFull(before!, after!),
Snapshot.diffFull(before!, after!),
Snapshot.diffFull(before!, after!),
])
// Fire multiple concurrent calls, they should all resolve to the same object.
const results = yield* Effect.all(
[
snapshot.diffFull(before!, after!),
snapshot.diffFull(before!, after!),
snapshot.diffFull(before!, after!),
],
{ concurrency: "unbounded" },
)
expect(results[0]).toBe(results[1])
expect(results[1]).toBe(results[2])
expect(results[0].length).toBeGreaterThan(0)
},
expect(results[0]).toBe(results[1])
expect(results[1]).toBe(results[2])
expect(results[0].length).toBeGreaterThan(0)
}),
),
})
})
@@ -14,6 +14,7 @@
import { test, expect, afterEach, mock } from "bun:test"
import { $ } from "bun"
import { Effect, Fiber } from "effect"
import { WithInstance } from "../../src/project/with-instance"
import { Server } from "../../src/server/server"
import { Session } from "../../src/session/session"
@@ -22,7 +23,11 @@ import { Filesystem } from "../../src/util/filesystem"
import * as Log from "@opencode-ai/core/util/log"
import { disposeAllInstances, tmpdir } from "../fixture/fixture"
Log.init({ print: false })
void Log.init({ print: false })
function run<A>(body: (snapshot: Snapshot.Interface) => Effect.Effect<A>) {
return Effect.runPromise(Snapshot.Service.use(body).pipe(Effect.provide(Snapshot.defaultLayer)))
}
afterEach(async () => {
mock.restore()
@@ -47,55 +52,60 @@ test("pathological diffFull workload finishes quickly and does not block abort",
await WithInstance.provide({
directory: tmp.path,
fn: async () => {
const session = await Session.create({})
fn: () =>
run((snapshot) =>
Effect.gen(function* () {
const session = yield* Effect.promise(() => Session.create({}))
const before = await Snapshot.track()
expect(before).toBeTruthy()
const before = yield* snapshot.track()
expect(before).toBeTruthy()
await Filesystem.write(`${tmp.path}/fat.json`, v2)
const after = await Snapshot.track()
expect(after).toBeTruthy()
yield* Effect.promise(() => Filesystem.write(`${tmp.path}/fat.json`, v2))
const after = yield* snapshot.track()
expect(after).toBeTruthy()
// Kick off a diffFull that exercises the freeze path.
const diffPromise = Snapshot.diffFull(before!, after!)
// Kick off a diffFull that exercises the freeze path.
const diff = yield* snapshot.diffFull(before!, after!).pipe(Effect.forkChild({ startImmediately: true }))
// Concurrently keep a tick counter running. If the event loop blocks we
// will see this count fall behind wall-clock elapsed.
let ticks = 0
const start = Date.now()
const timer = setInterval(() => {
ticks++
}, 25)
// Concurrently keep a tick counter running. If the event loop blocks we
// will see this count fall behind wall-clock elapsed.
let ticks = 0
const start = Date.now()
const timer = setInterval(() => {
ticks++
}, 25)
// Fire an abort request against the Hono app in the middle of the diff.
const app = Server.Default().app
const abortStart = Date.now()
const res = await app.request(`/session/${session.id}/abort`, { method: "POST" })
const abortLatency = Date.now() - abortStart
expect(res.status).toBe(200)
// The abort endpoint must respond well under a second even under load.
expect(abortLatency).toBeLessThan(2000)
// Fire an abort request against the Hono app in the middle of the diff.
const app = Server.Default().app
const abortStart = Date.now()
const res = yield* Effect.promise(() =>
Promise.resolve(app.request(`/session/${session.id}/abort`, { method: "POST" })),
)
const abortLatency = Date.now() - abortStart
expect(res.status).toBe(200)
// The abort endpoint must respond well under a second even under load.
expect(abortLatency).toBeLessThan(2000)
const diffs = await diffPromise
clearInterval(timer)
const total = Date.now() - start
const diffs = yield* Fiber.join(diff)
clearInterval(timer)
const total = Date.now() - start
// The freeze workload must finish in bounded time. Five seconds is
// generous even for a slow CI box; without the fix this hangs.
expect(total).toBeLessThan(5000)
// And we must have ticked at least a few times during the work proves
// the event loop stayed responsive (ESC would actually arrive).
expect(ticks).toBeGreaterThan(0)
// The freeze workload must finish in bounded time. Five seconds is
// generous even for a slow CI box; without the fix this hangs.
expect(total).toBeLessThan(5000)
// And we must have ticked at least a few times during the work, proving
// the event loop stayed responsive (ESC would actually arrive).
expect(ticks).toBeGreaterThan(0)
// With git-based diff the patch is a real unified diff, not empty.
const hit = diffs.find((d) => d.file === "fat.json")
expect(hit).toBeDefined()
expect(hit!.patch).toMatch(/^diff --git /m)
expect(hit!.patch).toContain("-v1_line_0")
expect(hit!.patch).toContain("+v2_line_0")
expect(hit!.additions).toBeGreaterThan(0)
expect(hit!.deletions).toBeGreaterThan(0)
},
// With git-based diff the patch is a real unified diff, not empty.
const hit = diffs.find((d) => d.file === "fat.json")
expect(hit).toBeDefined()
expect(hit!.patch).toMatch(/^diff --git /m)
expect(hit!.patch).toContain("-v1_line_0")
expect(hit!.patch).toContain("+v2_line_0")
expect(hit!.additions).toBeGreaterThan(0)
expect(hit!.deletions).toBeGreaterThan(0)
}),
),
})
})