Skip to content

refactor(cli): convert run command to effectCmd#25519

Merged
kitlangton merged 2 commits into
devfrom
kit/cli-effectify-run
May 3, 2026
Merged

refactor(cli): convert run command to effectCmd#25519
kitlangton merged 2 commits into
devfrom
kit/cli-effectify-run

Conversation

@kitlangton
Copy link
Copy Markdown
Contributor

Summary

Stage 3 of the run.ts conversion (Stage 1 is #25517 — function-form `instance`).

  • `cmd()` → `effectCmd({ instance: (args) => !args.attach, directory: ... })`
    • `--attach` path: no instance load (matches legacy — legacy didn't load either; SDK talks to the remote server)
    • default path: instance loaded by `effectCmd` via `InstanceStore.provide`, auto-disposed on exit (matches legacy `bootstrap()` finally)
  • Drop the `bootstrap(process.cwd(), async () => {...})` wrapper since `effectCmd` handles it now.
  • The entire legacy handler body is wrapped in `Effect.promise()` inside `Effect.fn` for byte-equivalent behavior. `process.exit(1)` paths and the internal `AppRuntime.runPromise(Agent.Service.use(...))` agent-validation call all preserved as-is.

Also extends `effectCmd` to type `handler` args as `WithDoubleDash` so `args["--"]` (yargs's "everything after `--` separator") works without casts. Matches the legacy `cmd()` wrapper's shape.

Why this conversion is safe

The `Effect.promise(async () => { ... })` wrap means the handler body executes as a Promise inside the Effect runtime — same async semantics as legacy `async (args) => { ... }`. No behavior change to:

  • arg parsing, file resolution, validation, `process.exit(1)` paths
  • session/share/execute/loop closures
  • agent validation (`sdk.app.agents` for `--attach`, `AppRuntime.runPromise(Agent.Service.use)` for default)
  • SDK construction (`createOpencodeClient` with remote URL or local fetch)

Stacked on #25517

Depends on the function-form `instance` opt-out landing first.

Test plan

  • `bun run typecheck`
  • `bun run test test/cli/` — 137 pass
  • `run` (no args) → `Error: You must provide a message or a command` exit 1
  • `run --fork "hi"` → `Error: --fork requires --continue or --session` exit 1
  • `run --file /tmp/nonexistent.txt "hi"` → `Error: File not found: ...` exit 1
  • `run --attach http://127.0.0.1:1 "hi"` → connects, no instance load, fails gracefully → `Error: Session not found` exit 1

kitlangton added 2 commits May 2, 2026 22:31
Stage 3 of the run.ts conversion: wrap-in-Effect.promise approach.

- cmd() → effectCmd({ instance: (args) => !args.attach, directory: ... })
  - --attach path: no instance load (matches legacy — legacy didn't load
    either; SDK talks to the remote server)
  - default path: instance loaded by effectCmd via InstanceStore.provide,
    auto-disposed on exit (matches legacy bootstrap() finally)
- Drop the bootstrap(process.cwd(), async () => {...}) wrapper since
  effectCmd handles it
- Drop AppRuntime import (the inner AppRuntime.runPromise(Agent.Service.use)
  for agent validation still imports it via the Effect... wait no, that
  call is on the !args.attach path where instance is loaded)
- The whole legacy handler body is wrapped in Effect.promise() inside
  Effect.fn for byte-equivalent behavior. Process.exit(1) paths and
  internal AppRuntime.runPromise calls all preserved as-is.

Also extends effectCmd to accept WithDoubleDash<Args> on the handler so
args["--"] works without casts (matches the legacy cmd() wrapper shape).
- Export WithDoubleDash from cmd.ts and import in effect-cmd.ts (was
  redefined). Single source of truth.
- Drop the "Default path: effectCmd already loaded the instance..."
  comment in run.ts — effectCmd's docstring already explains this.
@kitlangton kitlangton force-pushed the kit/cli-effectify-run branch from 5891431 to 809cc4e Compare May 3, 2026 02:31
@kitlangton kitlangton merged commit 7409dcc into dev May 3, 2026
9 checks passed
@kitlangton kitlangton deleted the kit/cli-effectify-run branch May 3, 2026 02:35
oleksii-honchar pushed a commit to oleksii-honchar/better-opencode that referenced this pull request May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant