refactor(core): consolidate filesystem services (#30447)
This commit is contained in:
@@ -32,7 +32,7 @@ Caller-heavy batch, all merged:
|
||||
|
||||
1. `src/config/config.ts`
|
||||
2. `src/provider/provider.ts`
|
||||
3. `src/file/index.ts`
|
||||
3. `../core/src/filesystem.ts`
|
||||
4. `src/lsp/index.ts`
|
||||
5. `src/mcp/index.ts`
|
||||
|
||||
@@ -168,7 +168,7 @@ Usually no.
|
||||
Prefer the direct form when there is only one expression:
|
||||
|
||||
```ts
|
||||
await AppRuntime.runPromise(File.Service.use((svc) => svc.read(path)))
|
||||
await Effect.runPromise(FileSystem.Service.use((svc) => svc.read({ path })))
|
||||
```
|
||||
|
||||
Use `Effect.gen(...)` when the workflow actually needs multiple yielded values or branching.
|
||||
@@ -179,7 +179,7 @@ These were the recurring mistakes and useful corrections from the first two batc
|
||||
|
||||
1. Tests should usually provide the specific service layer, not `AppRuntime`.
|
||||
2. If a test uses `provideTmpdirInstance(...)` and needs child processes, prefer `CrossSpawnSpawner.defaultLayer`.
|
||||
3. Instance-scoped services may need both the service layer and the right instance fixture. `File` tests, for example, needed `provideInstance(...)` plus `File.defaultLayer`.
|
||||
3. Location-scoped services may need both the service layer and the right location fixture. `FileSystem` tests, for example, provide `Location.Service` plus `FileSystem.locationLayer`.
|
||||
4. Do not wrap a single `Service.use(...)` call in `Effect.gen(...)` just to return it. Use the direct form.
|
||||
5. For CLI readability, extract file-local preload helpers when the handler starts doing config load + service load + batched effect fanout inline.
|
||||
6. When rebasing a facade branch after nearby merges, prefer the already-cleaned service/test version over older inline facade-era code.
|
||||
@@ -201,7 +201,7 @@ Most of the original facade-removal backlog is already done. The practical remai
|
||||
- [x] `src/worktree/index.ts` (`Worktree`) - service-local facades removed
|
||||
- [x] `src/plugin/index.ts` (`Plugin`) - service-local facades removed
|
||||
- [x] `src/snapshot/index.ts` (`Snapshot`) - service-local facades removed
|
||||
- [x] `src/file/index.ts` (`File`) - facades removed and merged
|
||||
- [x] `../core/src/filesystem.ts` (`FileSystem`) - legacy opencode service removed
|
||||
- [x] `src/lsp/index.ts` (`LSP`) - facades removed and merged
|
||||
- [x] `src/mcp/index.ts` (`MCP`) - facades removed and merged
|
||||
- [x] `src/config/config.ts` (`Config`) - facades removed and merged
|
||||
|
||||
@@ -179,7 +179,7 @@ Intentional boundaries:
|
||||
In effectified code, yield existing services instead of dropping to ad hoc
|
||||
platform APIs.
|
||||
|
||||
- Use `AppFileSystem.Service` instead of raw `fs/promises` for app file IO.
|
||||
- Use `FSUtil.Service` instead of raw `fs/promises` for app file IO.
|
||||
- Use `AppProcess.Service` instead of direct `ChildProcessSpawner.spawn` or
|
||||
legacy process helpers.
|
||||
- Use `HttpClient.HttpClient` instead of raw `fetch` inside Effect code.
|
||||
|
||||
@@ -14,7 +14,7 @@ Small follow-ups that do not fit neatly into the main facade, route, tool, or sc
|
||||
|
||||
- [ ] `config/paths.ts` - split pure helpers from effectful helpers.
|
||||
Keep `fileInDirectory(...)` as a plain function.
|
||||
- [ ] `config/paths.ts` - add a `ConfigPaths.Service` for the effectful operations so callers do not inherit `AppFileSystem.Service` directly.
|
||||
- [ ] `config/paths.ts` - add a `ConfigPaths.Service` for the effectful operations so callers do not inherit `FSUtil.Service` directly.
|
||||
Initial service surface should cover:
|
||||
- `projectFiles(...)`
|
||||
- `directories(...)`
|
||||
|
||||
@@ -33,7 +33,7 @@ genuinely outside `AppLayer`.
|
||||
|
||||
## Platform Edges
|
||||
|
||||
- Use `AppFileSystem.Service` instead of raw filesystem APIs in
|
||||
- Use `FSUtil.Service` instead of raw filesystem APIs in
|
||||
effectified services.
|
||||
- Use `AppProcess.Service` instead of raw process wrappers.
|
||||
- Use `HttpClient.HttpClient` instead of raw `fetch` in Effect code.
|
||||
|
||||
@@ -72,7 +72,7 @@ P6 OA
|
||||
- `PROC` AppProcess migration — prefer `AppProcess.Service` over raw
|
||||
process wrappers.
|
||||
Shrinks: direct spawn callsites and legacy process helpers.
|
||||
- `FS` AppFileSystem migration — prefer `AppFileSystem.Service` over raw
|
||||
- `FS` FSUtil migration — prefer `FSUtil.Service` over raw
|
||||
filesystem APIs.
|
||||
Shrinks: direct `fs` / `Bun.file` service callsites where inappropriate.
|
||||
- `RT` Runtime/facade cleanup — remove service-local `makeRuntime`
|
||||
@@ -229,7 +229,7 @@ Current rules:
|
||||
|
||||
## Lower Priority Tracks
|
||||
|
||||
- `PROC` / `FS` — continue AppProcess and AppFileSystem migrations as
|
||||
- `PROC` / `FS` — continue AppProcess and FSUtil migrations as
|
||||
focused PRs when touching relevant files.
|
||||
- `RT` — remove service-local runtime facades only when they are not an
|
||||
intentional boundary.
|
||||
|
||||
@@ -11,7 +11,7 @@ The current exported tools in `src/tool` all use `Tool.define(...)` with Effect-
|
||||
So the remaining work is no longer "convert tools to Effect at all". The remaining work is mostly:
|
||||
|
||||
1. remove Promise and raw platform bridges inside individual tool bodies
|
||||
2. swap tool internals to Effect-native services like `AppFileSystem`, `HttpClient`, and `ChildProcessSpawner`
|
||||
2. swap tool internals to Effect-native services like `FSUtil`, `HttpClient`, and `ChildProcessSpawner`
|
||||
3. keep tests and callers aligned with `yield* info.init()` and real service graphs
|
||||
|
||||
## Current shape
|
||||
@@ -67,11 +67,11 @@ Most exported tools are already on the intended Effect-native shape. The remaini
|
||||
|
||||
Current spot cleanups worth tracking:
|
||||
|
||||
- [x] `read.ts` — streams through `AppFileSystem.Service.stream` with `Stream.splitLines`; the legacy Node stream / `readline` helper is gone
|
||||
- [x] `read.ts` — streams through `FSUtil.Service.stream` with `Stream.splitLines`; the legacy Node stream / `readline` helper is gone
|
||||
- [ ] `bash.ts` — already uses Effect child-process primitives; only keep tracking shell-specific platform bridges and parser/loading details as they come up
|
||||
- [ ] `webfetch.ts` — already uses `HttpClient`; remaining work is limited to smaller boundary helpers like HTML text extraction
|
||||
- [ ] `file/ripgrep.ts` — adjacent to tool migration; still has raw fs/process usage that affects `grep.ts` and file-search routes
|
||||
- [x] `patch/index.ts` — apply path now returns `Effect` over `AppFileSystem.Service`; the parser and chunk replacer stay pure
|
||||
- [x] `patch/index.ts` — apply path now returns `Effect` over `FSUtil.Service`; the parser and chunk replacer stay pure
|
||||
|
||||
Notable items that are already effectively on the target path and do not need separate migration bullets right now:
|
||||
|
||||
|
||||
Reference in New Issue
Block a user