mirror of
https://github.com/nexu-io/open-design.git
synced 2026-05-31 19:04:39 +07:00
refactor(daemon): introduce HTTP Request Adapter + typed Deps (#2636)
* refactor(daemon): introduce HTTP Request Adapter + typed Deps (proof on active-context-routes)
Adds a typed HTTP boundary Adapter under apps/daemon/src/http/ that replaces
the untyped ServerContext service-locator pattern (30+ fields, mostly any)
for route handlers. Routes become pure (input, deps) -> Result<output>
functions, unit-testable without Express or supertest.
Six new modules under apps/daemon/src/http/:
- types.ts Result<T,E>, ok(), err(), JsonRouteSpec, Handler,
RouteInputContext, HttpMethod, InputParser
- parse.ts rawInput(req), validationError(message, issues?)
- response.ts sendJson(), sendApiError(), statusForError() +
ERROR_STATUS_BY_CODE map
- origin-guard.ts guardSameOrigin(req, origin) wrapping isLocalSameOrigin
as a Result
- adapter.ts defineJsonRoute(), mountJsonRoute() (only place that
knows about req/res)
- index.ts barrel
active-context-routes.ts migrated as proof of pattern. parsePostActive(),
handlePostActive(), handleGetActive() are now pure functions; postActiveRoute
and getActiveRoute are exported route specs. The wire signature
registerActiveContextRoutes(app, ctx) is preserved so server.ts is untouched.
Spec at specs/current/daemon-http-adapter.md captures the strangler migration
order for the remaining route files (mcp-routes, chat-routes, artifact
routes, etc.) and a StreamRoute follow-up where the Run Orchestrator lands.
Wire-format note: cross-origin response moves from the legacy
{ error: 'cross-origin request rejected' } shape to the structured
{ error: { code: 'FORBIDDEN', message: ... } } shape. Backwards-compatible
via the existing CompatibleErrorResponse = ApiErrorResponse | LegacyErrorResponse
union in @open-design/contracts.
Validation:
- pnpm install (post-rebase, exit 0)
- pnpm --filter @open-design/daemon typecheck (both tsconfig.json and
tsconfig.tests.json silent => pass)
- pnpm --filter @open-design/daemon test: 15 new tests pass
(tests/http/adapter.test.ts + tests/active-context-routes.test.ts).
84 pre-existing failures across 23 files are unchanged and unrelated
to this PR (Windows symlink / short-name / colon-in-filename, upstream
behavior drift, missing plugin marketplace fixtures, and a freshly-
added tools-connectors-cli suite of 38 failures that landed during
the rebase).
Sharpens W4/W5 of specs/current/maintainability-roadmap.md and unlocks
W6 (Run Orchestrator).
* chore: add core-js, electron-winstaller, protobufjs, sharp to pnpm.onlyBuiltDependencies
This commit is contained in:
parent
558fedd207
commit
aa0616062d
11 changed files with 686 additions and 61 deletions
|
|
@ -1,71 +1,128 @@
|
|||
import type { Express } from 'express';
|
||||
import { createApiError } from '@open-design/contracts';
|
||||
import { ACTIVE_CONTEXT_TTL_MS } from './constants.js';
|
||||
import type { RouteDeps } from './server-context.js';
|
||||
import { defineJsonRoute, err, mountJsonRoute, ok, type Result } from './http/index.js';
|
||||
|
||||
export interface RegisterActiveContextRoutesDeps extends RouteDeps<'db' | 'http' | 'projectStore'> {}
|
||||
|
||||
export function registerActiveContextRoutes(app: Express, ctx: RegisterActiveContextRoutesDeps) {
|
||||
const { db } = ctx;
|
||||
const { sendApiError, isLocalSameOrigin, resolvedPortRef } = ctx.http;
|
||||
const { getProject } = ctx.projectStore;
|
||||
const getResolvedPort = () => resolvedPortRef.current;
|
||||
// Soft "what is the user looking at right now in Open Design?" channel. The
|
||||
// web UI POSTs the current project + file on every route change; the MCP
|
||||
// surface reads it so a coding agent in another repo can resolve "the design
|
||||
// I have open" without the user typing the project id. In-memory only —
|
||||
// daemon restart clears it.
|
||||
interface ActiveContext {
|
||||
projectId: string;
|
||||
fileName: string | null;
|
||||
ts: number;
|
||||
}
|
||||
|
||||
// Soft "what is the user looking at right now in Open Design?" channel. The
|
||||
// web UI POSTs the current project + file on every route change; the MCP
|
||||
// surface reads it so a coding agent in another repo can resolve "the design
|
||||
// I have open" without the user typing the project id. In-memory only -
|
||||
// daemon restart clears it.
|
||||
let activeContext: { projectId: string; fileName: string | null; ts: number } | null = null;
|
||||
interface ActiveContextStore {
|
||||
current: ActiveContext | null;
|
||||
}
|
||||
|
||||
// Active context is private to the local machine. The daemon may bind beyond
|
||||
// loopback, so without an origin check a peer on the LAN could read what the
|
||||
// user is currently looking at (GET) or spoof it to redirect MCP fallbacks
|
||||
// (POST). The web proxies same-origin and MCP runs in-process via 127.0.0.1,
|
||||
// so both legitimate callers pass the check.
|
||||
app.post('/api/active', (req, res) => {
|
||||
if (!isLocalSameOrigin(req, getResolvedPort())) {
|
||||
return res.status(403).json({ error: 'cross-origin request rejected' });
|
||||
}
|
||||
try {
|
||||
const body = req.body || {};
|
||||
if (body.active === false) {
|
||||
activeContext = null;
|
||||
res.json({ active: false });
|
||||
return;
|
||||
}
|
||||
const projectId = typeof body.projectId === 'string' ? body.projectId : '';
|
||||
if (!projectId) {
|
||||
sendApiError(res, 400, 'BAD_REQUEST', 'projectId is required');
|
||||
return;
|
||||
}
|
||||
const fileName =
|
||||
typeof body.fileName === 'string' && body.fileName.length > 0
|
||||
? body.fileName
|
||||
: null;
|
||||
activeContext = { projectId, fileName, ts: Date.now() };
|
||||
res.json({ active: true, ...activeContext });
|
||||
} catch (err) {
|
||||
sendApiError(res, 400, 'BAD_REQUEST', String(err));
|
||||
}
|
||||
});
|
||||
type PostActiveInput =
|
||||
| { kind: 'clear' }
|
||||
| { kind: 'set'; projectId: string; fileName: string | null };
|
||||
|
||||
app.get('/api/active', (req, res) => {
|
||||
if (!isLocalSameOrigin(req, getResolvedPort())) {
|
||||
return res.status(403).json({ error: 'cross-origin request rejected' });
|
||||
}
|
||||
if (!activeContext || Date.now() - activeContext.ts > ACTIVE_CONTEXT_TTL_MS) {
|
||||
activeContext = null;
|
||||
res.json({ active: false });
|
||||
return;
|
||||
}
|
||||
const project = getProject(db, activeContext.projectId);
|
||||
res.json({
|
||||
active: true,
|
||||
projectId: activeContext.projectId,
|
||||
projectName: project?.name ?? null,
|
||||
fileName: activeContext.fileName,
|
||||
ts: activeContext.ts,
|
||||
ageMs: Date.now() - activeContext.ts,
|
||||
});
|
||||
type PostActiveOutput =
|
||||
| { active: false }
|
||||
| { active: true; projectId: string; fileName: string | null; ts: number };
|
||||
|
||||
type GetActiveOutput =
|
||||
| { active: false }
|
||||
| {
|
||||
active: true;
|
||||
projectId: string;
|
||||
projectName: string | null;
|
||||
fileName: string | null;
|
||||
ts: number;
|
||||
ageMs: number;
|
||||
};
|
||||
|
||||
interface ActiveContextDomainDeps {
|
||||
store: ActiveContextStore;
|
||||
db: unknown;
|
||||
getProject: (db: unknown, projectId: string) => { name?: string | null } | null | undefined;
|
||||
now: () => number;
|
||||
}
|
||||
|
||||
function parsePostActive(raw: { body: unknown }): Result<PostActiveInput> {
|
||||
const body = (raw.body ?? {}) as Record<string, unknown>;
|
||||
if (body.active === false) {
|
||||
return ok({ kind: 'clear' });
|
||||
}
|
||||
const projectId = typeof body.projectId === 'string' ? body.projectId : '';
|
||||
if (!projectId) {
|
||||
return err(createApiError('BAD_REQUEST', 'projectId is required'));
|
||||
}
|
||||
const fileName =
|
||||
typeof body.fileName === 'string' && body.fileName.length > 0 ? body.fileName : null;
|
||||
return ok({ kind: 'set', projectId, fileName });
|
||||
}
|
||||
|
||||
function handlePostActive(
|
||||
input: PostActiveInput,
|
||||
deps: ActiveContextDomainDeps,
|
||||
): Result<PostActiveOutput> {
|
||||
if (input.kind === 'clear') {
|
||||
deps.store.current = null;
|
||||
return ok({ active: false });
|
||||
}
|
||||
const next: ActiveContext = {
|
||||
projectId: input.projectId,
|
||||
fileName: input.fileName,
|
||||
ts: deps.now(),
|
||||
};
|
||||
deps.store.current = next;
|
||||
return ok({ active: true, ...next });
|
||||
}
|
||||
|
||||
function handleGetActive(
|
||||
_input: void,
|
||||
deps: ActiveContextDomainDeps,
|
||||
): Result<GetActiveOutput> {
|
||||
const current = deps.store.current;
|
||||
if (!current || deps.now() - current.ts > ACTIVE_CONTEXT_TTL_MS) {
|
||||
deps.store.current = null;
|
||||
return ok({ active: false });
|
||||
}
|
||||
const project = deps.getProject(deps.db, current.projectId);
|
||||
return ok({
|
||||
active: true,
|
||||
projectId: current.projectId,
|
||||
projectName: project?.name ?? null,
|
||||
fileName: current.fileName,
|
||||
ts: current.ts,
|
||||
ageMs: deps.now() - current.ts,
|
||||
});
|
||||
}
|
||||
|
||||
export const postActiveRoute = defineJsonRoute<PostActiveInput, PostActiveOutput, ActiveContextDomainDeps>({
|
||||
method: 'post',
|
||||
path: '/api/active',
|
||||
requireSameOrigin: true,
|
||||
parse: parsePostActive,
|
||||
handle: handlePostActive,
|
||||
});
|
||||
|
||||
export const getActiveRoute = defineJsonRoute<void, GetActiveOutput, ActiveContextDomainDeps>({
|
||||
method: 'get',
|
||||
path: '/api/active',
|
||||
requireSameOrigin: true,
|
||||
parse: () => ok(undefined),
|
||||
handle: handleGetActive,
|
||||
});
|
||||
|
||||
export function registerActiveContextRoutes(app: Express, ctx: RegisterActiveContextRoutesDeps): void {
|
||||
const store: ActiveContextStore = { current: null };
|
||||
const domainDeps: ActiveContextDomainDeps = {
|
||||
store,
|
||||
db: ctx.db,
|
||||
getProject: ctx.projectStore.getProject,
|
||||
now: () => Date.now(),
|
||||
};
|
||||
const adapter = { resolvedPortRef: ctx.http.resolvedPortRef };
|
||||
mountJsonRoute(app, postActiveRoute, domainDeps, adapter);
|
||||
mountJsonRoute(app, getActiveRoute, domainDeps, adapter);
|
||||
}
|
||||
|
|
|
|||
59
apps/daemon/src/http/adapter.ts
Normal file
59
apps/daemon/src/http/adapter.ts
Normal file
|
|
@ -0,0 +1,59 @@
|
|||
import type { Express, Request, Response } from 'express';
|
||||
import { createApiError } from '@open-design/contracts';
|
||||
import { rawInput } from './parse.js';
|
||||
import { sendApiError, sendJson, statusForError } from './response.js';
|
||||
import { guardSameOrigin, type OriginContext } from './origin-guard.js';
|
||||
import type { JsonRouteSpec } from './types.js';
|
||||
|
||||
export interface AdapterContext extends OriginContext {}
|
||||
|
||||
/**
|
||||
* Identity function that pins a route spec's generic parameters at the
|
||||
* definition site so callers do not have to repeat them. The returned spec
|
||||
* is consumed by `mountJsonRoute` (live) and by tests (direct invocation of
|
||||
* `route.parse` / `route.handle`).
|
||||
*/
|
||||
export function defineJsonRoute<Input, Output, Deps>(
|
||||
spec: JsonRouteSpec<Input, Output, Deps>,
|
||||
): JsonRouteSpec<Input, Output, Deps> {
|
||||
return spec;
|
||||
}
|
||||
|
||||
/**
|
||||
* Mounts one JsonRouteSpec on an Express app. The Adapter is the only code
|
||||
* here that knows about req/res; the route's parse and handle functions
|
||||
* operate on `RouteInputContext` and `Deps` respectively, so they are unit
|
||||
* testable without Express.
|
||||
*/
|
||||
export function mountJsonRoute<Input, Output, Deps>(
|
||||
app: Express,
|
||||
spec: JsonRouteSpec<Input, Output, Deps>,
|
||||
deps: Deps,
|
||||
adapter: AdapterContext,
|
||||
): void {
|
||||
app[spec.method](spec.path, async (req: Request, res: Response) => {
|
||||
try {
|
||||
if (spec.requireSameOrigin) {
|
||||
const origin = guardSameOrigin(req, adapter);
|
||||
if (!origin.ok) {
|
||||
sendApiError(res, statusForError(origin.error), origin.error);
|
||||
return;
|
||||
}
|
||||
}
|
||||
const parsed = spec.parse(rawInput(req));
|
||||
if (!parsed.ok) {
|
||||
sendApiError(res, statusForError(parsed.error), parsed.error);
|
||||
return;
|
||||
}
|
||||
const result = await spec.handle(parsed.value, deps);
|
||||
if (!result.ok) {
|
||||
sendApiError(res, statusForError(result.error), result.error);
|
||||
return;
|
||||
}
|
||||
sendJson(res, spec.successStatus ?? 200, result.value);
|
||||
} catch (e) {
|
||||
const message = e instanceof Error ? e.message : String(e);
|
||||
sendApiError(res, 500, createApiError('INTERNAL_ERROR', message));
|
||||
}
|
||||
});
|
||||
}
|
||||
5
apps/daemon/src/http/index.ts
Normal file
5
apps/daemon/src/http/index.ts
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
export * from './types.js';
|
||||
export * from './parse.js';
|
||||
export * from './response.js';
|
||||
export * from './origin-guard.js';
|
||||
export * from './adapter.js';
|
||||
20
apps/daemon/src/http/origin-guard.ts
Normal file
20
apps/daemon/src/http/origin-guard.ts
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
import type { Request } from 'express';
|
||||
import { createApiError } from '@open-design/contracts';
|
||||
import { isLocalSameOrigin } from '../origin-validation.js';
|
||||
import { err, ok, type Result } from './types.js';
|
||||
|
||||
export interface OriginContext {
|
||||
resolvedPortRef: { current: number };
|
||||
}
|
||||
|
||||
/**
|
||||
* Adapter wrapper around `isLocalSameOrigin` that yields a `Result` so the
|
||||
* HTTP Adapter can fold the origin decision into the same error-handling
|
||||
* pipeline as parse/handle failures.
|
||||
*/
|
||||
export function guardSameOrigin(req: Request, origin: OriginContext): Result<void> {
|
||||
if (isLocalSameOrigin(req, origin.resolvedPortRef.current)) {
|
||||
return ok(undefined);
|
||||
}
|
||||
return err(createApiError('FORBIDDEN', 'cross-origin request rejected'));
|
||||
}
|
||||
23
apps/daemon/src/http/parse.ts
Normal file
23
apps/daemon/src/http/parse.ts
Normal file
|
|
@ -0,0 +1,23 @@
|
|||
import type { Request } from 'express';
|
||||
import { createApiError, type ApiError } from '@open-design/contracts';
|
||||
import type { RouteInputContext } from './types.js';
|
||||
|
||||
export function rawInput(req: Request): RouteInputContext {
|
||||
return {
|
||||
body: req.body,
|
||||
query: (req.query ?? {}) as Record<string, unknown>,
|
||||
params: (req.params ?? {}) as Record<string, string>,
|
||||
};
|
||||
}
|
||||
|
||||
export function validationError(
|
||||
message: string,
|
||||
issues: Array<{ path: string; message: string }> = [],
|
||||
): ApiError {
|
||||
if (issues.length === 0) {
|
||||
return createApiError('BAD_REQUEST', message);
|
||||
}
|
||||
return createApiError('BAD_REQUEST', message, {
|
||||
details: { kind: 'validation', issues } as unknown as NonNullable<ApiError['details']>,
|
||||
});
|
||||
}
|
||||
32
apps/daemon/src/http/response.ts
Normal file
32
apps/daemon/src/http/response.ts
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
import type { Response } from 'express';
|
||||
import { createApiErrorResponse, type ApiError, type ApiErrorCode } from '@open-design/contracts';
|
||||
|
||||
export function sendJson(res: Response, status: number, body: unknown): void {
|
||||
res.status(status).json(body);
|
||||
}
|
||||
|
||||
export function sendApiError(res: Response, status: number, error: ApiError): void {
|
||||
res.status(status).json(createApiErrorResponse(error));
|
||||
}
|
||||
|
||||
const ERROR_STATUS_BY_CODE: Partial<Record<ApiErrorCode, number>> = {
|
||||
BAD_REQUEST: 400,
|
||||
UNAUTHORIZED: 401,
|
||||
FORBIDDEN: 403,
|
||||
NOT_FOUND: 404,
|
||||
CONFLICT: 409,
|
||||
PAYLOAD_TOO_LARGE: 413,
|
||||
UNSUPPORTED_MEDIA_TYPE: 415,
|
||||
VALIDATION_FAILED: 422,
|
||||
RATE_LIMITED: 429,
|
||||
PROJECT_NOT_FOUND: 404,
|
||||
FILE_NOT_FOUND: 404,
|
||||
ARTIFACT_NOT_FOUND: 404,
|
||||
INTERNAL_ERROR: 500,
|
||||
AGENT_UNAVAILABLE: 503,
|
||||
UPSTREAM_UNAVAILABLE: 502,
|
||||
};
|
||||
|
||||
export function statusForError(error: ApiError): number {
|
||||
return ERROR_STATUS_BY_CODE[error.code] ?? 500;
|
||||
}
|
||||
32
apps/daemon/src/http/types.ts
Normal file
32
apps/daemon/src/http/types.ts
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
import type { ApiError } from '@open-design/contracts';
|
||||
|
||||
export type Result<T, E = ApiError> =
|
||||
| { ok: true; value: T }
|
||||
| { ok: false; error: E };
|
||||
|
||||
export const ok = <T, E = ApiError>(value: T): Result<T, E> => ({ ok: true, value });
|
||||
export const err = <T = never, E = ApiError>(error: E): Result<T, E> => ({ ok: false, error });
|
||||
|
||||
export interface RouteInputContext {
|
||||
body: unknown;
|
||||
query: Record<string, unknown>;
|
||||
params: Record<string, string>;
|
||||
}
|
||||
|
||||
export type InputParser<Input> = (raw: RouteInputContext) => Result<Input>;
|
||||
|
||||
export type Handler<Input, Output, Deps> = (
|
||||
input: Input,
|
||||
deps: Deps,
|
||||
) => Promise<Result<Output>> | Result<Output>;
|
||||
|
||||
export type HttpMethod = 'get' | 'post' | 'put' | 'delete' | 'patch';
|
||||
|
||||
export interface JsonRouteSpec<Input, Output, Deps> {
|
||||
method: HttpMethod;
|
||||
path: string;
|
||||
requireSameOrigin?: boolean;
|
||||
parse: InputParser<Input>;
|
||||
handle: Handler<Input, Output, Deps>;
|
||||
successStatus?: number;
|
||||
}
|
||||
126
apps/daemon/tests/active-context-routes.test.ts
Normal file
126
apps/daemon/tests/active-context-routes.test.ts
Normal file
|
|
@ -0,0 +1,126 @@
|
|||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { getActiveRoute, postActiveRoute } from '../src/active-context-routes.js';
|
||||
import { ACTIVE_CONTEXT_TTL_MS } from '../src/constants.js';
|
||||
|
||||
interface MockStore {
|
||||
current:
|
||||
| {
|
||||
projectId: string;
|
||||
fileName: string | null;
|
||||
ts: number;
|
||||
}
|
||||
| null;
|
||||
}
|
||||
|
||||
function makeDeps(now = 1_000) {
|
||||
const store: MockStore = { current: null };
|
||||
// Annotated return type widens the mock so `.mockReturnValue(null)` is
|
||||
// allowed by the inferred Mock type later in the file.
|
||||
const getProject = vi.fn(
|
||||
(_db: unknown, id: string): { name?: string | null } | null | undefined => ({
|
||||
name: `Project ${id}`,
|
||||
}),
|
||||
);
|
||||
return {
|
||||
store,
|
||||
db: { fake: true },
|
||||
getProject,
|
||||
now: () => now,
|
||||
};
|
||||
}
|
||||
|
||||
const EMPTY_INPUT = { body: {}, query: {}, params: {} };
|
||||
|
||||
describe('active context — POST /api/active', () => {
|
||||
it('clears the store when body.active === false', async () => {
|
||||
const deps = makeDeps();
|
||||
deps.store.current = { projectId: 'p1', fileName: 'a.html', ts: 1 };
|
||||
const parsed = postActiveRoute.parse({ ...EMPTY_INPUT, body: { active: false } });
|
||||
expect(parsed.ok).toBe(true);
|
||||
if (!parsed.ok) return;
|
||||
const out = await postActiveRoute.handle(parsed.value, deps);
|
||||
expect(out).toEqual({ ok: true, value: { active: false } });
|
||||
expect(deps.store.current).toBeNull();
|
||||
});
|
||||
|
||||
it('rejects when projectId is missing', () => {
|
||||
const parsed = postActiveRoute.parse({ ...EMPTY_INPUT, body: {} });
|
||||
expect(parsed.ok).toBe(false);
|
||||
if (parsed.ok) return;
|
||||
expect(parsed.error.code).toBe('BAD_REQUEST');
|
||||
expect(parsed.error.message).toBe('projectId is required');
|
||||
});
|
||||
|
||||
it('stores projectId + fileName + timestamp on success', async () => {
|
||||
const deps = makeDeps(5_000);
|
||||
const parsed = postActiveRoute.parse({
|
||||
...EMPTY_INPUT,
|
||||
body: { projectId: 'p1', fileName: 'index.html' },
|
||||
});
|
||||
expect(parsed.ok).toBe(true);
|
||||
if (!parsed.ok) return;
|
||||
const out = await postActiveRoute.handle(parsed.value, deps);
|
||||
expect(out).toEqual({
|
||||
ok: true,
|
||||
value: { active: true, projectId: 'p1', fileName: 'index.html', ts: 5_000 },
|
||||
});
|
||||
expect(deps.store.current).toEqual({ projectId: 'p1', fileName: 'index.html', ts: 5_000 });
|
||||
});
|
||||
|
||||
it('treats empty fileName as null', async () => {
|
||||
const deps = makeDeps(7_000);
|
||||
const parsed = postActiveRoute.parse({
|
||||
...EMPTY_INPUT,
|
||||
body: { projectId: 'p1', fileName: '' },
|
||||
});
|
||||
expect(parsed.ok).toBe(true);
|
||||
if (!parsed.ok) return;
|
||||
const out = await postActiveRoute.handle(parsed.value, deps);
|
||||
expect(out.ok).toBe(true);
|
||||
if (!out.ok) return;
|
||||
expect(out.value).toMatchObject({ active: true, fileName: null });
|
||||
});
|
||||
});
|
||||
|
||||
describe('active context — GET /api/active', () => {
|
||||
it('returns inactive when nothing is stored', async () => {
|
||||
const deps = makeDeps();
|
||||
const out = await getActiveRoute.handle(undefined, deps);
|
||||
expect(out).toEqual({ ok: true, value: { active: false } });
|
||||
});
|
||||
|
||||
it('returns inactive and clears when TTL has expired', async () => {
|
||||
const deps = makeDeps(10_000 + ACTIVE_CONTEXT_TTL_MS);
|
||||
deps.store.current = { projectId: 'p1', fileName: null, ts: 9_000 };
|
||||
const out = await getActiveRoute.handle(undefined, deps);
|
||||
expect(out).toEqual({ ok: true, value: { active: false } });
|
||||
expect(deps.store.current).toBeNull();
|
||||
});
|
||||
|
||||
it('returns active payload with project name + ageMs when fresh', async () => {
|
||||
const deps = makeDeps(2_500);
|
||||
deps.store.current = { projectId: 'p7', fileName: 'plan.md', ts: 2_000 };
|
||||
const out = await getActiveRoute.handle(undefined, deps);
|
||||
expect(out.ok).toBe(true);
|
||||
if (!out.ok) return;
|
||||
expect(out.value).toEqual({
|
||||
active: true,
|
||||
projectId: 'p7',
|
||||
projectName: 'Project p7',
|
||||
fileName: 'plan.md',
|
||||
ts: 2_000,
|
||||
ageMs: 500,
|
||||
});
|
||||
expect(deps.getProject).toHaveBeenCalledWith(deps.db, 'p7');
|
||||
});
|
||||
|
||||
it('tolerates a missing project (projectName = null)', async () => {
|
||||
const deps = makeDeps(3_000);
|
||||
deps.getProject.mockReturnValue(null);
|
||||
deps.store.current = { projectId: 'p9', fileName: null, ts: 2_500 };
|
||||
const out = await getActiveRoute.handle(undefined, deps);
|
||||
expect(out.ok).toBe(true);
|
||||
if (!out.ok) return;
|
||||
expect(out.value).toMatchObject({ active: true, projectName: null });
|
||||
});
|
||||
});
|
||||
147
apps/daemon/tests/http/adapter.test.ts
Normal file
147
apps/daemon/tests/http/adapter.test.ts
Normal file
|
|
@ -0,0 +1,147 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { createApiError } from '@open-design/contracts';
|
||||
import { defineJsonRoute, err, mountJsonRoute, ok } from '../../src/http/index.js';
|
||||
import { isLocalSameOrigin } from '../../src/origin-validation.js';
|
||||
|
||||
vi.mock('../../src/origin-validation.js', () => ({
|
||||
isLocalSameOrigin: vi.fn(() => true),
|
||||
}));
|
||||
|
||||
interface MockApp {
|
||||
get: (path: string, handler: any) => void;
|
||||
post: (path: string, handler: any) => void;
|
||||
put: (path: string, handler: any) => void;
|
||||
delete: (path: string, handler: any) => void;
|
||||
patch: (path: string, handler: any) => void;
|
||||
handlers: Record<string, (req: any, res: any) => Promise<void> | void>;
|
||||
}
|
||||
|
||||
function makeApp(): MockApp {
|
||||
const handlers: MockApp['handlers'] = {};
|
||||
const make = (method: string) => (path: string, handler: any) => {
|
||||
handlers[`${method.toUpperCase()} ${path}`] = handler;
|
||||
};
|
||||
return {
|
||||
get: make('get'),
|
||||
post: make('post'),
|
||||
put: make('put'),
|
||||
delete: make('delete'),
|
||||
patch: make('patch'),
|
||||
handlers,
|
||||
};
|
||||
}
|
||||
|
||||
function makeRes() {
|
||||
return {
|
||||
status: vi.fn().mockReturnThis(),
|
||||
json: vi.fn().mockReturnThis(),
|
||||
};
|
||||
}
|
||||
|
||||
const adapter = { resolvedPortRef: { current: 7456 } };
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(isLocalSameOrigin).mockReturnValue(true);
|
||||
});
|
||||
|
||||
describe('http adapter', () => {
|
||||
it('parses input and returns the success payload', async () => {
|
||||
const route = defineJsonRoute<{ value: string }, { echoed: string }, unknown>({
|
||||
method: 'post',
|
||||
path: '/echo',
|
||||
parse: (raw) => ok({ value: String((raw.body as any).value) }),
|
||||
handle: (input) => ok({ echoed: input.value }),
|
||||
});
|
||||
const app = makeApp();
|
||||
mountJsonRoute(app as any, route, {}, adapter);
|
||||
const res = makeRes();
|
||||
await app.handlers['POST /echo']!({ body: { value: 'hi' }, query: {}, params: {} }, res);
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(res.json).toHaveBeenCalledWith({ echoed: 'hi' });
|
||||
});
|
||||
|
||||
it('returns 400 when parse fails', async () => {
|
||||
const route = defineJsonRoute<{ value: string }, unknown, unknown>({
|
||||
method: 'post',
|
||||
path: '/missing',
|
||||
parse: () => err(createApiError('BAD_REQUEST', 'required')),
|
||||
handle: () => ok({}),
|
||||
});
|
||||
const app = makeApp();
|
||||
mountJsonRoute(app as any, route, {}, adapter);
|
||||
const res = makeRes();
|
||||
await app.handlers['POST /missing']!({ body: {}, query: {}, params: {} }, res);
|
||||
expect(res.status).toHaveBeenCalledWith(400);
|
||||
expect(res.json).toHaveBeenCalledWith({ error: { code: 'BAD_REQUEST', message: 'required' } });
|
||||
});
|
||||
|
||||
it('maps a NOT_FOUND domain error to 404', async () => {
|
||||
const route = defineJsonRoute<void, unknown, unknown>({
|
||||
method: 'get',
|
||||
path: '/missing',
|
||||
parse: () => ok(undefined),
|
||||
handle: () => err(createApiError('NOT_FOUND', 'gone')),
|
||||
});
|
||||
const app = makeApp();
|
||||
mountJsonRoute(app as any, route, {}, adapter);
|
||||
const res = makeRes();
|
||||
await app.handlers['GET /missing']!({ body: {}, query: {}, params: {} }, res);
|
||||
expect(res.status).toHaveBeenCalledWith(404);
|
||||
expect(res.json).toHaveBeenCalledWith({ error: { code: 'NOT_FOUND', message: 'gone' } });
|
||||
});
|
||||
|
||||
it('blocks cross-origin requests when requireSameOrigin is set', async () => {
|
||||
vi.mocked(isLocalSameOrigin).mockReturnValue(false);
|
||||
const route = defineJsonRoute<void, { secret: number }, unknown>({
|
||||
method: 'get',
|
||||
path: '/secret',
|
||||
requireSameOrigin: true,
|
||||
parse: () => ok(undefined),
|
||||
handle: () => ok({ secret: 42 }),
|
||||
});
|
||||
const app = makeApp();
|
||||
mountJsonRoute(app as any, route, {}, adapter);
|
||||
const res = makeRes();
|
||||
await app.handlers['GET /secret']!({ body: {}, query: {}, params: {} }, res);
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(res.json).toHaveBeenCalledWith({
|
||||
error: { code: 'FORBIDDEN', message: 'cross-origin request rejected' },
|
||||
});
|
||||
});
|
||||
|
||||
it('catches thrown handler errors as INTERNAL_ERROR (500)', async () => {
|
||||
const route = defineJsonRoute<void, unknown, unknown>({
|
||||
method: 'get',
|
||||
path: '/boom',
|
||||
parse: () => ok(undefined),
|
||||
handle: () => {
|
||||
throw new Error('boom');
|
||||
},
|
||||
});
|
||||
const app = makeApp();
|
||||
mountJsonRoute(app as any, route, {}, adapter);
|
||||
const res = makeRes();
|
||||
await app.handlers['GET /boom']!({ body: {}, query: {}, params: {} }, res);
|
||||
expect(res.status).toHaveBeenCalledWith(500);
|
||||
expect(res.json).toHaveBeenCalledWith({
|
||||
error: { code: 'INTERNAL_ERROR', message: 'boom' },
|
||||
});
|
||||
});
|
||||
|
||||
it('passes deps through to the handler', async () => {
|
||||
interface Deps {
|
||||
tag: string;
|
||||
}
|
||||
const route = defineJsonRoute<void, { tag: string }, Deps>({
|
||||
method: 'get',
|
||||
path: '/deps',
|
||||
parse: () => ok(undefined),
|
||||
handle: (_input, deps) => ok({ tag: deps.tag }),
|
||||
});
|
||||
const app = makeApp();
|
||||
mountJsonRoute(app as any, route, { tag: 'injected' }, adapter);
|
||||
const res = makeRes();
|
||||
await app.handlers['GET /deps']!({ body: {}, query: {}, params: {} }, res);
|
||||
expect(res.json).toHaveBeenCalledWith({ tag: 'injected' });
|
||||
});
|
||||
});
|
||||
|
|
@ -51,8 +51,12 @@
|
|||
},
|
||||
"onlyBuiltDependencies": [
|
||||
"better-sqlite3",
|
||||
"core-js",
|
||||
"electron",
|
||||
"esbuild"
|
||||
"electron-winstaller",
|
||||
"esbuild",
|
||||
"protobufjs",
|
||||
"sharp"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
|
|
|||
120
specs/current/daemon-http-adapter.md
Normal file
120
specs/current/daemon-http-adapter.md
Normal file
|
|
@ -0,0 +1,120 @@
|
|||
# Daemon HTTP Adapter
|
||||
|
||||
## Purpose
|
||||
|
||||
Replace the untyped `ServerContext` service-locator pattern at the daemon HTTP
|
||||
boundary with a typed **Adapter Seam** plus a typed **Deps** record. Make
|
||||
request parsing, response shaping, and error mapping the only HTTP-aware code;
|
||||
let routes become pure `(input, deps) -> Result<output>` functions whose
|
||||
interface is the test surface.
|
||||
|
||||
This work sharpens W4 (validation at boundary), W5 (modularize `server.ts`),
|
||||
and unlocks W6 (run lifecycle) in `maintainability-roadmap.md`. It does not
|
||||
re-litigate those workstreams — it provides the seam they will land against.
|
||||
|
||||
## Module shape
|
||||
|
||||
Two Adapters share one request parser:
|
||||
|
||||
- **`JsonRoute<Input, Output, Deps>`** — one-shot JSON request/response. Most
|
||||
of `/api/*`. Shipped in this PR.
|
||||
- **`StreamRoute<Input, Event, Deps>`** — SSE streams. Deferred to a follow-up
|
||||
PR that introduces the Run Orchestrator (candidate #3 in the architectural
|
||||
sweep).
|
||||
|
||||
Both consume a typed `Deps` slice. The full `Deps` record is materialized by a
|
||||
future `composeDeps()` function; routes today still receive existing
|
||||
`ctx.<domain>` slices via the unchanged `RouteDeps<K extends keyof
|
||||
ServerContext>` shape so per-route migration does not touch `server.ts`.
|
||||
|
||||
## Glossary additions
|
||||
|
||||
The following terms are pinned by this spec. They are not present in
|
||||
`docs/architecture.md` or root `AGENTS.md` today:
|
||||
|
||||
- **Request Adapter** — the module that owns request parsing, response
|
||||
shaping, and error mapping at the HTTP boundary. Lives at
|
||||
`apps/daemon/src/http/`.
|
||||
- **Json Route / Stream Route** — the two route-definition Adapters behind
|
||||
the shared parser.
|
||||
- **Deps** — the typed record of domain interfaces injected into routes.
|
||||
Replaces `ServerContext` over the course of the migration.
|
||||
|
||||
## Files added
|
||||
|
||||
```
|
||||
apps/daemon/src/http/
|
||||
types.ts # Result<T, E>, JsonRouteSpec, InputParser, Handler
|
||||
parse.ts # rawInput(req), validationError(...)
|
||||
response.ts # sendJson, sendApiError, statusForError
|
||||
origin-guard.ts # guardSameOrigin(req, origin) -> Result<void>
|
||||
adapter.ts # defineJsonRoute, mountJsonRoute
|
||||
index.ts # barrel
|
||||
```
|
||||
|
||||
## Route shape
|
||||
|
||||
```ts
|
||||
export const postActiveRoute = defineJsonRoute<Input, Output, Deps>({
|
||||
method: 'post',
|
||||
path: '/api/active',
|
||||
requireSameOrigin: true,
|
||||
parse: parsePostActive, // RouteInputContext -> Result<Input>
|
||||
handle: handlePostActive, // (Input, Deps) -> Result<Output>
|
||||
});
|
||||
```
|
||||
|
||||
`registerActiveContextRoutes(app, ctx)` is unchanged at the call site in
|
||||
`server.ts`. Internally it wires the route specs through `mountJsonRoute`.
|
||||
|
||||
## Migration order (strangler)
|
||||
|
||||
1. **Done — this PR.** Scaffold the Adapter and migrate
|
||||
`active-context-routes.ts` as the proof of pattern.
|
||||
2. **Next.** Migrate `mcp-routes.ts` (smallest remaining same-origin set;
|
||||
already uses `isLocalSameOrigin` + `sendApiError` consistently).
|
||||
3. Migrate `chat-routes.ts` — introduce `StreamRoute` here. This composes
|
||||
with the Run Orchestrator (sweep candidate #3).
|
||||
4. Migrate artifact routes — composes with the Unified Artifact Validator
|
||||
(sweep candidate #7).
|
||||
5. Migrate remaining route files.
|
||||
6. Materialize a typed `composeDeps()` and delete `server-context.ts`'s
|
||||
`ServerContext` interface once all route registrars have been migrated.
|
||||
|
||||
## Validation strategy
|
||||
|
||||
This PR validates input via small per-route `parse` functions. The schema-
|
||||
library decision (Zod / Valibot / hand-rolled) is deferred. When it lands,
|
||||
the only change is that `parse` becomes `parse: schema` for chosen-library
|
||||
routes; the Adapter's interface is unaffected because schema invocation is
|
||||
internal to the route's parse function.
|
||||
|
||||
## Tests
|
||||
|
||||
- `apps/daemon/tests/http/adapter.test.ts` — Adapter behavior: success,
|
||||
parse-fail, handle-fail, same-origin block, thrown-error coverage,
|
||||
Deps pass-through.
|
||||
- `apps/daemon/tests/active-context-routes.test.ts` — Domain handlers
|
||||
tested through the exported `postActiveRoute` / `getActiveRoute` specs;
|
||||
no Express, no supertest, no mocking of `req`/`res`.
|
||||
|
||||
The existing e2e supertest suites continue to cover the wire surface and
|
||||
serve as the regression guard for the migrated route.
|
||||
|
||||
## Wire-format note
|
||||
|
||||
The cross-origin response for the migrated route changes from the legacy
|
||||
`{ error: 'cross-origin request rejected' }` to the structured
|
||||
`{ error: { code: 'FORBIDDEN', message: 'cross-origin request rejected' } }`
|
||||
shape defined by `packages/contracts/src/errors.ts`. The contracts package
|
||||
already exports `CompatibleErrorResponse` (= `ApiErrorResponse |
|
||||
LegacyErrorResponse`), so clients that parse either shape continue to work.
|
||||
|
||||
## Out of scope (deliberately)
|
||||
|
||||
- StreamRoute / SSE-aware Adapter. Lands with the Run Orchestrator follow-up.
|
||||
- A typed `composeDeps()` that replaces `ServerContext` wholesale. Lands when
|
||||
enough routes have migrated that the `any`-bag is mostly empty.
|
||||
- A new validation library. Deferred until the migration pattern is proven
|
||||
across at least three route files.
|
||||
- Express → Fastify swap (roadmap W12). Unaffected by this work.
|
||||
Loading…
Reference in a new issue