refactor(effect): raise declared failures with Effect.fail, not throw inside Effect.gen (E8)

packages/nikcli/src/server/httpapi/pty.ts

@@ -125,11 +125,13 @@ export namespace PtyHttpApi {
   export const ApiLive = HttpApiBuilder.layer(Api)
 
   /**
-   * Translate a `Pty.NotFoundError` to the declared 404 body. Anything
-   * else propagates as a defect — the service surfaces `never` on success
-   * channels for these handlers, so the only expected failure is "missing".
+   * Translate a `Pty.NotFoundError` to the declared 404 body. The parameter
+   * is the handler's declared failure type, so the `instanceof` guard is
+   * total today; it stays as the defect arm for the day the channel grows a
+   * second member, because mislabelling that member as a 404 is worse than
+   * a 500 that names it.
    */
-  const asNotFound = (cause: unknown): Effect.Effect<never, NotFoundErrorBody> => {
+  const asNotFound = (cause: Pty.NotFoundError): Effect.Effect<never, NotFoundErrorBody> => {
     if (cause instanceof Pty.NotFoundError) {
       return Effect.fail({
         name: "NotFoundError" as const,
@@ -139,14 +141,20 @@ export namespace PtyHttpApi {
     return Effect.die(cause)
   }
 
-  const catchNotFound = <A, E, R>(effect: Effect.Effect<A, E, R>) =>
-    effect.pipe(Effect.catch(asNotFound), Effect.catchDefect(asNotFound))
+  /**
+   * E8.2: the 404 arrives on the typed channel, so there is no defect arm.
+   * `handlers.get` / `handlers.update` `Effect.fail` a `Pty.NotFoundError`
+   * rather than `throw`ing it inside `Effect.gen`, which is what used to
+   * make a declared error reach this boundary as a defect.
+   */
+  const catchNotFound = <A, R>(effect: Effect.Effect<A, Pty.NotFoundError, R>) => effect.pipe(Effect.catch(asNotFound))
 
   /**
-   * Translate a `Pty.CreateError` to the declared 400 body. Anything else
-   * propagates as a defect.
+   * Translate a `Pty.CreateError` to the declared 400 body. `Pty.Error` is
+   * `CreateError` alone today, so the guard is total; it stays for the same
+   * reason `asNotFound`'s does.
    */
-  const asCreateError = (cause: unknown): Effect.Effect<never, CreateErrorBody> => {
+  const asCreateError = (cause: Pty.Error): Effect.Effect<never, CreateErrorBody> => {
     if (cause instanceof Pty.CreateError) {
       return Effect.fail({
         name: "PtyCreateError" as const,
@@ -159,8 +167,13 @@ export namespace PtyHttpApi {
     return Effect.die(cause)
   }
 
-  const catchCreateError = <A, E, R>(effect: Effect.Effect<A, E, R>) =>
-    effect.pipe(Effect.catch(asCreateError), Effect.catchDefect(asCreateError))
+  /**
+   * E8.2: `Pty.Service.create` already declares `Pty.Error` (`CreateError`)
+   * on its failure channel — `src/pty/index.ts` builds it in the `catch` of
+   * the `Effect.try` around `spawnPty`. Nothing produces it as a defect, so
+   * the defect arm this used to carry was dead compensation.
+   */
+  const catchCreateError = <A, R>(effect: Effect.Effect<A, Pty.Error, R>) => effect.pipe(Effect.catch(asCreateError))
 
   /**
    * Cast helpers — safe because `PtyCreateInput`/`PtyUpdateInput` are
@@ -190,7 +203,7 @@ export namespace PtyHttpApi {
         const pty = yield* Pty.Service
         const info = yield* pty.get(params.ptyID)
         if (!info) {
-          throw new Pty.NotFoundError({ message: "Session not found" })
+          return yield* Effect.fail(new Pty.NotFoundError({ message: "Session not found" }))
         }
         return info
       }).pipe(catchNotFound),
@@ -200,7 +213,7 @@ export namespace PtyHttpApi {
         const pty = yield* Pty.Service
         const info = yield* pty.update(params.ptyID, toPtyUpdateInput(payload))
         if (!info) {
-          throw new Pty.NotFoundError({ message: "Session not found" })
+          return yield* Effect.fail(new Pty.NotFoundError({ message: "Session not found" }))
         }
         return info
       }).pipe(catchNotFound),

packages/nikcli/src/session/prompt.ts

@@ -369,7 +369,13 @@ export namespace SessionPrompt {
   }
 
   export interface Interface {
-    assertNotBusy(sessionID: string): Effect.Effect<void>
+    /**
+     * Fails with `Session.BusyError` when the session already has a running
+     * turn. Declared on the typed channel (E8.1): a busy session is an
+     * expected 409, not a defect, so every caller — Effect-side or through
+     * the Promise bridge — sees it without a `catchDefect` arm.
+     */
+    assertNotBusy(sessionID: string): Effect.Effect<void, Session.BusyError>
     /**
      * Persist the user message (and optional tool permissions) without starting
      * the model loop. Used by `prompt_async` so clients can observe the message
@@ -2035,10 +2041,12 @@ NOTE: At any point in time through this workflow you should feel free to ask the
         Effect.gen(function* () {
           const match = (yield* PromptState.getServiceStateEffect())[sessionID]
           if (match)
-            throw new Session.BusyError({
-              sessionID,
-              message: "Session is busy",
-            })
+            return yield* Effect.fail(
+              new Session.BusyError({
+                sessionID,
+                message: "Session is busy",
+              }),
+            )
         }),
       admit: (input) => withInstanceContext(() => admit(input)),
       steerPending: (input) => withInstanceContext(() => steerPending(input)),

packages/nikcli/test/server/httpapi-pty.test.ts

@@ -1,6 +1,7 @@
 import { preserveTestEnv } from "../helpers/env"
 import { removeTestDir } from "../helpers/fs"
 import { afterAll, afterEach, describe, expect, it } from "bun:test"
+import { Cause, Effect } from "effect"
 import fs from "fs/promises"
 import os from "os"
 import path from "path"
@@ -14,6 +15,9 @@ preserveTestEnv(["NIKCLI_TEST_HOME", "NIKCLI_DISABLE_PROJECT_CONFIG"])
 const { Instance } = await import("@/project/instance")
 const { HttpApiBridge } = await import("@/server/httpapi/bridge")
 const { Server } = await import("@/server/server")
+const { Pty } = await import("@/pty")
+const { PtyHttpApi } = await import("@/server/httpapi/pty")
+const { runPromiseExitWithLayer, withCurrentInstance } = await import("@/effect")
 
 const projectDirs: string[] = []
 
@@ -101,6 +105,73 @@ describe("Pty HttpApi (Wave 4 Path B)", () => {
     expect(body).toBe(true)
   })
 
+  /**
+   * E8.2. `handlers.get` / `handlers.update` used to raise the declared
+   * `Pty.NotFoundError` with `throw` inside `Effect.gen`, so it reached the
+   * boundary as a *defect* and only became a 404 because `catchNotFound`
+   * carried an `Effect.catchDefect` arm alongside its typed one. The defect
+   * arm is gone; these assertions are what goes red if the `throw` returns,
+   * because a die is no longer recovered into the declared body.
+   */
+  it("handlers.get maps a missing session on the typed channel, with no defect", async () => {
+    const directory = await makeProjectDir()
+    await Instance.provide({
+      directory,
+      fn: async () => {
+        const exit = await runPromiseExitWithLayer(
+          Pty.defaultLayer,
+          withCurrentInstance(PtyHttpApi.handlers.get({ params: { ptyID: "pty_definitely_missing" } })),
+        )
+        expect(exit._tag).toBe("Failure")
+        if (exit._tag !== "Failure") return
+        expect(Cause.hasDies(exit.cause)).toBe(false)
+        expect(Cause.squash(exit.cause)).toEqual({
+          name: "NotFoundError",
+          data: { message: "Session not found" },
+        })
+      },
+    })
+  })
+
+  it("handlers.update maps a missing session on the typed channel, with no defect", async () => {
+    const directory = await makeProjectDir()
+    await Instance.provide({
+      directory,
+      fn: async () => {
+        const exit = await runPromiseExitWithLayer(
+          Pty.defaultLayer,
+          withCurrentInstance(
+            PtyHttpApi.handlers.update({
+              params: { ptyID: "pty_definitely_missing" },
+              payload: { title: "renamed" },
+            }),
+          ),
+        )
+        expect(exit._tag).toBe("Failure")
+        if (exit._tag !== "Failure") return
+        expect(Cause.hasDies(exit.cause)).toBe(false)
+        expect(Cause.squash(exit.cause)).toEqual({
+          name: "NotFoundError",
+          data: { message: "Session not found" },
+        })
+      },
+    })
+  })
+
+  it("handlers.list stays total — a success carries no failure channel to map", async () => {
+    const directory = await makeProjectDir()
+    await Instance.provide({
+      directory,
+      fn: async () => {
+        const exit = await runPromiseExitWithLayer(
+          Pty.defaultLayer,
+          withCurrentInstance(Effect.map(PtyHttpApi.handlers.list(), (list) => list.length)),
+        )
+        expect(exit._tag).toBe("Success")
+      },
+    })
+  })
+
   it("POST /pty rejects a malformed payload with 400 (schema layer)", async () => {
     const directory = await makeProjectDir()
     const response = await request("POST", "/pty", directory, {

packages/nikcli/test/session/session-lifecycle.test.ts

@@ -424,6 +424,47 @@ describe("session lifecycle", () => {
       })
     })
 
+    it("SessionPrompt.assertNotBusy fails with SessionBusyError on the typed channel", async () => {
+      await withProject(async () => {
+        const { PromptState } = await import("../../src/session/prompt-state")
+        const { SessionPrompt } = await import("../../src/session/prompt")
+        const session = await createSession()
+
+        // Not reserved: the assertion is a plain success, not an absent
+        // failure that happens to be swallowed somewhere.
+        const idle = await runPromiseExitWithLayer(
+          SessionPrompt.defaultLayer,
+          withCurrentInstance(
+            Effect.gen(function* () {
+              const prompt = yield* SessionPrompt.Service
+              return yield* prompt.assertNotBusy(session.id)
+            }),
+          ),
+        )
+        expect(idle._tag).toBe("Success")
+
+        PromptState.reserve(session.id)
+        const exit = await runPromiseExitWithLayer(
+          SessionPrompt.defaultLayer,
+          withCurrentInstance(
+            Effect.gen(function* () {
+              const prompt = yield* SessionPrompt.Service
+              return yield* prompt.assertNotBusy(session.id)
+            }),
+          ),
+        )
+        expect(exit._tag).toBe("Failure")
+        if (exit._tag !== "Failure") return
+        // E8.1. The busy assertion used to `throw` inside `Effect.gen`, which
+        // is a defect: it only reached callers typed because `SessionRevert`
+        // ran it through `runPromiseWithLayer` and re-mapped the rejection.
+        // `hasDies` is what separates the two, so this assertion is the one
+        // that goes red if the `throw` comes back.
+        expect(Cause.hasDies(exit.cause)).toBe(false)
+        expect(Cause.squash(exit.cause)).toBeInstanceOf(Session.BusyError)
+      })
+    })
+
     it("SessionSummary.diff rejects with SessionNotFoundError on a typed failure channel", async () => {
       await withProject(async () => {
         const { SessionSummary } = await import("../../src/session/summary")