refactor(truncation): effectify TruncateService, delete Scheduler (#17957)

This commit is contained in:
Kit Langton
2026-03-17 21:59:54 -04:00
committed by GitHub
parent 4b4dd2b882
commit 5dfe86dcb1
40 changed files with 405 additions and 482 deletions

View File

@@ -4,7 +4,7 @@ import { Effect, Layer, Option } from "effect"
import { AccountRepo } from "../../src/account/repo"
import { AccessToken, AccountID, OrgID, RefreshToken } from "../../src/account/schema"
import { Database } from "../../src/storage/db"
import { testEffect } from "../fixture/effect"
import { testEffect } from "../lib/effect"
const truncate = Layer.effectDiscard(
Effect.sync(() => {
@@ -16,24 +16,21 @@ const truncate = Layer.effectDiscard(
const it = testEffect(Layer.merge(AccountRepo.layer, truncate))
it.effect(
"list returns empty when no accounts exist",
it.effect("list returns empty when no accounts exist", () =>
Effect.gen(function* () {
const accounts = yield* AccountRepo.use((r) => r.list())
expect(accounts).toEqual([])
}),
)
it.effect(
"active returns none when no accounts exist",
it.effect("active returns none when no accounts exist", () =>
Effect.gen(function* () {
const active = yield* AccountRepo.use((r) => r.active())
expect(Option.isNone(active)).toBe(true)
}),
)
it.effect(
"persistAccount inserts and getRow retrieves",
it.effect("persistAccount inserts and getRow retrieves", () =>
Effect.gen(function* () {
const id = AccountID.make("user-1")
yield* AccountRepo.use((r) =>
@@ -59,8 +56,7 @@ it.effect(
}),
)
it.effect(
"persistAccount sets the active account and org",
it.effect("persistAccount sets the active account and org", () =>
Effect.gen(function* () {
const id1 = AccountID.make("user-1")
const id2 = AccountID.make("user-2")
@@ -97,8 +93,7 @@ it.effect(
}),
)
it.effect(
"list returns all accounts",
it.effect("list returns all accounts", () =>
Effect.gen(function* () {
const id1 = AccountID.make("user-1")
const id2 = AccountID.make("user-2")
@@ -133,8 +128,7 @@ it.effect(
}),
)
it.effect(
"remove deletes an account",
it.effect("remove deletes an account", () =>
Effect.gen(function* () {
const id = AccountID.make("user-1")
@@ -157,8 +151,7 @@ it.effect(
}),
)
it.effect(
"use stores the selected org and marks the account active",
it.effect("use stores the selected org and marks the account active", () =>
Effect.gen(function* () {
const id1 = AccountID.make("user-1")
const id2 = AccountID.make("user-2")
@@ -198,8 +191,7 @@ it.effect(
}),
)
it.effect(
"persistToken updates token fields",
it.effect("persistToken updates token fields", () =>
Effect.gen(function* () {
const id = AccountID.make("user-1")
@@ -233,8 +225,7 @@ it.effect(
}),
)
it.effect(
"persistToken with no expiry sets token_expiry to null",
it.effect("persistToken with no expiry sets token_expiry to null", () =>
Effect.gen(function* () {
const id = AccountID.make("user-1")
@@ -264,8 +255,7 @@ it.effect(
}),
)
it.effect(
"persistAccount upserts on conflict",
it.effect("persistAccount upserts on conflict", () =>
Effect.gen(function* () {
const id = AccountID.make("user-1")
@@ -305,8 +295,7 @@ it.effect(
}),
)
it.effect(
"remove clears active state when deleting the active account",
it.effect("remove clears active state when deleting the active account", () =>
Effect.gen(function* () {
const id = AccountID.make("user-1")
@@ -329,8 +318,7 @@ it.effect(
}),
)
it.effect(
"getRow returns none for nonexistent account",
it.effect("getRow returns none for nonexistent account", () =>
Effect.gen(function* () {
const row = yield* AccountRepo.use((r) => r.getRow(AccountID.make("nope")))
expect(Option.isNone(row)).toBe(true)

View File

@@ -1,12 +1,12 @@
import { expect } from "bun:test"
import { Duration, Effect, Layer, Option, Ref, Schema } from "effect"
import { Duration, Effect, Layer, Option, Schema } from "effect"
import { HttpClient, HttpClientResponse } from "effect/unstable/http"
import { AccountRepo } from "../../src/account/repo"
import { AccountService } from "../../src/account/service"
import { AccessToken, AccountID, DeviceCode, Login, Org, OrgID, RefreshToken, UserCode } from "../../src/account/schema"
import { Database } from "../../src/storage/db"
import { testEffect } from "../fixture/effect"
import { testEffect } from "../lib/effect"
const truncate = Layer.effectDiscard(
Effect.sync(() => {
@@ -34,8 +34,7 @@ const encodeOrg = Schema.encodeSync(Org)
const org = (id: string, name: string) => encodeOrg(new Org({ id: OrgID.make(id), name }))
it.effect(
"orgsByAccount groups orgs per account",
it.effect("orgsByAccount groups orgs per account", () =>
Effect.gen(function* () {
yield* AccountRepo.use((r) =>
r.persistAccount({
@@ -61,10 +60,10 @@ it.effect(
}),
)
const seen = yield* Ref.make<string[]>([])
const seen: Array<string> = []
const client = HttpClient.make((req) =>
Effect.gen(function* () {
yield* Ref.update(seen, (xs) => [...xs, `${req.method} ${req.url}`])
seen.push(`${req.method} ${req.url}`)
if (req.url === "https://one.example.com/api/orgs") {
return json(req, [org("org-1", "One")])
@@ -84,15 +83,14 @@ it.effect(
[AccountID.make("user-1"), [OrgID.make("org-1")]],
[AccountID.make("user-2"), [OrgID.make("org-2"), OrgID.make("org-3")]],
])
expect(yield* Ref.get(seen)).toEqual([
expect(seen).toEqual([
"GET https://one.example.com/api/orgs",
"GET https://two.example.com/api/orgs",
])
}),
)
it.effect(
"token refresh persists the new token",
it.effect("token refresh persists the new token", () =>
Effect.gen(function* () {
const id = AccountID.make("user-1")
@@ -133,8 +131,7 @@ it.effect(
}),
)
it.effect(
"config sends the selected org header",
it.effect("config sends the selected org header", () =>
Effect.gen(function* () {
const id = AccountID.make("user-1")
@@ -150,13 +147,11 @@ it.effect(
}),
)
const seen = yield* Ref.make<{ auth?: string; org?: string }>({})
const seen: { auth?: string; org?: string } = {}
const client = HttpClient.make((req) =>
Effect.gen(function* () {
yield* Ref.set(seen, {
auth: req.headers.authorization,
org: req.headers["x-org-id"],
})
seen.auth = req.headers.authorization
seen.org = req.headers["x-org-id"]
if (req.url === "https://one.example.com/api/config") {
return json(req, { config: { theme: "light", seats: 5 } })
@@ -169,15 +164,14 @@ it.effect(
const cfg = yield* AccountService.use((s) => s.config(id, OrgID.make("org-9"))).pipe(Effect.provide(live(client)))
expect(Option.getOrThrow(cfg)).toEqual({ theme: "light", seats: 5 })
expect(yield* Ref.get(seen)).toEqual({
expect(seen).toEqual({
auth: "Bearer at_1",
org: "org-9",
})
}),
)
it.effect(
"poll stores the account and first org on success",
it.effect("poll stores the account and first org on success", () =>
Effect.gen(function* () {
const login = new Login({
code: DeviceCode.make("device-code"),

View File

@@ -3,7 +3,7 @@ import path from "path"
import { tmpdir } from "../fixture/fixture"
import { Instance } from "../../src/project/instance"
import { Agent } from "../../src/agent/agent"
import { PermissionNext } from "../../src/permission/next"
import { PermissionNext } from "../../src/permission"
// Helper to evaluate permission for a tool with wildcard pattern
function evalPerm(agent: Agent.Info | undefined, permission: string): PermissionNext.Action | undefined {
@@ -76,7 +76,7 @@ test("explore agent denies edit and write", async () => {
})
test("explore agent asks for external directories and allows Truncate.GLOB", async () => {
const { Truncate } = await import("../../src/tool/truncation")
const { Truncate } = await import("../../src/tool/truncate")
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
@@ -463,7 +463,7 @@ test("legacy tools config maps write/edit/patch/multiedit to edit permission", a
})
test("Truncate.GLOB is allowed even when user denies external_directory globally", async () => {
const { Truncate } = await import("../../src/tool/truncation")
const { Truncate } = await import("../../src/tool/truncate")
await using tmp = await tmpdir({
config: {
permission: {
@@ -483,7 +483,7 @@ test("Truncate.GLOB is allowed even when user denies external_directory globally
})
test("Truncate.GLOB is allowed even when user denies external_directory per-agent", async () => {
const { Truncate } = await import("../../src/tool/truncation")
const { Truncate } = await import("../../src/tool/truncate")
await using tmp = await tmpdir({
config: {
agent: {
@@ -507,7 +507,7 @@ test("Truncate.GLOB is allowed even when user denies external_directory per-agen
})
test("explicit Truncate.GLOB deny is respected", async () => {
const { Truncate } = await import("../../src/tool/truncation")
const { Truncate } = await import("../../src/tool/truncate")
await using tmp = await tmpdir({
config: {
permission: {

View File

@@ -1,7 +0,0 @@
import { test } from "bun:test"
import { Effect, Layer } from "effect"
export const testEffect = <R, E>(layer: Layer.Layer<R, E, never>) => ({
effect: <A, E2>(name: string, value: Effect.Effect<A, E2, R>) =>
test(name, () => Effect.runPromise(value.pipe(Effect.provide(layer)))),
})

View File

@@ -0,0 +1,37 @@
import { test, type TestOptions } from "bun:test"
import { Cause, Effect, Exit, Layer } from "effect"
import type * as Scope from "effect/Scope"
import * as TestConsole from "effect/testing/TestConsole"
type Body<A, E, R> = Effect.Effect<A, E, R> | (() => Effect.Effect<A, E, R>)
const env = TestConsole.layer
const body = <A, E, R>(value: Body<A, E, R>) => Effect.suspend(() => (typeof value === "function" ? value() : value))
const run = <A, E, R, E2>(value: Body<A, E, R | Scope.Scope>, layer: Layer.Layer<R, E2, never>) =>
Effect.gen(function* () {
const exit = yield* body(value).pipe(Effect.scoped, Effect.provide(layer), Effect.exit)
if (Exit.isFailure(exit)) {
for (const err of Cause.prettyErrors(exit.cause)) {
yield* Effect.logError(err)
}
}
return yield* exit
}).pipe(Effect.runPromise)
const make = <R, E>(layer: Layer.Layer<R, E, never>) => {
const effect = <A, E2>(name: string, value: Body<A, E2, R | Scope.Scope>, opts?: number | TestOptions) =>
test(name, () => run(value, layer), opts)
effect.only = <A, E2>(name: string, value: Body<A, E2, R | Scope.Scope>, opts?: number | TestOptions) =>
test.only(name, () => run(value, layer), opts)
effect.skip = <A, E2>(name: string, value: Body<A, E2, R | Scope.Scope>, opts?: number | TestOptions) =>
test.skip(name, () => run(value, layer), opts)
return { effect }
}
export const it = make(env)
export const testEffect = <R, E>(layer: Layer.Layer<R, E, never>) => make(Layer.provideMerge(layer, env))

View File

@@ -0,0 +1,10 @@
import path from "path"
import { Effect, FileSystem } from "effect"
export const writeFileStringScoped = Effect.fn("test.writeFileStringScoped")(function* (file: string, text: string) {
const fs = yield* FileSystem.FileSystem
yield* fs.makeDirectory(path.dirname(file), { recursive: true })
yield* fs.writeFileString(file, text)
yield* Effect.addFinalizer(() => fs.remove(file, { force: true }).pipe(Effect.orDie))
return file
})

View File

@@ -1,5 +1,5 @@
import { describe, test, expect } from "bun:test"
import { PermissionNext } from "../src/permission/next"
import { PermissionNext } from "../src/permission"
import { Config } from "../src/config/config"
import { Instance } from "../src/project/instance"
import { tmpdir } from "./fixture/fixture"

View File

@@ -4,7 +4,7 @@ import { Effect } from "effect"
import { Bus } from "../../src/bus"
import { runtime } from "../../src/effect/runtime"
import { Instances } from "../../src/effect/instances"
import { PermissionNext } from "../../src/permission/next"
import { PermissionNext } from "../../src/permission"
import * as S from "../../src/permission/service"
import { PermissionID } from "../../src/permission/schema"
import { Instance } from "../../src/project/instance"
@@ -1005,7 +1005,7 @@ test("ask - abort should clear pending request", async () => {
fn: async () => {
const ctl = new AbortController()
const ask = runtime.runPromise(
S.PermissionService.use((svc) =>
S.PermissionEffect.Service.use((svc) =>
svc.ask({
sessionID: SessionID.make("session_test"),
permission: "bash",

View File

@@ -1,73 +0,0 @@
import { describe, expect, test } from "bun:test"
import { Scheduler } from "../src/scheduler"
import { Instance } from "../src/project/instance"
import { tmpdir } from "./fixture/fixture"
describe("Scheduler.register", () => {
const hour = 60 * 60 * 1000
test("defaults to instance scope per directory", async () => {
await using one = await tmpdir({ git: true })
await using two = await tmpdir({ git: true })
const runs = { count: 0 }
const id = "scheduler.instance." + Math.random().toString(36).slice(2)
const task = {
id,
interval: hour,
run: async () => {
runs.count += 1
},
}
await Instance.provide({
directory: one.path,
fn: async () => {
Scheduler.register(task)
await Instance.dispose()
},
})
expect(runs.count).toBe(1)
await Instance.provide({
directory: two.path,
fn: async () => {
Scheduler.register(task)
await Instance.dispose()
},
})
expect(runs.count).toBe(2)
})
test("global scope runs once across instances", async () => {
await using one = await tmpdir({ git: true })
await using two = await tmpdir({ git: true })
const runs = { count: 0 }
const id = "scheduler.global." + Math.random().toString(36).slice(2)
const task = {
id,
interval: hour,
run: async () => {
runs.count += 1
},
scope: "global" as const,
}
await Instance.provide({
directory: one.path,
fn: async () => {
Scheduler.register(task)
await Instance.dispose()
},
})
expect(runs.count).toBe(1)
await Instance.provide({
directory: two.path,
fn: async () => {
Scheduler.register(task)
await Instance.dispose()
},
})
expect(runs.count).toBe(1)
})
})

View File

@@ -5,8 +5,8 @@ import { BashTool } from "../../src/tool/bash"
import { Instance } from "../../src/project/instance"
import { Filesystem } from "../../src/util/filesystem"
import { tmpdir } from "../fixture/fixture"
import type { PermissionNext } from "../../src/permission/next"
import { Truncate } from "../../src/tool/truncation"
import type { PermissionNext } from "../../src/permission"
import { Truncate } from "../../src/tool/truncate"
import { SessionID, MessageID } from "../../src/session/schema"
const ctx = {

View File

@@ -3,7 +3,7 @@ import path from "path"
import type { Tool } from "../../src/tool/tool"
import { Instance } from "../../src/project/instance"
import { assertExternalDirectory } from "../../src/tool/external-directory"
import type { PermissionNext } from "../../src/permission/next"
import type { PermissionNext } from "../../src/permission"
import { SessionID, MessageID } from "../../src/session/schema"
const baseCtx: Omit<Tool.Context, "ask"> = {

View File

@@ -4,7 +4,7 @@ import { ReadTool } from "../../src/tool/read"
import { Instance } from "../../src/project/instance"
import { Filesystem } from "../../src/util/filesystem"
import { tmpdir } from "../fixture/fixture"
import { PermissionNext } from "../../src/permission/next"
import { PermissionNext } from "../../src/permission"
import { Agent } from "../../src/agent/agent"
import { SessionID, MessageID } from "../../src/session/schema"

View File

@@ -1,7 +1,7 @@
import { describe, expect, test } from "bun:test"
import path from "path"
import { pathToFileURL } from "url"
import type { PermissionNext } from "../../src/permission/next"
import type { PermissionNext } from "../../src/permission"
import type { Tool } from "../../src/tool/tool"
import { Instance } from "../../src/project/instance"
import { SkillTool } from "../../src/tool/skill"

View File

@@ -1,9 +1,13 @@
import { describe, test, expect, afterAll } from "bun:test"
import { Truncate } from "../../src/tool/truncation"
import { describe, test, expect } from "bun:test"
import { NodeFileSystem } from "@effect/platform-node"
import { Effect, FileSystem, Layer } from "effect"
import { Truncate } from "../../src/tool/truncate"
import { TruncateEffect } from "../../src/tool/truncate-effect"
import { Identifier } from "../../src/id/id"
import { Filesystem } from "../../src/util/filesystem"
import fs from "fs/promises"
import path from "path"
import { testEffect } from "../lib/effect"
import { writeFileStringScoped } from "../lib/filesystem"
const FIXTURES_DIR = path.join(import.meta.dir, "fixtures")
@@ -125,36 +129,24 @@ describe("Truncate", () => {
describe("cleanup", () => {
const DAY_MS = 24 * 60 * 60 * 1000
let oldFile: string
let recentFile: string
const it = testEffect(Layer.mergeAll(TruncateEffect.defaultLayer, NodeFileSystem.layer))
afterAll(async () => {
await fs.unlink(oldFile).catch(() => {})
await fs.unlink(recentFile).catch(() => {})
})
it.effect("deletes files older than 7 days and preserves recent files", () =>
Effect.gen(function* () {
const fs = yield* FileSystem.FileSystem
test("deletes files older than 7 days and preserves recent files", async () => {
await fs.mkdir(Truncate.DIR, { recursive: true })
yield* fs.makeDirectory(Truncate.DIR, { recursive: true })
// Create an old file (10 days ago)
const oldTimestamp = Date.now() - 10 * DAY_MS
const oldId = Identifier.create("tool", false, oldTimestamp)
oldFile = path.join(Truncate.DIR, oldId)
await Filesystem.write(oldFile, "old content")
const old = path.join(Truncate.DIR, Identifier.create("tool", false, Date.now() - 10 * DAY_MS))
const recent = path.join(Truncate.DIR, Identifier.create("tool", false, Date.now() - 3 * DAY_MS))
// Create a recent file (3 days ago)
const recentTimestamp = Date.now() - 3 * DAY_MS
const recentId = Identifier.create("tool", false, recentTimestamp)
recentFile = path.join(Truncate.DIR, recentId)
await Filesystem.write(recentFile, "recent content")
yield* writeFileStringScoped(old, "old content")
yield* writeFileStringScoped(recent, "recent content")
yield* TruncateEffect.Service.use((s) => s.cleanup())
await Truncate.cleanup()
// Old file should be deleted
expect(await Filesystem.exists(oldFile)).toBe(false)
// Recent file should still exist
expect(await Filesystem.exists(recentFile)).toBe(true)
})
expect(yield* fs.exists(old)).toBe(false)
expect(yield* fs.exists(recent)).toBe(true)
}),
)
})
})