refactor(core): simplify search root protocol (#31060)

This commit is contained in:
Kit Langton 2026-06-05 23:20:06 -04:00 committed by GitHub
parent 4ac4df448a
commit 09d9cf01f9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 18 additions and 89 deletions

View File

@ -153,17 +153,13 @@ export class ListTarget extends Schema.Class<ListTarget>("FileSystem.ListTarget"
resource: Schema.String, resource: Schema.String,
}) {} }) {}
/** Canonical read authority for Location-scoped search and metadata leaves. */ /** Canonical root and permission resource for Location-scoped search. */
export class RootTarget extends Schema.Class<RootTarget>("FileSystem.RootTarget")({ export class RootTarget extends Schema.Class<RootTarget>("FileSystem.RootTarget")({
absolute: Schema.String,
real: Schema.String, real: Schema.String,
directory: Schema.String,
root: Schema.String, root: Schema.String,
resource: Schema.String, resource: Schema.String,
reference: Schema.NonEmptyString.pipe(Schema.optional), reference: Schema.NonEmptyString.pipe(Schema.optional),
type: Schema.Literals(["file", "directory"]), type: Schema.Literals(["file", "directory"]),
dev: Schema.Number,
ino: Schema.Number.pipe(Schema.optional),
}) {} }) {}
export class Entry extends Schema.Class<Entry>("FileSystem.Entry")({ export class Entry extends Schema.Class<Entry>("FileSystem.Entry")({
@ -221,9 +217,8 @@ export interface Interface {
readonly resolveReadPath: (input: ReadInput) => Effect.Effect<ReadPath> readonly resolveReadPath: (input: ReadInput) => Effect.Effect<ReadPath>
readonly readTool: (input: ReadInput, page?: TextPageInput) => Effect.Effect<Content | TextPage> readonly readTool: (input: ReadInput, page?: TextPageInput) => Effect.Effect<Content | TextPage>
readonly list: (input?: ListInput) => Effect.Effect<Entry[]> readonly list: (input?: ListInput) => Effect.Effect<Entry[]>
/** Select a contained canonical read root without asserting leaf policy. */ /** Resolve a contained canonical search root and its permission resource. */
readonly resolveRoot: (input?: ListInput) => Effect.Effect<RootTarget> readonly resolveRoot: (input?: ListInput) => Effect.Effect<RootTarget>
readonly revalidateRoot: (target: RootTarget) => Effect.Effect<RootTarget>
readonly resolveList: (input?: ListInput) => Effect.Effect<ListTarget> readonly resolveList: (input?: ListInput) => Effect.Effect<ListTarget>
readonly listResolved: (target: ListTarget) => Effect.Effect<Entry[]> readonly listResolved: (target: ListTarget) => Effect.Effect<Entry[]>
readonly listPage: (input?: ListPageInput) => Effect.Effect<ListPage> readonly listPage: (input?: ListPageInput) => Effect.Effect<ListPage>
@ -535,22 +530,8 @@ export const layer = Layer.effect(
resource: input.reference === undefined ? relative : `${input.reference}:${relative}`, resource: input.reference === undefined ? relative : `${input.reference}:${relative}`,
reference: input.reference, reference: input.reference,
type, type,
dev: info.dev,
ino: Option.getOrUndefined(info.ino),
}) })
}) })
const revalidateRoot = Effect.fn("FileSystem.revalidateRoot")(function* (target: RootTarget) {
const canonical = yield* fs.realPath(target.absolute).pipe(Effect.orDie)
if (canonical !== target.real) return yield* Effect.die(new Error("Search root changed after approval"))
const info = yield* fs.stat(canonical).pipe(Effect.orDie)
if (
info.type !== (target.type === "file" ? "File" : "Directory") ||
info.dev !== target.dev ||
Option.getOrUndefined(info.ino) !== target.ino
)
return yield* Effect.die(new Error("Search root identity changed after approval"))
return target
})
const listResolved = Effect.fn("FileSystem.listResolved")(function* (directory: ListTarget) { const listResolved = Effect.fn("FileSystem.listResolved")(function* (directory: ListTarget) {
return yield* fs.readDirectoryEntries(directory.real).pipe( return yield* fs.readDirectoryEntries(directory.real).pipe(
Effect.orDie, Effect.orDie,
@ -613,7 +594,6 @@ export const layer = Layer.effect(
return yield* listResolved(yield* resolveList(input)) return yield* listResolved(yield* resolveList(input))
}), }),
resolveRoot, resolveRoot,
revalidateRoot,
resolveList, resolveList,
listResolved, listResolved,
listPage: Effect.fn("FileSystem.listPage")(function* (input) { listPage: Effect.fn("FileSystem.listPage")(function* (input) {

View File

@ -24,14 +24,9 @@ export const MAX_LINE_PREVIEW_LENGTH = 2_000
export const ResultLimit = PositiveInt.check(Schema.isLessThanOrEqualTo(MAX_RESULT_LIMIT)) export const ResultLimit = PositiveInt.check(Schema.isLessThanOrEqualTo(MAX_RESULT_LIMIT))
const RootInput = {
path: Schema.String.pipe(Schema.optional),
reference: Schema.NonEmptyString.pipe(Schema.optional),
}
export const FilesInput = Schema.Struct({ export const FilesInput = Schema.Struct({
pattern: Schema.String, pattern: Schema.String,
...RootInput, ...FileSystem.ListInput.fields,
limit: ResultLimit.pipe(Schema.optional), limit: ResultLimit.pipe(Schema.optional),
}) })
export type FilesInput = typeof FilesInput.Type & { readonly signal?: AbortSignal } export type FilesInput = typeof FilesInput.Type & { readonly signal?: AbortSignal }
@ -39,7 +34,7 @@ export type FilesInput = typeof FilesInput.Type & { readonly signal?: AbortSigna
export const GrepInput = Schema.Struct({ export const GrepInput = Schema.Struct({
pattern: Schema.String, pattern: Schema.String,
include: Schema.String.pipe(Schema.optional), include: Schema.String.pipe(Schema.optional),
...RootInput, ...FileSystem.ListInput.fields,
limit: ResultLimit.pipe(Schema.optional), limit: ResultLimit.pipe(Schema.optional),
}) })
export type GrepInput = typeof GrepInput.Type & { readonly signal?: AbortSignal } export type GrepInput = typeof GrepInput.Type & { readonly signal?: AbortSignal }
@ -82,11 +77,8 @@ export class GrepResult extends Schema.Class<GrepResult>("LocationSearch.GrepRes
}) {} }) {}
export interface Interface { export interface Interface {
readonly files: (input: FilesInput, root?: FileSystem.RootTarget) => Effect.Effect<FilesResult, Ripgrep.Error> readonly files: (input: FilesInput) => Effect.Effect<FilesResult, Ripgrep.Error>
readonly grep: ( readonly grep: (input: GrepInput) => Effect.Effect<GrepResult, Ripgrep.Error | Ripgrep.InvalidPatternError>
input: GrepInput,
root?: FileSystem.RootTarget,
) => Effect.Effect<GrepResult, Ripgrep.Error | Ripgrep.InvalidPatternError>
} }
export class Service extends Context.Service<Service, Interface>()("@opencode/v2/LocationSearch") {} export class Service extends Context.Service<Service, Interface>()("@opencode/v2/LocationSearch") {}
@ -123,8 +115,8 @@ export const layer = Layer.effect(
}) })
return Service.of({ return Service.of({
files: Effect.fn("LocationSearch.files")(function* (input, approvedRoot) { files: Effect.fn("LocationSearch.files")(function* (input) {
const root = yield* filesystem.revalidateRoot(approvedRoot ?? (yield* filesystem.resolveRoot(input))) const root = yield* filesystem.resolveRoot(input)
if (root.type !== "directory") if (root.type !== "directory")
return yield* Effect.die(new globalThis.Error("Files search path must be a directory")) return yield* Effect.die(new globalThis.Error("Files search path must be a directory"))
const result = yield* ripgrep.files({ const result = yield* ripgrep.files({
@ -145,8 +137,8 @@ export const layer = Layer.effect(
partial: result.partial || items.length !== result.items.length, partial: result.partial || items.length !== result.items.length,
}) })
}), }),
grep: Effect.fn("LocationSearch.grep")(function* (input, approvedRoot) { grep: Effect.fn("LocationSearch.grep")(function* (input) {
const root = yield* filesystem.revalidateRoot(approvedRoot ?? (yield* filesystem.resolveRoot(input))) const root = yield* filesystem.resolveRoot(input)
const cwd = root.type === "directory" ? root.real : path.dirname(root.real) const cwd = root.type === "directory" ? root.real : path.dirname(root.real)
const result = yield* ripgrep.grep({ const result = yield* ripgrep.grep({
cwd, cwd,

View File

@ -45,8 +45,8 @@ const definition = Tool.make({
}) })
/** /**
* Location-scoped glob leaf. FileSystem selects a canonical root for * Location-scoped glob leaf. FileSystem supplies canonical permission metadata;
* permission metadata; LocationSearch owns containment and traversal. * LocationSearch resolves the current root and owns containment and traversal.
* *
* TODO: Revisit root-specific search permission resources if named-reference policy needs independent allow/deny rules. * TODO: Revisit root-specific search permission resources if named-reference policy needs independent allow/deny rules.
*/ */
@ -73,7 +73,7 @@ export const layer = Layer.effectDiscard(
limit: parameters.limit, limit: parameters.limit,
}, },
}) })
return yield* search.files(parameters, root) return yield* search.files(parameters)
}).pipe( }).pipe(
Effect.catchCause((cause) => Effect.catchCause((cause) =>
Effect.fail( Effect.fail(

View File

@ -60,8 +60,8 @@ const definition = Tool.make({
}) })
/** /**
* Location-scoped grep leaf. FileSystem selects a canonical root for * Location-scoped grep leaf. FileSystem supplies canonical permission metadata;
* permission metadata; LocationSearch owns containment and ripgrep execution. * LocationSearch resolves the current root and owns containment and ripgrep execution.
* *
* TODO: Revisit root-specific search permission resources if named-reference policy needs independent allow/deny rules. * TODO: Revisit root-specific search permission resources if named-reference policy needs independent allow/deny rules.
*/ */
@ -89,7 +89,7 @@ export const layer = Layer.effectDiscard(
limit: parameters.limit, limit: parameters.limit,
}, },
}) })
return yield* search.grep(parameters, root) return yield* search.grep(parameters)
}).pipe( }).pipe(
Effect.catchCause((cause) => { Effect.catchCause((cause) => {
const error = Cause.squash(cause) const error = Cause.squash(cause)

View File

@ -243,32 +243,6 @@ describe("LocationSearch", () => {
), ),
) )
it.live("rejects an approved root swapped to a symlink before ripgrep traversal", () =>
withTmp((directory) =>
Effect.gen(function* () {
if (process.platform === "win32") return
const source = path.join(directory, "src")
const outside = `${directory}-outside`
yield* Effect.promise(async () => {
await fs.mkdir(source)
await fs.mkdir(outside)
await fs.writeFile(path.join(outside, "secret.txt"), "secret\n")
})
const filesystem = yield* FileSystem.Service
const approved = yield* filesystem.resolveRoot({ path: RelativePath.make("src") })
yield* Effect.promise(async () => {
await fs.rmdir(source)
await fs.symlink(outside, source)
})
expect(
Exit.isFailure(yield* (yield* LocationSearch.Service).files({ pattern: "*" }, approved).pipe(Effect.exit)),
).toBe(true)
yield* Effect.promise(() => fs.rm(outside, { recursive: true, force: true }))
}).pipe(provide(directory)),
),
)
it.live("honors a pre-aborted cancellation signal", () => it.live("honors a pre-aborted cancellation signal", () =>
withTmp((directory) => withTmp((directory) =>
Effect.gen(function* () { Effect.gen(function* () {

View File

@ -13,7 +13,6 @@ const sessionID = SessionV2.ID.make("ses_glob_tool_test")
const assertions: PermissionV2.AssertInput[] = [] const assertions: PermissionV2.AssertInput[] = []
const resolutions: FileSystem.ListInput[] = [] const resolutions: FileSystem.ListInput[] = []
const searches: LocationSearch.FilesInput[] = [] const searches: LocationSearch.FilesInput[] = []
const roots: FileSystem.RootTarget[] = []
let allow = true let allow = true
let result = new LocationSearch.FilesResult({ items: [], truncated: false, partial: false }) let result = new LocationSearch.FilesResult({ items: [], truncated: false, partial: false })
@ -45,17 +44,13 @@ const filesystem = Layer.succeed(
const relative = input.path ?? RelativePath.make(".") const relative = input.path ?? RelativePath.make(".")
const resource = input.reference === undefined ? relative : `${input.reference}:${relative}` const resource = input.reference === undefined ? relative : `${input.reference}:${relative}`
return new FileSystem.RootTarget({ return new FileSystem.RootTarget({
absolute: `/project/${relative}`,
real: `/project/${relative}`, real: `/project/${relative}`,
directory: "/project",
root: "/project", root: "/project",
resource, resource,
reference: input.reference, reference: input.reference,
type: "directory", type: "directory",
dev: 1,
}) })
}), }),
revalidateRoot: Effect.succeed,
resolveList: () => Effect.die("unused"), resolveList: () => Effect.die("unused"),
listResolved: () => Effect.die("unused"), listResolved: () => Effect.die("unused"),
listPage: () => Effect.die("unused"), listPage: () => Effect.die("unused"),
@ -69,10 +64,9 @@ const filesystem = Layer.succeed(
const search = Layer.succeed( const search = Layer.succeed(
LocationSearch.Service, LocationSearch.Service,
LocationSearch.Service.of({ LocationSearch.Service.of({
files: (input, root) => files: (input) =>
Effect.sync(() => { Effect.sync(() => {
searches.push(input) searches.push(input)
if (root) roots.push(root)
return result return result
}), }),
grep: () => Effect.die("unused"), grep: () => Effect.die("unused"),
@ -92,7 +86,6 @@ const reset = () => {
assertions.length = 0 assertions.length = 0
resolutions.length = 0 resolutions.length = 0
searches.length = 0 searches.length = 0
roots.length = 0
allow = true allow = true
result = new LocationSearch.FilesResult({ items: [], truncated: false, partial: false }) result = new LocationSearch.FilesResult({ items: [], truncated: false, partial: false })
} }
@ -130,7 +123,6 @@ describe("GlobTool", () => {
]) ])
expect(resolutions).toEqual([{ path: RelativePath.make("src"), reference: undefined }]) expect(resolutions).toEqual([{ path: RelativePath.make("src"), reference: undefined }])
expect(searches).toEqual([{ pattern: "**/*.ts", path: RelativePath.make("src"), limit: 12 }]) expect(searches).toEqual([{ pattern: "**/*.ts", path: RelativePath.make("src"), limit: 12 }])
expect(roots).toMatchObject([{ resource: "src" }])
}), }),
) )

View File

@ -22,7 +22,6 @@ import { testEffect } from "./lib/effect"
const assertions: PermissionV2.AssertInput[] = [] const assertions: PermissionV2.AssertInput[] = []
const searches: LocationSearch.GrepInput[] = [] const searches: LocationSearch.GrepInput[] = []
const roots: FileSystem.RootTarget[] = []
let allow = true let allow = true
let result = new LocationSearch.GrepResult({ items: [], truncated: false, partial: false }) let result = new LocationSearch.GrepResult({ items: [], truncated: false, partial: false })
let searchFailure: Ripgrep.InvalidPatternError | undefined let searchFailure: Ripgrep.InvalidPatternError | undefined
@ -37,17 +36,13 @@ const filesystem = Layer.succeed(
resolveRoot: (input = {}) => resolveRoot: (input = {}) =>
Effect.succeed( Effect.succeed(
new FileSystem.RootTarget({ new FileSystem.RootTarget({
absolute: `/project/${input.path ?? "."}`,
real: `/project/${input.path ?? "."}`, real: `/project/${input.path ?? "."}`,
directory: "/project",
root: "/project", root: "/project",
resource: input.reference === undefined ? (input.path ?? ".") : `${input.reference}:${input.path ?? "."}`, resource: input.reference === undefined ? (input.path ?? ".") : `${input.reference}:${input.path ?? "."}`,
reference: input.reference, reference: input.reference,
type: "directory", type: "directory",
dev: 1,
}), }),
), ),
revalidateRoot: Effect.succeed,
resolveList: () => Effect.die("unused"), resolveList: () => Effect.die("unused"),
listResolved: () => Effect.die("unused"), listResolved: () => Effect.die("unused"),
listPage: () => Effect.die("unused"), listPage: () => Effect.die("unused"),
@ -61,10 +56,9 @@ const search = Layer.succeed(
LocationSearch.Service, LocationSearch.Service,
LocationSearch.Service.of({ LocationSearch.Service.of({
files: () => Effect.die("unused"), files: () => Effect.die("unused"),
grep: (input, root) => grep: (input) =>
Effect.sync(() => { Effect.sync(() => {
searches.push(input) searches.push(input)
if (root) roots.push(root)
if (searchFailure) throw searchFailure if (searchFailure) throw searchFailure
return result return result
}), }),
@ -107,7 +101,6 @@ const settle = (input: Record<string, unknown>) =>
const reset = () => { const reset = () => {
assertions.length = 0 assertions.length = 0
searches.length = 0 searches.length = 0
roots.length = 0
allow = true allow = true
searchFailure = undefined searchFailure = undefined
result = new LocationSearch.GrepResult({ items: [], truncated: false, partial: false }) result = new LocationSearch.GrepResult({ items: [], truncated: false, partial: false })
@ -172,7 +165,6 @@ describe("GrepTool", () => {
}, },
]) ])
expect(searches).toEqual([{ pattern: "needle", path: RelativePath.make("src"), include: "*.ts", limit: 2 }]) expect(searches).toEqual([{ pattern: "needle", path: RelativePath.make("src"), include: "*.ts", limit: 2 }])
expect(roots).toMatchObject([{ resource: "src" }])
}), }),
) )

View File

@ -43,7 +43,6 @@ const filesystem = Layer.succeed(
return Effect.succeed(readResult) return Effect.succeed(readResult)
}, },
resolveRoot: () => Effect.die("unused"), resolveRoot: () => Effect.die("unused"),
revalidateRoot: Effect.succeed,
list: () => Effect.die("unused"), list: () => Effect.die("unused"),
resolveList: () => Effect.die("unused"), resolveList: () => Effect.die("unused"),
listResolved: () => Effect.die("unused"), listResolved: () => Effect.die("unused"),