From 864e86c985551e0b31098b4e1c6a5ce010dad116 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 18:22:49 +0000 Subject: [PATCH 1/3] refactor(effect): raise declared failures with Effect.fail, not throw inside Effect.gen (E6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E5 closed the session HTTP boundary but recorded one remainder it did not claim: `SessionPrompt.assertNotBusy` was declared `Effect.Effect` and raised `Session.BusyError` with `throw` inside `Effect.gen`, so it reached `SessionRevert`'s callers typed only by accident of the Promise bridge. A sweep for that shape found one other live site. `httpapi/pty.ts` threw the declared `Pty.NotFoundError` inside `Effect.gen` in `handlers.get` and `handlers.update`, so `GET /pty/:id` answered 404 rather than 500 only because `catchNotFound` carried an `Effect.catchDefect` arm beside its typed one — the same compensation E5 removed from `httpapi/session.ts`. `catchCreateError` carried the same pair, and there the defect arm was dead: `Pty.Service.create` builds `CreateError` in the `catch` of the `Effect.try` around `spawnPty`, so it is always typed. Both now fail on the typed channel. `catchNotFound` / `catchCreateError` narrow their input to the declared error type and drop the defect arm, so a re-introduced `throw` is a visible 500 instead of being silently absorbed. No HTTP wire change. The existing route tests still pin the 404 body and the busy revert as `Session.BusyError`; the added service-level assertions read `Cause.hasDies === false`, which is what separates a fail from a die and therefore what goes red if a `throw` comes back. specs/ROADMAP.md admits this as E6 with its own acceptance gate rather than reopening E5's met one, and marks E5's caveat closed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01C76KUCD7BVi9bB9brPDsjP --- packages/nikcli/src/server/httpapi/pty.ts | 41 +++++++---- packages/nikcli/src/session/prompt.ts | 18 +++-- .../nikcli/test/server/httpapi-pty.test.ts | 71 +++++++++++++++++++ .../test/session/session-lifecycle.test.ts | 41 +++++++++++ specs/ROADMAP.md | 19 ++++- 5 files changed, 170 insertions(+), 20 deletions(-) diff --git a/packages/nikcli/src/server/httpapi/pty.ts b/packages/nikcli/src/server/httpapi/pty.ts index 6c9b2f72d..a22b7696a 100644 --- a/packages/nikcli/src/server/httpapi/pty.ts +++ b/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 => { + const asNotFound = (cause: Pty.NotFoundError): Effect.Effect => { if (cause instanceof Pty.NotFoundError) { return Effect.fail({ name: "NotFoundError" as const, @@ -139,14 +141,21 @@ export namespace PtyHttpApi { return Effect.die(cause) } - const catchNotFound = (effect: Effect.Effect) => - effect.pipe(Effect.catch(asNotFound), Effect.catchDefect(asNotFound)) + /** + * E6.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 = (effect: Effect.Effect) => + 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 => { + const asCreateError = (cause: Pty.Error): Effect.Effect => { if (cause instanceof Pty.CreateError) { return Effect.fail({ name: "PtyCreateError" as const, @@ -159,8 +168,14 @@ export namespace PtyHttpApi { return Effect.die(cause) } - const catchCreateError = (effect: Effect.Effect) => - effect.pipe(Effect.catch(asCreateError), Effect.catchDefect(asCreateError)) + /** + * E6.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 = (effect: Effect.Effect) => + effect.pipe(Effect.catch(asCreateError)) /** * Cast helpers — safe because `PtyCreateInput`/`PtyUpdateInput` are @@ -190,7 +205,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 +215,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), diff --git a/packages/nikcli/src/session/prompt.ts b/packages/nikcli/src/session/prompt.ts index 4143a5a39..8b2d94a1f 100644 --- a/packages/nikcli/src/session/prompt.ts +++ b/packages/nikcli/src/session/prompt.ts @@ -369,7 +369,13 @@ export namespace SessionPrompt { } export interface Interface { - assertNotBusy(sessionID: string): Effect.Effect + /** + * Fails with `Session.BusyError` when the session already has a running + * turn. Declared on the typed channel (E6.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 /** * 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)), diff --git a/packages/nikcli/test/server/httpapi-pty.test.ts b/packages/nikcli/test/server/httpapi-pty.test.ts index b9150218b..a14efb1e9 100644 --- a/packages/nikcli/test/server/httpapi-pty.test.ts +++ b/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) }) + /** + * E6.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, { diff --git a/packages/nikcli/test/session/session-lifecycle.test.ts b/packages/nikcli/test/session/session-lifecycle.test.ts index a8ab42b69..35c1849a8 100644 --- a/packages/nikcli/test/session/session-lifecycle.test.ts +++ b/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 + // E6.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") diff --git a/specs/ROADMAP.md b/specs/ROADMAP.md index 59eff4c32..8915ea7d5 100644 --- a/specs/ROADMAP.md +++ b/specs/ROADMAP.md @@ -40,6 +40,7 @@ The **E4 service-side slices landed** (2026-08-19): `Session.Info` and every `Me | **R1** | Done | Keyed scoped instance runtime (2026-08-26) | | **T3** | Done | Output codecs on `todowrite` / `todoread` / `browser_control` (2026-08-24) | | **P3** | Done | `normalizeMessages` characterized; kept as-is on the measurement (2026-08-24) | +| **E6** | Done | Declared failures raised with `Effect.fail`, not `throw` inside `Effect.gen` | ### Release integrity (C1) — landed 2026-08-23 @@ -149,7 +150,7 @@ These are evidenced leftovers, not product ideas. `Now` items are independent an ### Execute next -- **Order** — Nothing is queued. E5, P2 and H8 landed on 2026-08-24; the three `Later` items below each state the coverage they wait on. +- **Order** — Nothing is queued. E5, P2 and H8 landed on 2026-08-24; E6 closed E5's own recorded caveat on 2026-08-26; the three `Later` items below each state the coverage they wait on. - **H8.1 — Put auth on the contract.** Landed 2026-08-24; see the dated log. - **P2.1 — Push list work into SQL.** Landed 2026-08-24, measured below. P2.2 is now unblocked, but it is a separate decision: read the measurement before scheduling it. - **P2.2 — Decided 2026-08-24 against the measurement.** The logging policy landed; the parsed-URL carry-through is **rejected** and the benches are **not scheduled**. Reasoning and numbers in the dated log below. @@ -160,13 +161,27 @@ These are evidenced leftovers, not product ideas. `Now` items are independent an - **Evidence** — `httpapi/session.ts` applied `Effect.catchDefect(asSessionError)` after mapping the typed failure channel, `SessionRevert.Interface` and the route-facing `SessionSummary` methods exposed `unknown`, and both modules used the untyped `Effect.tryPromise(() => ...)` form, so `Effect.tryPromise` wrapped `SessionNotFoundError` in `UnknownError` and missing-session revert and diff answered 500. `Session.BusyError` was already a `Schema.TaggedErrorClass`, so the contract vocabulary existed. - **Implementation** — E5.2 / E5.3 landed in `ff061973ec`: `Session.asSessionError` is exported, `SessionRevert.Interface` and `SessionSummary.summarize` / `diff` carry `Session.Error`, and the domain-rejecting handler bridges (`MessageV2.get` ×2, `SessionContext.breakdown` ×2, `SessionV2.entries`, `Monitor.get` / `readLog` / `cancel`) use `Effect.tryPromise({ catch: Session.asSessionError })`. `computeDiff` keeps `unknown` — it is real dependency I/O. E5.1 / E5.4 closed it: `declaredErrors` is a single `Effect.catch(asSessionError)`, and the `background` handler dropped its defect arm. The ten remaining `Effect.promise` sites are the audited unknown-I/O set — `Array.fromAsync`, the two session-delete cancels, the `collectSystemPaths` import and call, and the four `Delegation` job routes — and stay on `orDie`. The one `catchDefect` left in the file is the best-effort MCP toggle log, which swallows both channels on purpose and is not part of this boundary. - **Caveat for the next reader.** `SessionPrompt.assertNotBusy` is still declared `Effect.Effect` and raises by `throw` inside `Effect.gen`. It reaches callers typed only because `SessionRevert` runs it through `runPromiseWithLayer` and re-maps the rejection with `Session.asSessionError`; the busy assertion below pins that behavior. Narrowing that signature to `Session.BusyError` with an explicit `Effect.fail` is a separate cleanup, not a reopening of E5. + **Caveat for the next reader — closed 2026-08-26 as E6.1.** `SessionPrompt.assertNotBusy` was declared `Effect.Effect` and raised by `throw` inside `Effect.gen`. It reached callers typed only because `SessionRevert` ran it through `runPromiseWithLayer` and re-mapped the rejection with `Session.asSessionError`; the busy assertion below pins that behavior. Narrowing that signature to `Session.BusyError` with an explicit `Effect.fail` was called a separate cleanup rather than a reopening of E5, and it is one — see E6. **Corrected 2026-08-18 for `loop.ts` / `mission.ts`.** Both already carry the typed channel: declared 404/400 schemas plus `failNotFound` / `failValidation`, and their managers use the return-`undefined` convention the handlers already check. The `fromPromise` `orDie` wraps genuine I/O, not domain errors. There are no `Engine.LoopNotFoundError` / `MissionNotFoundError` / `MissionAlreadyExistsError` tags to fail with — an earlier draft of this item invented them. The one real gap there is fixed (see landed work). - **Depends on** — nothing. H4 landed, so there was already one boundary to fix. H8 waited for this typed vocabulary and no longer does. - **Done when** — Met. Domain methods map `Session.Error` on the typed channel; return-`undefined` plus an explicit `Effect.fail` remains valid for loop/mission. Session handlers map schema-declared errors without `catchDefect`, and `Effect.promise` / `orDie` remains only for genuinely unknown I/O. The service-level assertions in `test/session/session-lifecycle.test.ts` assert `Cause.hasDies === false` before squashing, so they separate `Effect.fail` from `Effect.die` instead of reading through both; route tests separately pin the unchanged 404/409 wire bodies. +### Declared failures on the typed channel (E6) — landed 2026-08-26 + +E5 closed the session HTTP boundary: declared errors are mapped from the typed channel and `catchDefect(asSessionError)` is gone. It recorded one remainder in its own text and did not claim it. E6 is that remainder plus the one other place in `src` with the same shape. It is a separate item because E5's acceptance gate is about the session boundary, and reopening a met gate to absorb new work is how a gate stops meaning anything. + +- **Buys** — A declared domain error is a failure, not a defect, at the point it is raised. A handler that maps it needs one arm, not two, and a caller that forgets to map it gets a type error rather than a 500. +- **Evidence** — Two sites, both in the repository at the time of writing: + - `session/prompt.ts` declared `assertNotBusy(sessionID): Effect.Effect` and raised `Session.BusyError` with `throw` inside `Effect.gen`. `Session.BusyError` is already a `Schema.TaggedErrorClass` and already has a declared 409 body (`httpapi/session.ts`), so the vocabulary existed; only the signature and the raise did not use it. It reached `SessionRevert`'s callers typed purely by accident of the Promise bridge — `runPromiseWithLayer` rejects with the squashed cause and `Session.asSessionError` passes a `BusyError` straight through. + - `server/httpapi/pty.ts` raised the declared `Pty.NotFoundError` with `throw` inside `Effect.gen` in `handlers.get` and `handlers.update`, so the only reason `GET /pty/:id` answered 404 rather than 500 was that `catchNotFound` carried `Effect.catchDefect(asNotFound)` beside its typed arm. `catchCreateError` carried the same pair, and there its defect arm was dead: `Pty.Service.create` builds `CreateError` in the `catch` of an `Effect.try` around `spawnPty`, so it is always typed. +- **Implementation** — `assertNotBusy` is `Effect.Effect` and fails with `Effect.fail`. The pty handlers fail with `Effect.fail`, and `catchNotFound` / `catchCreateError` narrow their input to `Pty.NotFoundError` / `Pty.Error` and drop the `catchDefect` arm — so a re-introduced `throw` is a 500, visibly, instead of being silently absorbed. +- **Depends on** — E5, for the vocabulary and for the `Cause.hasDies` assertion style that separates a `fail` from a `die`. +- **Done when** — Met 2026-08-26. No HTTP wire change: `test/server/httpapi-pty.test.ts` still pins the 404 body byte-for-byte, and `test/session/session-lifecycle.test.ts` still pins the busy revert as `Session.BusyError`. Service-level assertions read `Cause.hasDies === false` on `SessionPrompt.assertNotBusy` and on both pty handlers, which is the check that goes red if the `throw` comes back — with the defect arms removed there is nothing left to recover it. + + **What E6 deliberately does not cover.** The `throw`s in `codemode/interpreter/` are not this: the interpreter unwinds by throwing `InterpreterRuntimeError` on purpose and catches it at its own boundary (`codemode/interpreter/errors.ts` matches on it twice), which is a design, not a leak. `session/prompt-commands.ts` throws `Session.BusyError` from an `async` function, which is the Promise side of the bridge and is the normal way to reject there. `httpapi/session.ts:908` throws a bare `Error` on a part-id mismatch; turning that into a declared 400 is a wire change (500 today) and needs its own item with a regenerated client, so it is not folded in here. + ### Request-path cuts (P2) - **Buys** — Encoded JSON requests stop paying for work the contract already did. Hot polls (`/event`, `/session/status`, TUI) stop dominating logs and extra SQL. From c93b93342e459264e7f8fa824b58fd0605266c8a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 19:05:32 +0000 Subject: [PATCH 2/3] docs(roadmap): record E6's landing, its mutation evidence, and one pre-existing red gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the dated landing log for E6 with the verified counts, and applies prettier to the two combinators whose signatures changed. The mutation run is the part worth keeping: restoring the two `throw`s turns exactly three tests red, and one of them is the *pre-existing* pty 404 route test. That is what makes the claim behavioural rather than stylistic — with the defect arm removed, a re-introduced `throw` answers 500, not 404, so the raise and the arm removal have to land together. Also records, without fixing, that `generate:httpapi-clients` reorders the Event declarations in the generated SDK types by 29 lines. It reproduces with the E6 source reverted, so it is pre-existing; C1 makes generated drift blocking, so it wants its own item rather than being buried in this diff. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01C76KUCD7BVi9bB9brPDsjP --- packages/nikcli/src/server/httpapi/pty.ts | 6 ++---- specs/ROADMAP.md | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/packages/nikcli/src/server/httpapi/pty.ts b/packages/nikcli/src/server/httpapi/pty.ts index a22b7696a..0a2a7bd88 100644 --- a/packages/nikcli/src/server/httpapi/pty.ts +++ b/packages/nikcli/src/server/httpapi/pty.ts @@ -147,8 +147,7 @@ export namespace PtyHttpApi { * 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 = (effect: Effect.Effect) => - effect.pipe(Effect.catch(asNotFound)) + const catchNotFound = (effect: Effect.Effect) => effect.pipe(Effect.catch(asNotFound)) /** * Translate a `Pty.CreateError` to the declared 400 body. `Pty.Error` is @@ -174,8 +173,7 @@ export namespace PtyHttpApi { * the `Effect.try` around `spawnPty`. Nothing produces it as a defect, so * the defect arm this used to carry was dead compensation. */ - const catchCreateError = (effect: Effect.Effect) => - effect.pipe(Effect.catch(asCreateError)) + const catchCreateError = (effect: Effect.Effect) => effect.pipe(Effect.catch(asCreateError)) /** * Cast helpers — safe because `PtyCreateInput`/`PtyUpdateInput` are diff --git a/specs/ROADMAP.md b/specs/ROADMAP.md index 8915ea7d5..14aea8ec1 100644 --- a/specs/ROADMAP.md +++ b/specs/ROADMAP.md @@ -682,6 +682,20 @@ P2.2 was queued behind "only after P2.1 records its result". It did, so this is **One cast worth not having.** The middleware layer was first written as `Layer.succeed(Middleware, { … } as never)`. A cast to `never` silences every check, so it came out before the commit, and the typecheck confirms it was never needed: `Layer.succeed` types its value from the service key, which instantiates `authorize`'s generic at `HttpServerResponse` and collapses its two return branches. With the cast in place that would have been unknowable — which is the argument against writing it in the first place. +### 2026-08-26 — E6 (declared failures on the typed channel) + +E5's own text recorded a remainder it did not claim, and a sweep for that shape found one more. Both are closed here. The sweep is worth describing because "grep for `throw`" does not answer the question: what matters is a `throw` whose *nearest enclosing* function is an `Effect.gen` body, so the search brace-matched each `Effect.gen(function* () {` and reported the throws inside it. That found 57 occurrences across `src`, of which all but three are `codemode/interpreter/`, where throwing is the interpreter's own unwind mechanism and `codemode/interpreter/errors.ts` matches on `InterpreterRuntimeError` at the boundary. The three that were not that: `session/prompt.ts` (E6.1), `httpapi/pty.ts` ×2 (E6.2), and `httpapi/session.ts:908`, which is left alone because fixing it is a 500→400 wire change and needs its own item. + +**The pty finding is the one with a user-visible consequence.** `handlers.get` and `handlers.update` threw the declared `Pty.NotFoundError` inside `Effect.gen`, so it arrived at the boundary as a defect, and `GET /pty/:id` returned 404 only because `catchNotFound` paired `Effect.catchDefect(asNotFound)` with its typed arm. That is precisely the compensation E5 deleted from `httpapi/session.ts`. Removing the defect arm without moving the raise onto the typed channel would have turned every missing-pty lookup into a 500 — which is exactly what the mutation run below demonstrates, and why the two halves land together. + +**`catchCreateError`'s defect arm was dead.** `Pty.Service.create` builds `CreateError` in the `catch` of the `Effect.try` around `spawnPty`, so it is always typed; nothing in `src/pty/index.ts` throws at all. Both combinators now take the declared error type rather than `E`, so the compiler — not a second runtime arm — is what keeps the mapping total. + +**Verified by mutation, both ways.** Restoring the two `throw`s turns exactly three tests red and leaves the rest green: the two new typed-channel assertions, plus the **pre-existing** `GET /pty/:id ... returns 404 with the declared error body` route test, which is the one that proves the behavioural claim rather than describing it. Notably the busy-revert test stays green under mutation, and that is correct: it reaches its assertion through `runPromiseWithLayer`, which squashes a die and a fail alike — which is the whole reason E5 called the old signature typed only by accident. + +**Verified.** `bun run typecheck` clean on `packages/nikcli`. `bun test test/session/ test/server/` 780 pass / 1 fail across 90 files; the failure is `httpapi-top-level.test.ts` and is **not this change** — it fails identically with `session/prompt.ts` and `httpapi/pty.ts` reverted to their pre-E6 state. `bun run check:routes --strict` ok at 338 contracts / 315 handlers / 23 raw. `bunx oxlint` and `bunx prettier --check` clean on the four touched files. + +**One pre-existing gate is red and is not this item's to fix.** `bun run generate:httpapi-clients` produces a 29-insertion / 29-deletion reordering of the `Event*` declarations in `packages/sdk/js/src/httpapi/generated/types.ts` — same content, different order, deterministic across runs and reproducible with the E6 source reverted. C1 makes generated drift blocking, so this wants its own item: either the event union gets a stable sort in `packages/httpapi-codegen`, or the committed file is stale against the current generator. It is recorded rather than silently regenerated here, because committing a reordering into this change would bury it. + ## Follow working rules - Commit at phase boundaries, not per file. H4 and H5 land together. From 37529edc92254c7580c633a5f973e4298001349a Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 19:20:25 +0000 Subject: [PATCH 3/3] =?UTF-8?q?docs(roadmap):=20correct=20the=20drift=20cl?= =?UTF-8?q?aim=20=E2=80=94=20validate=20passes,=20it=20was=20a=20sandbox?= =?UTF-8?q?=20artifact?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The E8 landing log recorded the generated-client `Event*` reordering as a pre-existing red gate that needed its own item. That was wrong, and the correction matters because it would otherwise send someone to open an item for a check that is green. `validate` runs exactly that check — `generate:httpapi-clients` followed by `git diff --exit-code -- packages/sdk/js/src/httpapi/generated` — and it passes in CI on this commit. The committed file is correct. The reordering was real in the development sandbox and reproduced with the E8 source reverted, so it was never this change; the likely cause is an interrupted `bun install` there leaving a module graph in which a different set of event modules registered, and declaration order follows registration. Recorded as that rather than deleted, so the next person who regenerates locally and sees an uncaused diff knows to check `validate` first. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01C76KUCD7BVi9bB9brPDsjP --- specs/ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specs/ROADMAP.md b/specs/ROADMAP.md index 2698a3bb7..e0dbcbdfd 100644 --- a/specs/ROADMAP.md +++ b/specs/ROADMAP.md @@ -743,7 +743,7 @@ E5's own text recorded a remainder it did not claim, and a sweep for that shape **Verified.** `bun run typecheck` clean on `packages/nikcli`. `bun test test/session/ test/server/` 780 pass / 1 fail across 90 files; the failure is `httpapi-top-level.test.ts` and is **not this change** — it fails identically with `session/prompt.ts` and `httpapi/pty.ts` reverted to their pre-E8 state. `bun run check:routes --strict` ok at 338 contracts / 315 handlers / 23 raw. `bunx oxlint` and `bunx prettier --check` clean on the four touched files. -**One pre-existing gate is red and is not this item's to fix.** `bun run generate:httpapi-clients` produces a 29-insertion / 29-deletion reordering of the `Event*` declarations in `packages/sdk/js/src/httpapi/generated/types.ts` — same content, different order, deterministic across runs and reproducible with the E8 source reverted. C1 makes generated drift blocking, so this wants its own item: either the event union gets a stable sort in `packages/httpapi-codegen`, or the committed file is stale against the current generator. It is recorded rather than silently regenerated here, because committing a reordering into this change would bury it. +**A local-only regeneration difference, and why it is not drift.** Regenerating the HttpApi clients in the development sandbox reorders 29 `Event*` declarations in `packages/sdk/js/src/httpapi/generated/types.ts` — same content, different order. It was first recorded here as a pre-existing red gate, which was wrong: `validate` runs exactly that check (`generate:httpapi-clients` followed by `git diff --exit-code -- packages/sdk/js/src/httpapi/generated`) and it **passes** in CI on this commit. The committed file is correct. The reordering reproduced with the E8 source reverted, so it is not this change either — the likely cause is an interrupted `bun install` in that sandbox leaving a module graph in which a different set of event modules registered, and event declaration order follows registration. Worth knowing if you regenerate locally and see a diff you did not cause: check `validate` before opening an item for it. ## Follow working rules