diff --git a/apps/server/src/coil/autoResume/guards.test.ts b/apps/server/src/coil/autoResume/guards.test.ts index 87ce1c7febd5..8056dd37d01f 100644 --- a/apps/server/src/coil/autoResume/guards.test.ts +++ b/apps/server/src/coil/autoResume/guards.test.ts @@ -1,5 +1,11 @@ import { describe, expect, it } from "vite-plus/test"; -import type { OrchestrationThread } from "@t3tools/contracts"; +import { + type OrchestrationThread, + type OrchestrationThreadShell, + ProjectId, + ProviderInstanceId, + ThreadId, +} from "@t3tools/contracts"; import { cancelReason, @@ -123,6 +129,48 @@ describe("simple predicates", () => { expect(isClaudeThread(makeThread({ providerName: null }))).toBe(false); }); + // A shell carries `session` with the same `OrchestrationSession | null` type the full + // thread does, so it satisfies the guard's `Pick<…, "session">` argument. + // + // Built with NO cast, deliberately: the cast-free literal is the only thing pinning the + // widening. Revert `isClaudeThread` to `(thread: OrchestrationThread)` and this file stops + // type-checking, which is the failure a shell-only caller (the loop supervisor reads the + // shell stream) would otherwise hit at build time instead of here. + it("isClaudeThread accepts a thread shell, not just a full thread", () => { + const shell: OrchestrationThreadShell = { + id: ThreadId.make("thread-1"), + projectId: ProjectId.make("project-1"), + title: "Thread", + modelSelection: { instanceId: ProviderInstanceId.make("claude"), model: "opus" }, + runtimeMode: "full-access", + interactionMode: "default", + branch: "feature", + worktreePath: "/repo", + latestTurn: null, + createdAt: "2026-09-01T00:00:00.000Z", + updatedAt: "2026-09-02T00:00:00.000Z", + archivedAt: null, + settledOverride: null, + settledAt: null, + session: { + threadId: ThreadId.make("thread-1"), + status: "ready", + providerName: "claudeAgent", + runtimeMode: "full-access", + activeTurnId: null, + lastError: null, + updatedAt: "2026-09-02T00:00:00.000Z", + }, + latestUserMessageAt: "2026-09-02T00:00:00.000Z", + hasPendingApprovals: false, + hasPendingUserInput: false, + hasActionableProposedPlan: false, + }; + + expect(isClaudeThread(shell)).toBe(true); + expect(isClaudeThread({ ...shell, session: null })).toBe(false); + }); + it("threadIsGone covers deleted / archived", () => { expect(threadIsGone(makeThread({ deletedAt: "2026-01-01" }))).toBe(true); expect(threadIsGone(makeThread({ archivedAt: "2026-01-01" }))).toBe(true); diff --git a/apps/server/src/coil/autoResume/guards.ts b/apps/server/src/coil/autoResume/guards.ts index ad6f8b6a7e9a..3bcaecf382ed 100644 --- a/apps/server/src/coil/autoResume/guards.ts +++ b/apps/server/src/coil/autoResume/guards.ts @@ -1,9 +1,11 @@ /** * Pure guard helpers for auto-resume. * - * All predicates operate on the authoritative `OrchestrationThread` read-model snapshot - * (the full-thread shape, which carries `messages` and `activities` — NOT the shell-only - * derived flags). Keeping them pure makes every guard trivially unit-testable. + * Predicates operate on the authoritative `OrchestrationThread` read-model snapshot (the + * full-thread shape, which carries `messages` and `activities` — NOT the shell-only derived + * flags). The one exception is `isClaudeThread`, which asks for only the field it reads so a + * caller holding a thread shell can reuse it. Keeping them pure makes every guard trivially + * unit-testable. * * @module coil/autoResume/guards */ @@ -91,11 +93,21 @@ function isStaleRequestFailureDetail(payload: Record | null): b /** * The thread's session is backed by the Claude driver. The driver slug is - * `"claudeAgent"` (ClaudeAdapter.ts:96); `session.providerName` is set from - * `event.provider` (ProviderRuntimeIngestion.ts:1442). + * `"claudeAgent"`; `session.providerName` is set from `event.provider` in + * `ProviderRuntimeIngestion`. */ export const CLAUDE_DRIVER_KIND = "claudeAgent"; -export function isClaudeThread(thread: OrchestrationThread): boolean { + +/** + * Takes the narrowest shape it reads rather than a whole `OrchestrationThread`, so a caller + * holding only an `OrchestrationThreadShell` can ask too — the shell declares `session` with + * the identical `OrchestrationSession | null` type, so it is structurally assignable and no + * second copy of this predicate has to exist. + * + * Every other guard in this module still takes the full thread, because they read `messages` + * and `activities`, which the shell does not carry. + */ +export function isClaudeThread(thread: Pick): boolean { const name = thread.session?.providerName; return typeof name === "string" && name.toLowerCase() === CLAUDE_DRIVER_KIND.toLowerCase(); } diff --git a/apps/server/src/coil/autoResume/http.test.ts b/apps/server/src/coil/autoResume/http.test.ts index ad7a2f80629e..d9902f438abc 100644 --- a/apps/server/src/coil/autoResume/http.test.ts +++ b/apps/server/src/coil/autoResume/http.test.ts @@ -3,145 +3,76 @@ * Route-level tests for `/api/coil/auto-resume`. * * Serves ONLY the fork's route layer (not the whole `makeRoutesLayer`) over a real HTTP - * server on an ephemeral port, with `EnvironmentAuth` mocked. + * server on an ephemeral port, with `EnvironmentAuth` mocked (see `coil/http/testAuth.ts`). * - * Scope note: because auth is mocked, these tests do NOT prove that the route's - * `authenticateWithOperateScope` faithfully mirrors upstream's private + * Scope note: because auth is mocked, these tests do NOT prove that the shared + * `coil/http/auth.ts` helper faithfully mirrors upstream's private * `authenticateRawRouteWithScope` — that remains a logic mirror tracked in SEAMS.md. - * What they DO prove is that a rejected credential and a missing scope each render as a - * 401/403 through the route's `catchTags` rather than escaping as a 500 or an unhandled - * defect, and that the GET/POST contract round-trips through a real store. + * What they DO prove is that a rejected credential and a missing scope each render through + * the route's `catchTags` rather than escaping as a 500 or an unhandled defect, and that the + * GET/POST contract round-trips through a real store. The helper's own bodies are pinned in + * `coil/http/auth.test.ts`. */ -import * as NodeHttpServer from "@effect/platform-node/NodeHttpServer"; -import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, describe, it } from "@effect/vitest"; import { AuthOrchestrationOperateScope } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; -import { - FetchHttpClient, - HttpClient, - HttpClientRequest, - HttpRouter, - HttpServer, -} from "effect/unstable/http"; -import * as NodeHttp from "node:http"; +import type { HttpClient, HttpServer } from "effect/unstable/http"; import * as NodePath from "node:path"; import * as EnvironmentAuth from "../../auth/EnvironmentAuth.ts"; +import { authFails, authOk, getJson, postJson, runServed, serveRoutes } from "../http/testAuth.ts"; import { autoResumeRouteLayer } from "./http.ts"; import { AutoResumeStore, makeAutoResumeStore } from "./state.ts"; const PATH = "/api/coil/auto-resume"; -const authedSession = (scopes: ReadonlyArray) => - ({ - sessionId: "test-session", - subject: "test", - method: "bearer-access-token", - scopes, - }) as unknown as EnvironmentAuth.AuthenticatedSession; - -/** Mock auth that succeeds with the given scopes. */ -const authOk = (scopes: ReadonlyArray = [AuthOrchestrationOperateScope]) => - Layer.mock(EnvironmentAuth.EnvironmentAuth)({ - authenticateHttpRequest: () => Effect.succeed(authedSession(scopes)), - }); - -/** - * Mock auth that rejects the credential. - * - * Must be a member of the `ServerAuthCredentialError` union - * (`ServerAuthMissingCredentialError | ServerAuthInvalidCredentialError`) — that union is - * what `isServerAuthCredentialError` guards on. An error outside it (and outside - * `ServerAuthInternalError`) matches neither `catchIf` branch and surfaces as a 500. That - * is equally true of upstream's OTLP route, which uses the identical two branches, so it - * is a shared property rather than a fork divergence. - */ -const authRejects = Layer.mock(EnvironmentAuth.EnvironmentAuth)({ - authenticateHttpRequest: () => - Effect.fail(new EnvironmentAuth.ServerAuthInvalidCredentialError({})), -}); - -const baseUrl = (pathname: string) => - Effect.gen(function* () { - const server = yield* HttpServer.HttpServer; - const address = server.address as HttpServer.TcpAddress; - return `http://127.0.0.1:${address.port}${pathname}`; - }); - -const getJson = (pathname: string) => - Effect.gen(function* () { - const client = yield* HttpClient.HttpClient; - return yield* client.get(yield* baseUrl(pathname)); - }); - -const postJson = (pathname: string, body: unknown) => - Effect.gen(function* () { - const client = yield* HttpClient.HttpClient; - const request = HttpClientRequest.bodyJsonUnsafe( - HttpClientRequest.post(yield* baseUrl(pathname)), - body, - ); - return yield* client.execute(request); - }); +const authRejects = authFails(new EnvironmentAuth.ServerAuthInvalidCredentialError({})); /** Serves the route over an ephemeral port with the given auth layer and a real store. */ const withRoute = ( - authLayer: Layer.Layer, + auth: Layer.Layer, body: Effect.Effect, -): Promise => - Effect.gen(function* () { - const fs = yield* FileSystem.FileSystem; - const root = yield* fs.makeTempDirectoryScoped({ prefix: "coil-http-" }); - const storeLayer = Layer.effect( - AutoResumeStore, - makeAutoResumeStore(NodePath.join(root, "state.json")), - ); - - const served = HttpRouter.serve(autoResumeRouteLayer, { - disableListenLog: true, - disableLogger: true, - }).pipe( - Layer.provide(storeLayer), - Layer.provide(authLayer), - // provideMerge, not provide: the test body needs HttpServer in context to read the - // ephemeral port off `server.address`. - Layer.provideMerge(NodeHttpServer.layer(() => NodeHttp.createServer(), { port: 0 })), - ); - - return yield* body.pipe(Effect.provide(Layer.merge(served, FetchHttpClient.layer))); - }).pipe(Effect.scoped, Effect.provide(NodeServices.layer), Effect.runPromise); +) => + runServed( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const root = yield* fs.makeTempDirectoryScoped({ prefix: "coil-http-" }); + const storeLayer = Layer.effect( + AutoResumeStore, + makeAutoResumeStore(NodePath.join(root, "state.json")), + ); + return yield* serveRoutes({ + routes: autoResumeRouteLayer, + deps: Layer.merge(storeLayer, auth), + body, + }); + }), + ); describe("/api/coil/auto-resume", () => { - it("rejects a bad credential with 401/403 instead of leaking a 500", () => + it("rejects a bad credential with a 401 instead of leaking a 500", () => withRoute( authRejects, Effect.gen(function* () { const res = yield* getJson(`${PATH}?threadId=thread-a`); - assert.isTrue( - res.status === 401 || res.status === 403, - `expected 401/403 for a rejected credential, got ${res.status}`, - ); + assert.strictEqual(res.status, 401); }), )); - it("rejects a session lacking the operate scope", () => + it("rejects a session lacking the operate scope with a 403", () => withRoute( authOk([]), Effect.gen(function* () { const res = yield* getJson(`${PATH}?threadId=thread-a`); - assert.isTrue( - res.status === 401 || res.status === 403, - `expected 401/403 without the operate scope, got ${res.status}`, - ); + assert.strictEqual(res.status, 403); }), )); it("GET defaults a never-seen thread to enabled", () => withRoute( - authOk(), + authOk([AuthOrchestrationOperateScope]), Effect.gen(function* () { const res = yield* getJson(`${PATH}?threadId=fresh`); assert.strictEqual(res.status, 200); @@ -154,7 +85,7 @@ describe("/api/coil/auto-resume", () => { it("GET without a threadId is a 400, not a crash", () => withRoute( - authOk(), + authOk([AuthOrchestrationOperateScope]), Effect.gen(function* () { const res = yield* getJson(PATH); assert.strictEqual(res.status, 400); @@ -163,7 +94,7 @@ describe("/api/coil/auto-resume", () => { it("POST patches one field without clobbering the other", () => withRoute( - authOk(), + authOk([AuthOrchestrationOperateScope]), Effect.gen(function* () { const off = yield* postJson(PATH, { threadId: "t1", enabled: false }); assert.strictEqual(off.status, 200); @@ -189,7 +120,7 @@ describe("/api/coil/auto-resume", () => { // next boot. Pinned here at the HTTP boundary, not just in the store. it("handles a prototype-chain threadId as an ordinary unknown thread", () => withRoute( - authOk(), + authOk([AuthOrchestrationOperateScope]), Effect.gen(function* () { for (const hostile of ["constructor", "__proto__", "toString"]) { const res = yield* getJson(`${PATH}?threadId=${encodeURIComponent(hostile)}`); @@ -207,7 +138,7 @@ describe("/api/coil/auto-resume", () => { it("POST with a malformed body is a 400, not a 500", () => withRoute( - authOk(), + authOk([AuthOrchestrationOperateScope]), Effect.gen(function* () { const res = yield* postJson(PATH, { nope: true }); assert.strictEqual(res.status, 400); diff --git a/apps/server/src/coil/autoResume/http.ts b/apps/server/src/coil/autoResume/http.ts index ab5e083d51a6..6ff818f74b6f 100644 --- a/apps/server/src/coil/autoResume/http.ts +++ b/apps/server/src/coil/autoResume/http.ts @@ -17,53 +17,15 @@ import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; -import { - HttpRouter, - HttpServerRequest, - HttpServerRespondable, - HttpServerResponse, -} from "effect/unstable/http"; +import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"; -import * as EnvironmentAuth from "../../auth/EnvironmentAuth.ts"; -import { - failEnvironmentAuthInvalid, - failEnvironmentInternal, - failEnvironmentScopeRequired, -} from "../../auth/http.ts"; +import { authenticateWithScope, routeAuthErrorTags } from "../http/auth.ts"; import { AutoResumeStore, type AutoResumeStoreShape } from "./state.ts"; export const AUTO_RESUME_ROUTE_PATH = "/api/coil/auto-resume"; -/** - * MIRROR of the module-private `authenticateRawRouteWithScope` in `apps/server/src/http.ts` - * (which the OTLP proxy route uses). It is not exported, so importing it would mean editing - * an upstream file; the fork replicates the ~15 lines instead. Registered as a logic mirror - * in docs/coil/SEAMS.md — if upstream changes how raw routes authenticate, this must follow. - * - * Operate (not read) scope: these endpoints mutate scheduling behaviour. - */ -const authenticateWithOperateScope = Effect.gen(function* () { - const request = yield* HttpServerRequest.HttpServerRequest; - const serverAuth = yield* EnvironmentAuth.EnvironmentAuth; - const session = yield* serverAuth.authenticateHttpRequest(request).pipe( - Effect.catchIf(EnvironmentAuth.isServerAuthCredentialError, (error) => - // Second argument tracks upstream's `authenticateRawRouteWithScope`, which - // this mirrors. `dpopFailureReason` is optional, so dropping it compiles — - // it just costs a relay client the precise reason (clock skew being the - // motivating case) that every other environment endpoint reports. - failEnvironmentAuthInvalid( - EnvironmentAuth.serverAuthCredentialReason(error), - EnvironmentAuth.serverAuthDpopFailureReason(error), - ), - ), - Effect.catchIf(EnvironmentAuth.isServerAuthInternalError, (error) => - failEnvironmentInternal("internal_error", error), - ), - ); - if (!session.scopes.includes(AuthOrchestrationOperateScope)) { - return yield* failEnvironmentScopeRequired(AuthOrchestrationOperateScope); - } -}); +/** Operate (not read) scope: these endpoints mutate scheduling behaviour. */ +const authenticateWithOperateScope = authenticateWithScope(AuthOrchestrationOperateScope); const WriteBody = Schema.Struct({ threadId: Schema.String, @@ -108,13 +70,7 @@ const makeGetRoute = (store: AutoResumeStoreShape) => return HttpServerResponse.text("Missing threadId", { status: 400 }); } return HttpServerResponse.jsonUnsafe(yield* readThreadState(store, threadId)); - }).pipe( - Effect.catchTags({ - EnvironmentAuthInvalidError: HttpServerRespondable.toResponse, - EnvironmentInternalError: HttpServerRespondable.toResponse, - EnvironmentScopeRequiredError: HttpServerRespondable.toResponse, - }), - ), + }).pipe(Effect.catchTags(routeAuthErrorTags)), ); const makePostRoute = (store: AutoResumeStoreShape) => @@ -144,13 +100,7 @@ const makePostRoute = (store: AutoResumeStoreShape) => } return HttpServerResponse.jsonUnsafe(yield* readThreadState(store, body.threadId)); - }).pipe( - Effect.catchTags({ - EnvironmentAuthInvalidError: HttpServerRespondable.toResponse, - EnvironmentInternalError: HttpServerRespondable.toResponse, - EnvironmentScopeRequiredError: HttpServerRespondable.toResponse, - }), - ), + }).pipe(Effect.catchTags(routeAuthErrorTags)), ); /** diff --git a/apps/server/src/coil/http/auth.test.ts b/apps/server/src/coil/http/auth.test.ts new file mode 100644 index 000000000000..6a1a0ae18ddb --- /dev/null +++ b/apps/server/src/coil/http/auth.test.ts @@ -0,0 +1,115 @@ +/** + * Behaviour tests for the shared raw-route auth helper. + * + * The route suites only assert the status a caller sees for their own endpoints. This pins + * the helper itself, once, for every fork route that shares it: all three typed failures with + * their exact statuses and bodies, the forwarded DPoP reason (the drift that existed while + * there were two copies), and the fact that an authorised caller gets the session back. + * + * Auth itself is mocked, so this does NOT prove the helper still mirrors upstream's private + * `authenticateRawRouteWithScope` — that stays a logic mirror tracked in docs/coil/SEAMS.md. + */ +import { assert, describe, it } from "@effect/vitest"; +import { + type AuthEnvironmentScope, + AuthOrchestrationOperateScope, + AuthOrchestrationReadScope, +} from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import type * as Layer from "effect/Layer"; +import { HttpRouter, HttpServerResponse } from "effect/unstable/http"; + +import * as EnvironmentAuth from "../../auth/EnvironmentAuth.ts"; +import { authenticateWithScope, routeAuthErrorTags } from "./auth.ts"; +import { authFails, authOk, getJson, jsonBody, runServed, serveRoutes } from "./testAuth.ts"; + +const PATH = "/probe"; + +/** + * A route that does nothing but authenticate and echo the session id back, so the assertions + * read the helper's output rather than some feature's response shape. + */ +const probeRoute = (scope: AuthEnvironmentScope) => + HttpRouter.add( + "GET", + PATH, + Effect.gen(function* () { + const session = yield* authenticateWithScope(scope); + return HttpServerResponse.jsonUnsafe({ sessionId: String(session.sessionId) }); + }).pipe(Effect.catchTags(routeAuthErrorTags)), + ); + +const callProbe = ( + auth: Layer.Layer, + scope: AuthEnvironmentScope, +) => + runServed( + serveRoutes({ + routes: probeRoute(scope), + deps: auth, + body: Effect.gen(function* () { + const response = yield* getJson(PATH); + return { status: response.status, body: yield* jsonBody(response) }; + }), + }), + ); + +describe("authenticateWithScope", () => { + it("renders a missing credential as a 401 auth_invalid body", async () => { + const { status, body } = await callProbe( + authFails(new EnvironmentAuth.ServerAuthMissingCredentialError({})), + AuthOrchestrationOperateScope, + ); + assert.strictEqual(status, 401); + assert.strictEqual(body.code, "auth_invalid"); + assert.strictEqual(body.reason, "missing_credential"); + }); + + // The two pre-promotion copies disagreed here: only the auto-resume one forwarded the + // reason, so a Web Push client behind a skewed clock got a bare 401 it could not act on. + // The promotion reconciled to the forwarding form, which is what upstream's raw-route + // auth does; this is the case that would notice a regression back to the other one. + it("forwards the DPoP failure reason on an invalid credential", async () => { + const { status, body } = await callProbe( + authFails( + new EnvironmentAuth.ServerAuthInvalidCredentialError({ dpopFailureReason: "time_window" }), + ), + AuthOrchestrationOperateScope, + ); + assert.strictEqual(status, 401); + assert.strictEqual(body.reason, "invalid_credential"); + assert.strictEqual(body.dpopFailureReason, "time_window"); + }); + + // The third branch, and the one a route would otherwise leak as an unhandled defect: an + // internal auth failure is a 500 with a body, not a bare crash. + it("renders an internal auth failure as a 500 internal_error body", async () => { + const { status, body } = await callProbe( + authFails(new EnvironmentAuth.ServerAuthSessionCredentialValidationError({ cause: "boom" })), + AuthOrchestrationOperateScope, + ); + assert.strictEqual(status, 500); + assert.strictEqual(body.code, "internal_error"); + assert.strictEqual(body.reason, "internal_error"); + assert.isString(body.traceId); + }); + + it("renders a missing scope as a 403 naming the scope it wanted", async () => { + const { status, body } = await callProbe( + authOk([AuthOrchestrationReadScope]), + AuthOrchestrationOperateScope, + ); + assert.strictEqual(status, 403); + assert.strictEqual(body.code, "insufficient_scope"); + assert.strictEqual(body.requiredScope, AuthOrchestrationOperateScope); + }); + + it("passes the session through once the scope is held", async () => { + const { status, body } = await callProbe( + authOk([AuthOrchestrationReadScope, AuthOrchestrationOperateScope]), + AuthOrchestrationOperateScope, + ); + assert.strictEqual(status, 200); + assert.strictEqual(body.sessionId, "test-session"); + }); +}); diff --git a/apps/server/src/coil/http/auth.ts b/apps/server/src/coil/http/auth.ts new file mode 100644 index 000000000000..74d84b977695 --- /dev/null +++ b/apps/server/src/coil/http/auth.ts @@ -0,0 +1,74 @@ +/** + * Raw-route authentication shared by every fork-owned HTTP route. + * + * MIRROR of the module-private `authenticateRawRouteWithScope` in `apps/server/src/http.ts` + * (which the OTLP proxy route uses). It is not exported, so importing it would mean editing + * an upstream file; the fork replicates the ~15 lines instead. Registered as a logic mirror + * in docs/coil/SEAMS.md — if upstream changes how raw routes authenticate, this must follow. + * + * It lives here, and not beside a route, because there is exactly ONE mirror to keep faithful. + * `autoResume/http.ts` and `webPush/http.ts` each grew their own copy and immediately drifted + * (only one of them reported the DPoP failure reason); the next fork route calls this instead + * of pasting a third. + * + * @module coil/http/auth + */ + +import type { AuthEnvironmentScope } from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import { HttpServerRequest, HttpServerRespondable } from "effect/unstable/http"; + +import * as EnvironmentAuth from "../../auth/EnvironmentAuth.ts"; +import { + failEnvironmentAuthInvalid, + failEnvironmentInternal, + failEnvironmentScopeRequired, +} from "../../auth/http.ts"; + +/** + * Authenticates the in-flight request and asserts `scope`, failing with the same typed + * errors every other environment endpoint uses: `EnvironmentAuthInvalidError` (401), + * `EnvironmentScopeRequiredError` (403), `EnvironmentInternalError` (500). + * + * `dpopFailureReason` is forwarded because upstream's raw-route mirror forwards it; without + * it a relay client loses the precise reason (clock skew being the motivating case) that + * every other environment endpoint reports. + * + * Returns the authenticated session, so a route that needs to record *which* device called + * (Web Push subscribe) does not have to authenticate twice. Callers that only need the + * gate can discard it. + */ +export const authenticateWithScope = (scope: AuthEnvironmentScope) => + Effect.gen(function* () { + const request = yield* HttpServerRequest.HttpServerRequest; + const serverAuth = yield* EnvironmentAuth.EnvironmentAuth; + const session = yield* serverAuth.authenticateHttpRequest(request).pipe( + Effect.catchIf(EnvironmentAuth.isServerAuthCredentialError, (error) => + failEnvironmentAuthInvalid( + EnvironmentAuth.serverAuthCredentialReason(error), + EnvironmentAuth.serverAuthDpopFailureReason(error), + ), + ), + Effect.catchIf(EnvironmentAuth.isServerAuthInternalError, (error) => + failEnvironmentInternal("internal_error", error), + ), + ); + if (!session.scopes.includes(scope)) { + return yield* failEnvironmentScopeRequired(scope); + } + return session; + }); + +/** + * Renders the three failures `authenticateWithScope` can raise as their own responses. + * + * Pass to `Effect.catchTags` on every fork route handler: a raw route has no `HttpApi` + * wrapper to do it, so an uncaught typed error escapes as a 500 instead of the 401/403 the + * client expects. Promoted alongside the helper it belongs to — the two were pasted together + * into both route modules. + */ +export const routeAuthErrorTags = { + EnvironmentAuthInvalidError: HttpServerRespondable.toResponse, + EnvironmentInternalError: HttpServerRespondable.toResponse, + EnvironmentScopeRequiredError: HttpServerRespondable.toResponse, +} as const; diff --git a/apps/server/src/coil/http/testAuth.ts b/apps/server/src/coil/http/testAuth.ts new file mode 100644 index 000000000000..0a3cf01bfcc3 --- /dev/null +++ b/apps/server/src/coil/http/testAuth.ts @@ -0,0 +1,127 @@ +// @effect-diagnostics nodeBuiltinImport:off +/** + * Test-only scaffolding for the fork's raw HTTP routes. + * + * Every coil route authenticates through `coil/http/auth.ts`, so every route test needs the + * same three things: a mocked `EnvironmentAuth`, a real server on an ephemeral port, and a + * client pointed at it. They were pasted per feature; this is the one copy. + * + * NOT a `.test.ts` file on purpose — vitest must not collect it, and nothing in the server + * bundle imports it, so it is dropped from `vp pack` output the same way + * `coil/autoResume/replay/reactorHarness.ts` is. + * + * @module coil/http/testAuth + */ + +import * as NodeHttpServer from "@effect/platform-node/NodeHttpServer"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import type * as Scope from "effect/Scope"; +import { + FetchHttpClient, + HttpClient, + HttpClientRequest, + type HttpClientResponse, + HttpRouter, + HttpServer, +} from "effect/unstable/http"; +import * as NodeHttp from "node:http"; + +import * as EnvironmentAuth from "../../auth/EnvironmentAuth.ts"; + +/** + * A session shaped like the real thing in the fields the auth helper reads (`scopes`, + * `sessionId`). Cast because the full `AuthenticatedSession` carries branded ids and a + * `DateTime` expiry that no assertion here depends on. + */ +export const authedSession = (scopes: ReadonlyArray) => + ({ + sessionId: "test-session", + subject: "test", + method: "bearer-access-token", + scopes, + }) as unknown as EnvironmentAuth.AuthenticatedSession; + +/** Auth that succeeds, granting exactly `scopes`. */ +export const authOk = (scopes: ReadonlyArray) => + Layer.mock(EnvironmentAuth.EnvironmentAuth)({ + authenticateHttpRequest: () => Effect.succeed(authedSession(scopes)), + }); + +/** + * Auth that fails with `error`. + * + * The error must be a member of `ServerAuthCredentialError` or `ServerAuthInternalError` — + * those two unions are what the helper's `catchIf` branches guard on, and anything outside + * them matches neither and surfaces as a 500. That is equally true of upstream's OTLP route, + * which uses the identical two branches, so it is a shared property rather than a fork + * divergence. + */ +export const authFails = ( + error: EnvironmentAuth.ServerAuthCredentialError | EnvironmentAuth.ServerAuthInternalError, +) => + Layer.mock(EnvironmentAuth.EnvironmentAuth)({ + authenticateHttpRequest: () => Effect.fail(error), + }); + +export const baseUrl = (pathname: string) => + Effect.gen(function* () { + const server = yield* HttpServer.HttpServer; + const address = server.address as HttpServer.TcpAddress; + return `http://127.0.0.1:${address.port}${pathname}`; + }); + +export const getJson = (pathname: string) => + Effect.gen(function* () { + const client = yield* HttpClient.HttpClient; + return yield* client.get(yield* baseUrl(pathname)); + }); + +export const postJson = (pathname: string, body: unknown) => + Effect.gen(function* () { + const client = yield* HttpClient.HttpClient; + const request = HttpClientRequest.bodyJsonUnsafe( + HttpClientRequest.post(yield* baseUrl(pathname)), + body, + ); + return yield* client.execute(request); + }); + +/** Reads a JSON response body as a plain record, for field-by-field assertions. */ +export const jsonBody = (response: HttpClientResponse.HttpClientResponse) => + Effect.map(response.json, (value) => value as Record); + +/** + * Serves `routes` over an ephemeral port with `deps` provided, and runs `body` against it. + * + * Anything `deps` does not satisfy stays in the returned effect's requirements, so a missing + * dependency is a type error at `runServed`, not a runtime surprise. + */ +export const serveRoutes = (options: { + readonly routes: Layer.Layer; + readonly deps: Layer.Layer; + readonly body: Effect.Effect; +}) => { + const served = HttpRouter.serve(options.routes, { + disableListenLog: true, + disableLogger: true, + }).pipe( + Layer.provide(options.deps), + // provideMerge, not provide: the body needs HttpServer in context to read the ephemeral + // port off `server.address`. + Layer.provideMerge(NodeHttpServer.layer(() => NodeHttp.createServer(), { port: 0 })), + ); + return options.body.pipe(Effect.provide(Layer.merge(served, FetchHttpClient.layer))); +}; + +/** + * Discharges the platform services a served route needs and runs it as a promise. + * + * The parameter type is the check that matters: `R` is contravariant, so an effect needing + * fewer services still fits, while one that still needs a feature dependency (its store, say) + * does not compile — rather than failing at runtime with a missing-service defect. + */ +export const runServed = ( + effect: Effect.Effect, +): Promise => effect.pipe(Effect.scoped, Effect.provide(NodeServices.layer), Effect.runPromise); diff --git a/apps/server/src/coil/webPush/http.test.ts b/apps/server/src/coil/webPush/http.test.ts new file mode 100644 index 000000000000..b89c22aa136e --- /dev/null +++ b/apps/server/src/coil/webPush/http.test.ts @@ -0,0 +1,152 @@ +// @effect-diagnostics nodeBuiltinImport:off +/** + * Route-level tests for `/api/coil/push/*`. + * + * These routes used to carry their own paste of the raw-route auth mirror and had no test at + * all, which is how they came to drop `dpopFailureReason` without anyone noticing. Now they + * share `coil/http/auth.ts`, so this suite pins what a Web Push client actually sees: the + * status per failure, the DPoP reason on the wire, and the fact that a subscribe is recorded + * against the calling session (the reason the helper returns one). + * + * Serves ONLY the fork's route layer over a real HTTP server on an ephemeral port, with + * `EnvironmentAuth` mocked (see `coil/http/testAuth.ts`). + */ +import { assert, describe, it } from "@effect/vitest"; +import { AuthOrchestrationOperateScope, AuthOrchestrationReadScope } from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Layer from "effect/Layer"; +import type { HttpClient, HttpServer } from "effect/unstable/http"; +import * as NodePath from "node:path"; + +import * as EnvironmentAuth from "../../auth/EnvironmentAuth.ts"; +import { + authFails, + authOk, + getJson, + jsonBody, + postJson, + runServed, + serveRoutes, +} from "../http/testAuth.ts"; +import { + SUBSCRIBE_ROUTE_PATH, + UNSUBSCRIBE_ROUTE_PATH, + VAPID_KEY_ROUTE_PATH, + webPushRouteLayer, +} from "./http.ts"; +import { + makePushSubscriptionStore, + PushSubscriptionStore, + type PushSubscriptionStoreShape, +} from "./state.ts"; +import { WebPushVapid } from "./vapid.ts"; + +const VAPID = { + publicKey: "test-public-key", + privateKey: "test-private-key", + subject: "mailto:test@example.com", +} as const; + +const SUBSCRIPTION = { + endpoint: "https://push.example/abc", + keys: { p256dh: "p256dh-value", auth: "auth-value" }, +}; + +/** + * Serves the routes with a real store on a temp file. The store value is built first and + * handed to the layer, so a test can read what the route wrote without going back over HTTP. + */ +const withRoutes = ( + auth: Layer.Layer, + body: ( + store: PushSubscriptionStoreShape, + ) => Effect.Effect, +) => + runServed( + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const root = yield* fs.makeTempDirectoryScoped({ prefix: "coil-push-http-" }); + const store = yield* makePushSubscriptionStore(NodePath.join(root, "push.json")); + return yield* serveRoutes({ + routes: webPushRouteLayer, + deps: Layer.mergeAll( + Layer.succeed(PushSubscriptionStore, store), + Layer.succeed(WebPushVapid, VAPID), + auth, + ), + body: body(store), + }); + }), + ); + +describe("/api/coil/push", () => { + it("rejects a missing credential with a 401", () => + withRoutes(authFails(new EnvironmentAuth.ServerAuthMissingCredentialError({})), () => + Effect.gen(function* () { + const res = yield* getJson(VAPID_KEY_ROUTE_PATH); + assert.strictEqual(res.status, 401); + assert.strictEqual((yield* jsonBody(res)).reason, "missing_credential"); + }), + )); + + // The one declared behaviour change of the auth promotion: before it, these three routes + // rendered a bare 401 and a client behind a skewed clock could not tell why. Pinned at the + // route, not just at the helper, because that is where it regressed. + it("reports the DPoP failure reason on a rejected credential", () => + withRoutes( + authFails( + new EnvironmentAuth.ServerAuthInvalidCredentialError({ dpopFailureReason: "time_window" }), + ), + () => + Effect.gen(function* () { + const res = yield* postJson(SUBSCRIBE_ROUTE_PATH, SUBSCRIPTION); + assert.strictEqual(res.status, 401); + const body = yield* jsonBody(res); + assert.strictEqual(body.reason, "invalid_credential"); + assert.strictEqual(body.dpopFailureReason, "time_window"); + }), + )); + + it("rejects a read-only session from subscribe with a 403", () => + withRoutes(authOk([AuthOrchestrationReadScope]), (store) => + Effect.gen(function* () { + const res = yield* postJson(SUBSCRIBE_ROUTE_PATH, SUBSCRIPTION); + assert.strictEqual(res.status, 403); + assert.strictEqual((yield* jsonBody(res)).requiredScope, AuthOrchestrationOperateScope); + assert.lengthOf(yield* store.list, 0, "a refused subscribe must not reach the store"); + }), + )); + + it("serves the VAPID public key to a read-scoped session", () => + withRoutes(authOk([AuthOrchestrationReadScope]), () => + Effect.gen(function* () { + const res = yield* getJson(VAPID_KEY_ROUTE_PATH); + assert.strictEqual(res.status, 200); + assert.strictEqual((yield* jsonBody(res)).publicKey, VAPID.publicKey); + }), + )); + + it("records a subscribe against the calling session, and drops it on unsubscribe", () => + withRoutes(authOk([AuthOrchestrationOperateScope]), (store) => + Effect.gen(function* () { + const subscribed = yield* postJson(SUBSCRIBE_ROUTE_PATH, SUBSCRIPTION); + assert.strictEqual(subscribed.status, 200); + assert.strictEqual((yield* jsonBody(subscribed)).ok, true); + + // The session id is the reason the shared helper returns the session at all: the + // subscribe route records which device registered. + const stored = yield* store.list; + assert.deepStrictEqual( + stored.map((record) => ({ endpoint: record.endpoint, sessionId: record.sessionId })), + [{ endpoint: SUBSCRIPTION.endpoint, sessionId: "test-session" }], + ); + + const removed = yield* postJson(UNSUBSCRIBE_ROUTE_PATH, { + endpoint: SUBSCRIPTION.endpoint, + }); + assert.strictEqual(removed.status, 200); + assert.lengthOf(yield* store.list, 0); + }), + )); +}); diff --git a/apps/server/src/coil/webPush/http.ts b/apps/server/src/coil/webPush/http.ts index 2c611c92562b..890f21a86d0f 100644 --- a/apps/server/src/coil/webPush/http.ts +++ b/apps/server/src/coil/webPush/http.ts @@ -7,33 +7,19 @@ * * Raw routes, not WS-RPC: an RPC would force edits to `@t3tools/contracts` + `ws.ts` + its * scope map. These mount via `CoilRoutesLive` in coil/index.ts, so server.ts is untouched. - * See docs/coil/SEAMS.md and coil/autoResume/http.ts (the mirrored template). + * Auth is the shared `coil/http/auth.ts` mirror, not a local paste. See docs/coil/SEAMS.md. * * @module coil/webPush/http */ -import { - type AuthEnvironmentScope, - AuthOrchestrationOperateScope, - AuthOrchestrationReadScope, -} from "@t3tools/contracts"; +import { AuthOrchestrationOperateScope, AuthOrchestrationReadScope } from "@t3tools/contracts"; import * as DateTime from "effect/DateTime"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Schema from "effect/Schema"; -import { - HttpRouter, - HttpServerRequest, - HttpServerRespondable, - HttpServerResponse, -} from "effect/unstable/http"; +import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"; -import * as EnvironmentAuth from "../../auth/EnvironmentAuth.ts"; -import { - failEnvironmentAuthInvalid, - failEnvironmentInternal, - failEnvironmentScopeRequired, -} from "../../auth/http.ts"; +import { authenticateWithScope, routeAuthErrorTags } from "../http/auth.ts"; import { PushSubscriptionStore, type PushSubscriptionStoreShape } from "./state.ts"; import { WebPushVapid, type WebPushVapidKeys } from "./vapid.ts"; @@ -41,35 +27,6 @@ export const VAPID_KEY_ROUTE_PATH = "/api/coil/push/vapid-public-key"; export const SUBSCRIBE_ROUTE_PATH = "/api/coil/push/subscribe"; export const UNSUBSCRIBE_ROUTE_PATH = "/api/coil/push/unsubscribe"; -/** - * MIRROR of the module-private raw-route auth in apps/server/src/http.ts (also mirrored by - * coil/autoResume/http.ts). Returns the session so the subscribe route can record which device - * registered. Registered as a logic mirror in docs/coil/SEAMS.md. - */ -const authenticateWithScope = (scope: AuthEnvironmentScope) => - Effect.gen(function* () { - const request = yield* HttpServerRequest.HttpServerRequest; - const serverAuth = yield* EnvironmentAuth.EnvironmentAuth; - const session = yield* serverAuth.authenticateHttpRequest(request).pipe( - Effect.catchIf(EnvironmentAuth.isServerAuthCredentialError, (error) => - failEnvironmentAuthInvalid(EnvironmentAuth.serverAuthCredentialReason(error)), - ), - Effect.catchIf(EnvironmentAuth.isServerAuthInternalError, (error) => - failEnvironmentInternal("internal_error", error), - ), - ); - if (!session.scopes.includes(scope)) { - return yield* failEnvironmentScopeRequired(scope); - } - return session; - }); - -const respondableTags = { - EnvironmentAuthInvalidError: HttpServerRespondable.toResponse, - EnvironmentInternalError: HttpServerRespondable.toResponse, - EnvironmentScopeRequiredError: HttpServerRespondable.toResponse, -} as const; - const SubscribeBody = Schema.Struct({ endpoint: Schema.String, keys: Schema.Struct({ @@ -91,7 +48,7 @@ const makeVapidKeyRoute = (vapid: WebPushVapidKeys) => Effect.gen(function* () { yield* authenticateWithScope(AuthOrchestrationReadScope); return HttpServerResponse.jsonUnsafe({ publicKey: vapid.publicKey }); - }).pipe(Effect.catchTags(respondableTags)), + }).pipe(Effect.catchTags(routeAuthErrorTags)), ); const makeSubscribeRoute = (store: PushSubscriptionStoreShape) => @@ -121,7 +78,7 @@ const makeSubscribeRoute = (store: PushSubscriptionStoreShape) => createdAt, }); return HttpServerResponse.jsonUnsafe({ ok: true }); - }).pipe(Effect.catchTags(respondableTags)), + }).pipe(Effect.catchTags(routeAuthErrorTags)), ); const makeUnsubscribeRoute = (store: PushSubscriptionStoreShape) => @@ -144,7 +101,7 @@ const makeUnsubscribeRoute = (store: PushSubscriptionStoreShape) => } yield* store.removeByEndpoint(body.endpoint); return HttpServerResponse.jsonUnsafe({ ok: true }); - }).pipe(Effect.catchTags(respondableTags)), + }).pipe(Effect.catchTags(routeAuthErrorTags)), ); /** diff --git a/docs/coil/SEAMS.md b/docs/coil/SEAMS.md index 4e7809c4e0d9..d29d61131991 100644 --- a/docs/coil/SEAMS.md +++ b/docs/coil/SEAMS.md @@ -547,7 +547,7 @@ conflict during rebase, so nothing warns you when the original changes and the m | Fork mirror | Mirrors upstream | Risk if upstream changes | | --------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `apps/server/src/coil/autoResume/guards.ts` (`hasOpenBlockingRequest`) | `decider.ts` (private, unexported) | Could miss a new blocking-request activity kind and auto-resume into a prompt. **Re-checked 2026-08-08: byte-identical.** Upstream's two commits to `decider.ts` this range were both thread pinning (#5312, #5581) and added no activity kind; the four `requested`/`resolved` kinds and the stale-failure escape hatch match exactly. **2026-09-02: `threadIsGone` in this file no longer reads `settledOverride`.** #8600 gave the server a one-minute sweep that dispatches `thread.auto-settle` through `thread.settle`'s own decider case, emitting an identical `thread.settled` with no provenance — so settledness stopped meaning "the user is done here" and was cancelling week-long arms on day three. Watch `ThreadSettlementReactor.ts` / `ThreadSettlementPolicy.ts`: if upstream ever adds a provenance marker, the "user settled it" cancel can come back. | -| `apps/server/src/coil/autoResume/http.ts` (`authenticateWithOperateScope`) | `http.ts` (`authenticateRawRouteWithScope`, private, unexported) | `/api/coil/auto-resume` could authenticate more weakly than the routes beside it. **Re-checked 2026-08-08: still accurate.** Upstream's only commit to `apps/server/src/http.ts` this range was the Effect beta.103 upgrade (#5331), which deleted the gzip helpers and left `authenticateRawRouteWithScope` untouched. Compared line by line: the mirror is identical bar being specialised to the operate scope. | +| `apps/server/src/coil/http/auth.ts` (`authenticateWithScope`) | `http.ts` (`authenticateRawRouteWithScope`, private, unexported) | Every fork raw route could authenticate more weakly than the routes beside it. **Re-checked 2026-09-02 (Phase 0): still accurate.** Previously re-checked 2026-08-08. Upstream's only commit to `apps/server/src/http.ts` this range was the Effect beta.103 upgrade (#5331), which deleted the gzip helpers and left `authenticateRawRouteWithScope` untouched. Compared line by line, the mirror matches upstream's behaviour with three intentional deltas: (1) it is exported, so fork routes import it instead of pasting it; (2) it returns the authenticated session, which upstream's discards, because the Web Push subscribe route records which device registered; (3) its scope parameter is typed `AuthEnvironmentScope` (all 8 literals) where upstream narrows to the two orchestration scopes — inherited from the webPush copy, and both callers pass orchestration scopes, so it is a wider type over identical use. **One mirror as of Phase 0:** it used to be pasted separately into `coil/autoResume/http.ts` and `coil/webPush/http.ts`, and the two had drifted (only auto-resume forwarded `dpopFailureReason`); both now call this module. | | `apps/server/src/orchestration/Layers/CrashRecoveryReconciler.ts` (`getSnapshot()`) | `ProjectionSnapshotQuery.getSnapshot()` vs. the lighter `getCommandReadModel()` | **Live risk, found at the 2026-08-02 sync, re-confirmed unchanged 2026-08-08 — still not fixed.** Upstream moved its own orchestration-snapshot route off `getSnapshot()` onto `getCommandReadModel()` in this range, commenting that hydrating every message and activity payload "has OOM-killed servers". The fork's boot reconciler still calls `getSnapshot()`, and it runs on the startup path **before commands are accepted**, so an OOM there is a hard boot failure rather than one slow request. Both return `OrchestrationReadModel` and the reconciler only reads `thread.session` / `thread.latestTurn` metadata, so the swap looks like a drop-in — but it was deliberately left out of the sync commit and needs its own PR with coverage. | | `apps/web/src/outbox/**` (thread outbox) | `apps/mobile/src/state/thread-outbox-*.ts` (upstream-authored, still maintained) | The web outbox is a hand port of upstream's mobile one, function for function. Two divergences are deliberate: the web queue drops image attachments (mobile persists them as base64 data URLs, which localStorage cannot hold) and orders on an explicit `sortKey` for user reordering where mobile sorts on `createdAt` alone. A third divergence closed at the 2026-08-14 sync: upstream #6543 made mobile steer active turns by default too (its outbox delivery gate is now connectivity-only — `thread-outbox-model.ts` dropped the `!threadBusy` term), so both platforms steer. Upstream reworking its mobile outbox produces no conflict here. | | `apps/web/src/coil/AutoResumeOverlay.tsx` (`COMPOSER_OVERLAY_SELECTOR`, `chat-composer-horizontal-inset`) | `apps/web/src/components/ChatView.tsx` — the `[data-chat-composer-overlay="true"]` element it measures for `composerOverlayHeight`, and the `.chat-composer-horizontal-inset` class in `index.css` that the composer wrapper uses | **Read-only presentation dependencies, not code edits.** The auto-resume capsule is anchored bottom-right, immediately above the docked composer and flush with its right edge. The overlay mounts as a sibling of `` in the route file and cannot receive the composer's geometry as a prop without widening that seam, so it measures the data attribute with a `ResizeObserver` instead. Horizontal alignment additionally reuses the composer's own inset class, because that inset is `0.75rem` at base, `1.25rem` from `40rem` up, and carries `env(safe-area-inset-right)` — any hard-coded value overhangs on wide viewports. If the attribute disappears the capsule falls back to a fixed 76px offset; if the class is renamed the capsule's right edge drifts from the composer's. Both degrade visually, neither breaks. Re-check at each sync. **2026-08-10 (#67): the measurement now converts coordinate spaces rather than assuming they match.** The composer's `offsetParent` is the chat column; the capsule's is `SidebarInset`. Those boxes coincide only while no panel is open, so the original "measure against the composer's parent, apply against ours" shortcut stranded the capsule by 352px horizontally when the inline right panel opened and by the drawer's full height when the terminal drawer opened. `autoResumeAnchor.ts` now derives `bottom`/`left`/`width` from the composer's rect expressed in the capsule's own offsetParent coordinates, and all three boxes are observed. **This adds no new upstream dependency** — same selector, same class — but it does mean the capsule now depends on the chat column remaining the composer's nearest positioned ancestor. |