fix(format): set stdin/stdout/stderr to ignore for formatter subprocesses

ChildProcess.make defaults to "pipe" for all stdio streams. Formatters
that wait on stdin EOF or write enough output to fill pipe buffers would
hang. Matches the original Process.spawn behavior.

packages/opencode/specs/effect-migration.md

@@ -212,8 +212,81 @@ Fully migrated (single namespace, InstanceState where needed, flattened facade):
 
 Still open and likely worth migrating:
 
-- [ ] `Session`
-- [ ] `SessionProcessor`
-- [ ] `SessionPrompt`
-- [ ] `SessionCompaction`
-- [ ] `Provider`
+- [x] `Session` — `session/index.ts`
+- [ ] `SessionProcessor` — blocked by AI SDK v6 PR (#18433)
+- [ ] `SessionPrompt` — blocked by AI SDK v6 PR (#18433)
+- [ ] `SessionCompaction` — blocked by AI SDK v6 PR (#18433)
+- [ ] `Provider` — blocked by AI SDK v6 PR (#18433)
+
+Other services not yet migrated:
+
+- [ ] `SessionSummary` — `session/summary.ts`
+- [ ] `SessionTodo` — `session/todo.ts`
+- [ ] `SessionRevert` — `session/revert.ts`
+- [ ] `Instruction` — `session/instruction.ts`
+- [ ] `ShareNext` — `share/share-next.ts`
+- [ ] `SyncEvent` — `sync/index.ts`
+- [ ] `Storage` — `storage/storage.ts`
+- [ ] `Workspace` — `control-plane/workspace.ts`
+
+## Tool interface → Effect
+
+Once individual tools are effectified, change `Tool.Info` (`tool/tool.ts`) so `init` and `execute` return `Effect` instead of `Promise`. This lets tool implementations compose natively with the Effect pipeline rather than being wrapped in `Effect.promise()` at the call site. Requires:
+
+1. Migrate each tool to return Effects
+2. Update `Tool.define()` factory to work with Effects
+3. Update `SessionPrompt` to `yield*` tool results instead of `await`ing — blocked by AI SDK v6 PR (#18433)
+
+Individual tools, ordered by value:
+
+- [ ] `apply_patch.ts` — HIGH: multi-step orchestration, error accumulation, Bus events
+- [ ] `read.ts` — HIGH: streaming I/O, readline, binary detection → FileSystem + Stream
+- [ ] `edit.ts` — HIGH: multi-step diff/format/publish pipeline, FileWatcher lock
+- [ ] `grep.ts` — MEDIUM: spawns ripgrep → ChildProcessSpawner, timeout handling
+- [ ] `write.ts` — MEDIUM: permission checks, diagnostics polling, Bus events
+- [ ] `codesearch.ts` — MEDIUM: HTTP + SSE + manual timeout → HttpClient + Effect.timeout
+- [ ] `webfetch.ts` — MEDIUM: fetch with UA retry, size limits → HttpClient
+- [ ] `websearch.ts` — MEDIUM: MCP over HTTP → HttpClient
+- [ ] `batch.ts` — MEDIUM: parallel execution, per-call error recovery → Effect.all
+- [ ] `task.ts` — MEDIUM: task state management
+- [ ] `glob.ts` — LOW: simple async generator
+- [ ] `lsp.ts` — LOW: dispatch switch over LSP operations
+- [ ] `skill.ts` — LOW: skill tool adapter
+- [ ] `plan.ts` — LOW: plan file operations
+
+## Effect service adoption in already-migrated code
+
+Some services are effectified but still use raw `Filesystem.*` or `Process.spawn` instead of the Effect equivalents. These are low-hanging fruit — the layers already exist, they just need the dependency swap.
+
+### `Filesystem.*` → `AppFileSystem.Service` (yield in layer)
+
+- [ ] `file/index.ts` — 11 calls (the File service itself)
+- [ ] `config/config.ts` — 7 calls
+- [ ] `auth/index.ts` — 3 calls
+- [ ] `skill/index.ts` — 3 calls
+- [ ] `file/time.ts` — 1 call
+
+### `Process.spawn` → `ChildProcessSpawner` (yield in layer)
+
+- [ ] `format/index.ts` — 1 call
+
+## Filesystem consolidation
+
+`util/filesystem.ts` (raw fs wrapper) is used by **64 files**. The effectified `AppFileSystem` service (`filesystem/index.ts`) exists but only has **8 consumers**. As services and tools are effectified, they should switch from `Filesystem.*` to yielding `AppFileSystem.Service` — this happens naturally during each migration, not as a separate effort.
+
+Similarly, **28 files** still import raw `fs` or `fs/promises` directly. These should migrate to `AppFileSystem` or `Filesystem.*` as they're touched.
+
+Current raw fs users that will convert during tool migration:
+- `tool/read.ts` — fs.createReadStream, readline
+- `tool/apply_patch.ts` — fs/promises
+- `tool/bash.ts` — fs/promises
+- `file/ripgrep.ts` — fs/promises
+- `storage/storage.ts` — fs/promises
+- `patch/index.ts` — fs, fs/promises
+
+## Primitives & utilities
+
+- [ ] `util/lock.ts` — reader-writer lock → Effect Semaphore/Permit
+- [ ] `util/flock.ts` — file-based distributed lock with heartbeat → Effect.repeat + addFinalizer
+- [ ] `util/process.ts` — child process spawn wrapper → return Effect instead of Promise
+- [ ] `util/lazy.ts` — replace uses in Effect code with Effect.cached; keep for sync-only code

packages/opencode/src/format/index.ts

@@ -1,4 +1,6 @@
 import { Effect, Layer, ServiceMap } from "effect"
+import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"
+import * as CrossSpawnSpawner from "@/effect/cross-spawn-spawner"
 import { InstanceState } from "@/effect/instance-state"
 import { makeRuntime } from "@/effect/run-service"
 import path from "path"
@@ -6,7 +8,6 @@ import { mergeDeep } from "remeda"
 import z from "zod"
 import { Config } from "../config/config"
 import { Instance } from "../project/instance"
-import { Process } from "../util/process"
 import { Log } from "../util/log"
 import * as Formatter from "./formatter"
 
@@ -36,6 +37,7 @@ export namespace Format {
     Service,
     Effect.gen(function* () {
       const config = yield* Config.Service
+      const spawner = yield* ChildProcessSpawner.ChildProcessSpawner
 
       const state = yield* InstanceState.make(
         Effect.fn("Format.state")(function* (_ctx) {
@@ -98,38 +100,48 @@ export namespace Format {
             return checks.filter((x) => x.enabled).map((x) => x.item)
           }
 
-          async function formatFile(filepath: string) {
-            log.info("formatting", { file: filepath })
-            const ext = path.extname(filepath)
+          function formatFile(filepath: string) {
+            return Effect.gen(function* () {
+              log.info("formatting", { file: filepath })
+              const ext = path.extname(filepath)
 
-            for (const item of await getFormatter(ext)) {
-              log.info("running", { command: item.command })
-              try {
-                const proc = Process.spawn(
-                  item.command.map((x) => x.replace("$FILE", filepath)),
-                  {
-                    cwd: Instance.directory,
-                    env: { ...process.env, ...item.environment },
-                    stdout: "ignore",
-                    stderr: "ignore",
-                  },
-                )
-                const exit = await proc.exited
-                if (exit !== 0) {
+              for (const item of yield* Effect.promise(() => getFormatter(ext))) {
+                log.info("running", { command: item.command })
+                const cmd = item.command.map((x) => x.replace("$FILE", filepath))
+                const code = yield* spawner
+                  .spawn(
+                    ChildProcess.make(cmd[0]!, cmd.slice(1), {
+                      cwd: Instance.directory,
+                      env: item.environment,
+                      extendEnv: true,
+                      stdin: "ignore",
+                      stdout: "ignore",
+                      stderr: "ignore",
+                    }),
+                  )
+                  .pipe(
+                    Effect.flatMap((handle) => handle.exitCode),
+                    Effect.scoped,
+                    Effect.catch(() =>
+                      Effect.sync(() => {
+                        log.error("failed to format file", {
+                          error: "spawn failed",
+                          command: item.command,
+                          ...item.environment,
+                          file: filepath,
+                        })
+                        return ChildProcessSpawner.ExitCode(1)
+                      }),
+                    ),
+                  )
+                if (code !== 0) {
                   log.error("failed", {
                     command: item.command,
                     ...item.environment,
                   })
                 }
-              } catch (error) {
-                log.error("failed to format file", {
-                  error,
-                  command: item.command,
-                  ...item.environment,
-                  file: filepath,
-                })
               }
-            }
+            })
           }
 
           log.info("init")
@@ -162,14 +174,14 @@ export namespace Format {
 
       const file = Effect.fn("Format.file")(function* (filepath: string) {
         const { formatFile } = yield* InstanceState.get(state)
-        yield* Effect.promise(() => formatFile(filepath))
+        yield* formatFile(filepath)
       })
 
       return Service.of({ init, status, file })
     }),
   )
 
-  export const defaultLayer = layer.pipe(Layer.provide(Config.defaultLayer))
+  export const defaultLayer = layer.pipe(Layer.provide(Config.defaultLayer), Layer.provide(CrossSpawnSpawner.defaultLayer))
 
   const { runPromise } = makeRuntime(Service, defaultLayer)
 

packages/opencode/test/format/format.test.ts

@@ -1,17 +1,13 @@
-import { NodeChildProcessSpawner, NodeFileSystem, NodePath } from "@effect/platform-node"
+import { NodeFileSystem } from "@effect/platform-node"
 import { describe, expect } from "bun:test"
 import { Effect, Layer } from "effect"
 import { provideTmpdirInstance } from "../fixture/fixture"
 import { testEffect } from "../lib/effect"
+import * as CrossSpawnSpawner from "../../src/effect/cross-spawn-spawner"
 import { Format } from "../../src/format"
-import { Config } from "../../src/config/config"
 import * as Formatter from "../../src/format/formatter"
 
-const node = NodeChildProcessSpawner.layer.pipe(
-  Layer.provideMerge(Layer.mergeAll(NodeFileSystem.layer, NodePath.layer)),
-)
-
-const it = testEffect(Layer.mergeAll(Format.layer, node).pipe(Layer.provide(Config.defaultLayer)))
+const it = testEffect(Layer.mergeAll(Format.defaultLayer, CrossSpawnSpawner.defaultLayer, NodeFileSystem.layer))
 
 describe("Format", () => {
   it.effect("status() returns built-in formatters when no config overrides", () =>