Skip to content

Commit 4bfd61c

Browse files
committed
fix(cli): propagate CliError through global FormatError instead of process.exit
Three review findings addressed: 1. process.exit(1) inside Effect.sync killed the process before the AppRuntime fiber's scope could close — file handles, plugin disposers, etc. would leak. Now CliError propagates as a normal Effect failure; the existing top-level try/catch in src/index.ts handles formatting + exit after the runtime settles. 2. CliError now hits the global FormatError pipeline (cli/error.ts) rather than bypassing it. Adds a CliError branch that respects the optional exitCode field via process.exitCode. 3. effectCmd's generic signature tightened to cmd<{}, Args>(...); the args cast is unavoidable because yargs wraps the type in ArgumentsCamelCase<WithDoubleDash<...>>. Localized to one cast at the boundary with a comment. Smoke tested: - bun run dev models — happy path ok - bun run dev models nonexistent — formatted error, exit 1 Followup noted inline in models.ts: lift ModelsDev into an Effect Service so the Effect.promise wrap drops out.
1 parent edb2852 commit 4bfd61c

3 files changed

Lines changed: 28 additions & 22 deletions

File tree

packages/opencode/src/cli/cmd/models.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export const ModelsCommand = effectCmd({
2626
}),
2727
handler: Effect.fn("Cli.models")(function* (args) {
2828
if (args.refresh) {
29+
// followup: lift ModelsDev into an Effect Service so this drops the Effect.promise wrap.
2930
yield* Effect.promise(() => ModelsDev.refresh(true))
3031
UI.println(UI.Style.TEXT_SUCCESS_BOLD + "Models cache refreshed" + UI.Style.TEXT_NORMAL)
3132
}

packages/opencode/src/cli/effect-cmd.ts

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ import { Effect, Schema } from "effect"
33
import { AppRuntime, type AppServices } from "@/effect/app-runtime"
44
import { InstanceStore } from "@/project/instance-store"
55
import { cmd } from "./cmd/cmd"
6-
import { UI } from "./ui"
76

87
/**
9-
* User-visible command failure. Use `fail("...")` from a handler to surface a
10-
* non-zero exit with a printed message. Anything else escapes as a defect and
11-
* the runtime prints the cause.
8+
* User-visible command failure. Throw via `fail("...")` from an effectCmd handler
9+
* to surface a printed message + non-zero exit. Recognised by the global error
10+
* formatter in `src/cli/error.ts` (FormatError), so the existing top-level
11+
* catch + cleanup in `src/index.ts` runs normally.
1212
*/
1313
export class CliError extends Schema.TaggedErrorClass<CliError>()("CliError", {
1414
message: Schema.String,
@@ -18,14 +18,16 @@ export class CliError extends Schema.TaggedErrorClass<CliError>()("CliError", {
1818
export const fail = (message: string, exitCode = 1) => Effect.fail(new CliError({ message, exitCode }))
1919

2020
/**
21-
* Effect-native CLI command builder. Wraps yargs `cmd()` with:
22-
* - `InstanceStore.provide` so `InstanceRef` is available inside the handler
23-
* - `AppRuntime.runPromise` so any AppServices can be yielded directly
24-
* - `CliError` interception so domain failures translate to a clean exit
21+
* Effect-native CLI command builder. Wraps yargs `cmd()` so the handler body is
22+
* an `Effect` with `InstanceRef` provided and any `AppServices` yieldable.
2523
*
26-
* The handler is typically an `Effect.fn("Cli.<name>")(function*(args){...})` value,
27-
* which gives each CLI run a named tracing span automatically. Eventually `cmd()` can
28-
* swap to effect/cli's `Command.make(...)` without touching the handler bodies.
24+
* Errors propagate to the existing top-level handler in `src/index.ts`; use
25+
* `fail("...")` for user-visible domain failures (clean exit, formatted message).
26+
*
27+
* Handlers are typically `Effect.fn("Cli.<name>")(function*(args) { ... })`,
28+
* which adds a named tracing span per CLI invocation. Once all commands use
29+
* `effectCmd`, swapping the underlying `cmd()` factory for effect/cli's
30+
* `Command.make(...)` won't touch any handler bodies.
2931
*/
3032
export const effectCmd = <Args, A>(opts: {
3133
command: string | readonly string[]
@@ -35,20 +37,16 @@ export const effectCmd = <Args, A>(opts: {
3537
directory?: (args: Args) => string
3638
handler: (args: Args) => Effect.Effect<A, CliError, AppServices | InstanceStore.Service>
3739
}) =>
38-
cmd<unknown, Args>({
40+
cmd<{}, Args>({
3941
command: opts.command,
4042
describe: opts.describe,
4143
builder: opts.builder as never,
42-
async handler(args) {
43-
const directory = opts.directory?.(args as Args) ?? process.cwd()
44-
const program = InstanceStore.Service.use((s) => s.provide({ directory }, opts.handler(args as Args))).pipe(
45-
Effect.catchTag("CliError", (e) =>
46-
Effect.sync(() => {
47-
UI.error(e.message)
48-
process.exit(e.exitCode ?? 1)
49-
}),
50-
),
44+
async handler(rawArgs) {
45+
// yargs typing wraps Args in ArgumentsCamelCase<WithDoubleDash<...>>; cast at the boundary.
46+
const args = rawArgs as unknown as Args
47+
const directory = opts.directory?.(args) ?? process.cwd()
48+
await AppRuntime.runPromise(
49+
InstanceStore.Service.use((s) => s.provide({ directory }, opts.handler(args))),
5150
)
52-
await AppRuntime.runPromise(program)
5351
},
5452
})

packages/opencode/src/cli/error.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ function isTaggedError(error: unknown, tag: string): boolean {
1515
}
1616

1717
export function FormatError(input: unknown) {
18+
// CliError: domain failure surfaced from an effectCmd handler via fail("...")
19+
if (isTaggedError(input, "CliError")) {
20+
const data = input as ErrorLike & { exitCode?: number }
21+
if (data.exitCode != null) process.exitCode = data.exitCode
22+
return data.message ?? ""
23+
}
24+
1825
// MCPFailed: { name: string }
1926
if (NamedError.hasName(input, "MCPFailed")) {
2027
return `MCP server "${(input as ErrorLike).data?.name}" failed. Note, opencode does not support MCP authentication yet.`

0 commit comments

Comments
 (0)