KirinCode/packages/kirincode/specs/effect/facades.md

Facade removal checklist

Concrete inventory of the remaining makeRuntime(...)-backed facades in packages/opencode.

Current status on this branch:

• src/ has 5 makeRuntime(...) call sites total.
• 2 are intentionally excluded from this checklist: src/bus/index.ts and src/effect/cross-spawn-spawner.ts.
• That leaves 2 live runtime-backed service facades still worth tracking here: src/npm/index.ts and src/cli/cmd/tui/config/tui.ts.

Recent progress:

• Wave 1 is merged: Pty, Skill, Vcs, ToolRegistry, Auth.
• Wave 2 is merged: Config, Provider, File, LSP, MCP.

Priority hotspots

• src/cli/cmd/tui/config/tui.ts still exports makeRuntime(...) plus async facade helpers for get() and waitForDependencies().
• src/npm/index.ts still exports makeRuntime(...) plus async facade helpers for install(), add(), outdated(), and which().

Completed Batches

Low-risk batch, all merged:

1. src/pty/index.ts
2. src/skill/index.ts
3. src/project/vcs.ts
4. src/tool/registry.ts
5. src/auth/index.ts

Caller-heavy batch, all merged:

1. src/config/config.ts
2. src/provider/provider.ts
3. ../core/src/filesystem.ts
4. src/lsp/index.ts
5. src/mcp/index.ts

Shared pattern:

• one service file still exports makeRuntime(...) + async facades
• one or two route or CLI entrypoints call those facades directly
• tests call the facade directly and need to switch to yield* svc.method(...)
• once callers are gone, delete makeRuntime(...), remove async facade exports, and drop the makeRuntime import

Done means

For each service in the low-risk batch, the work is complete only when all of these are true:

1. all production callers stop using Namespace.method(...) facade calls
2. all direct test callers stop using the facade and instead yield the service from context
3. the service file no longer has makeRuntime(...)
4. the service file no longer exports runtime-backed facade helpers
5. grep for the migrated facade methods only finds the service implementation itself or unrelated names

Caller templates

Route handlers

Use one AppRuntime.runPromise(Effect.gen(...)) body and yield the service inside it.

const value = await AppRuntime.runPromise(
  Effect.gen(function* () {
    const pty = yield* Pty.Service
    return yield* pty.list()
  }),
)

If two service calls are independent, keep them in the same effect body and use Effect.all(...).

Plain async CLI or script entrypoints

If the caller is not itself an Effect service yet, still prefer one contiguous AppRuntime.runPromise(Effect.gen(...)) block for the whole unit of work.

const skills = await AppRuntime.runPromise(
  Effect.gen(function* () {
    const auth = yield* Auth.Service
    const skill = yield* Skill.Service
    yield* auth.set(key, info)
    return yield* skill.all()
  }),
)

Only fall back to AppRuntime.runPromise(Service.use(...)) for truly isolated one-off calls or awkward callback boundaries. Do not stack multiple tiny runPromise(...) calls in the same contiguous workflow.

This is the right intermediate state. Do not block facade removal on effectifying the whole CLI file.

Bootstrap or fire-and-forget startup code

If the old facade call existed only to kick off initialization, call the service through the existing runtime for that file.

void BootstrapRuntime.runPromise(Vcs.Service.use((svc) => svc.init()))

Do not reintroduce a dedicated runtime in the service just for bootstrap.

Tests

Convert facade tests to full effect style.

it.effect("does the thing", () =>
  Effect.gen(function* () {
    const svc = yield* Pty.Service
    const info = yield* svc.create({ command: "cat", title: "a" })
    yield* svc.remove(info.id)
  }).pipe(Effect.provide(Pty.defaultLayer)),
)

If the repo test already uses testEffect(...), prefer testEffect(Service.defaultLayer) and yield* Service.Service inside the test body.

Do not route tests through AppRuntime unless the test is explicitly exercising the app runtime. For facade removal, tests should usually provide the specific service layer they need.

If the test uses provideTmpdirInstance(...), remember that fixture needs a live ChildProcessSpawner layer. For services whose defaultLayer does not already provide that infra, prefer the repo-standard cross-spawn layer:

const infra = CrossSpawnSpawner.defaultLayer

const it = testEffect(Layer.mergeAll(MyService.defaultLayer, infra))

Without that extra layer, tests fail at runtime with Service not found: effect/process/ChildProcessSpawner.

Questions already answered

Do we need to effectify the whole caller first?

No.

• route files: compose the handler with AppRuntime.runPromise(Effect.gen(...))
• CLI and scripts: use AppRuntime.runPromise(Service.use(...))
• bootstrap: use the existing bootstrap runtime

Facade removal does not require a bigger refactor than that.

Should tests keep calling the namespace from async test bodies?

No. Convert them now.

The end state is yield* svc.method(...), not await Namespace.method(...) inside async tests.

Should we keep runPromise exported for convenience?

No. For this batch the goal is to delete the service-local runtime entirely.

What if a route has websocket callbacks or nested async handlers?

Keep the route shape, but replace each facade call with AppRuntime.runPromise(Service.use(...)) or wrap the surrounding async section in one Effect.gen(...) when practical. Do not keep the service facade just because the route has callback-shaped code.

Should we use one runPromise per service call?

No.

Default to one contiguous AppRuntime.runPromise(Effect.gen(...)) block per handler, command, or workflow. Yield every service you need inside that block.

Multiple tiny runPromise(...) calls are only acceptable when the caller structure forces it, such as websocket lifecycle callbacks, external callback APIs, or genuinely unrelated one-off operations.

Should we wrap a single service expression in Effect.gen(...)?

Usually no.

Prefer the direct form when there is only one expression:

await Effect.runPromise(FileSystem.Service.use((svc) => svc.read({ path })))

Use Effect.gen(...) when the workflow actually needs multiple yielded values or branching.

Learnings

These were the recurring mistakes and useful corrections from the first two batches:

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. 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.

Remaining work

Most of the original facade-removal backlog is already done. The practical remaining work is narrower now:

1. remove the Npm runtime-backed facade from src/npm/index.ts
2. remove the TuiConfig runtime-backed facade from src/cli/cmd/tui/config/tui.ts

Checklist

• src/npm/index.ts (Npm) - still exports runtime-backed async facade helpers on top of Npm.Service
• src/cli/cmd/tui/config/tui.ts (TuiConfig) - still exports runtime-backed async facade helpers on top of TuiConfig.Service
• src/session/session.ts / src/session/prompt.ts / src/session/revert.ts / src/session/summary.ts - service-local facades removed
• src/agent/agent.ts (Agent) - service-local facades removed
• src/permission/index.ts (Permission) - service-local facades removed
• src/worktree/index.ts (Worktree) - service-local facades removed
• src/plugin/index.ts (Plugin) - service-local facades removed
• src/snapshot/index.ts (Snapshot) - service-local facades removed
• ../core/src/filesystem.ts (FileSystem) - legacy kirincode service removed
• src/lsp/index.ts (LSP) - facades removed and merged
• src/mcp/index.ts (MCP) - facades removed and merged
• src/config/config.ts (Config) - facades removed and merged
• src/provider/provider.ts (Provider) - facades removed and merged
• src/pty/index.ts (Pty) - facades removed and merged
• src/skill/index.ts (Skill) - facades removed and merged
• src/project/vcs.ts (Vcs) - facades removed and merged
• src/tool/registry.ts (ToolRegistry) - facades removed and merged
• src/auth/index.ts (Auth) - facades removed and merged

Excluded makeRuntime(...) sites

• src/bus/index.ts - core bus plumbing, not a normal facade-removal target.
• src/effect/cross-spawn-spawner.ts - runtime helper for ChildProcessSpawner, not a service namespace facade.