feat(cli): add effectCmd wrapper + convert models command#25429
Merged
kitlangton merged 2 commits intodevfrom May 2, 2026
Merged
feat(cli): add effectCmd wrapper + convert models command#25429kitlangton merged 2 commits intodevfrom
kitlangton merged 2 commits intodevfrom
Conversation
Adds cli/effect-cmd.ts with:
- effectCmd({command, describe, builder, directory?, handler}) — wraps yargs
cmd() so the handler is an Effect, with InstanceRef provided, AppServices
yieldable, and a Cli.<name> tracing span via Effect.fn at the call site
- CliError + fail() helpers for clean exit-with-message paths
Converts cli/cmd/models.ts as the first end-to-end smoke test:
- Drops Instance.provide({fn}) ALS wrapper
- Drops AppRuntime.runPromise(Effect.gen(...)) per-call boundary crossing
- Yields Provider.Service once at the top, uses freely
- Replaces process.exit(1) with yield* fail('...')
- Uses Effect.fn('Cli.models') for traced span
Net diff: models.ts 88 → 66 lines; new helper 54 lines.
Smoke tested:
- bun run dev models — prints model list
- bun run dev models nonexistent-provider — prints error, exits 1
Path to effect/cli: command bodies are already Effect-typed. Once we
swap the cmd() factory for effect/cli's Command.make(), nothing in the
handler bodies has to change — only the args parser.
…ocess.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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First step toward Effect-native CLI commands. Adds an `effectCmd` wrapper that lets handler bodies be `Effect`s with services yieldable and tracing spans baked in — and converts `models.ts` as the first end-to-end smoke test.
Why
Today every CLI handler is the same Promise/ALS shape:
```ts
async handler(args) {
await Instance.provide({ // Promise wrapper, ALS bind
directory: process.cwd(),
async fn() {
const project = Instance.project // ALS read
await AppRuntime.runPromise( // Promise→Effect→Promise→Effect dance
Service.use((s) => s.method(...)) // for ONE call
)
if (...) process.exit(1) // ad-hoc exit
}
})
}
```
After:
```ts
handler: Effect.fn("Cli.")(function* (args) {
const ctx = yield* InstanceRef
const svc = yield* Service // yield once, use many
if (...) return yield* fail("message") // domain error → clean exit
...
})
```
What's in the wrapper
Path to effect/cli
`effectCmd` is a stepping stone. Once all 18 CLIs use it, swapping the underlying `cmd()` factory for `effect/cli`'s `Command.make(...)` doesn't touch the handler bodies — only the args parser changes (yargs `Argv` → effect/cli `Args.*`).
Numbers
Smoke tests
Followup
Convert remaining 17 CLIs (`pr`, `mcp`, `github`, `plug`, `agent`, `providers`, `debug/*`, `import`, `stats`, etc). They follow the same pattern; can be batched 4-5 per PR or swarmed by directory.