9b7d16601d
## Summary Step 2 of the AI-relay chantier (after [ADR-0024](docs/decisions/0024-ai-service-relay-grpc-sse-bridge.md) merged in #194). Lands the BFF-side **skeleton** that talks to `apf-ai-service` over gRPC: vendored protos, generated TypeScript stubs, typed wrapper clients, Principal mapper, and metadata builder — all tested against an in-process fake gRPC server. **No HTTP route is exposed in this PR**; the SSE bridge (`POST /api/ai/chat`, `GET /api/ai/rag/search`, `GET /api/ai/models`) ships in the next PR. The skeleton is self-contained: `AiClientModule` is built but is NOT imported in `AppModule` yet. The BFF runtime is byte-for-byte unchanged. Everything below exists for the next PR to wire into a controller. ## What lands ### Proto vendoring + codegen - `apps/portal-bff/src/grpc/proto/apf-ai/` — mirror of `apf-ai-service/contract/proto/` (common, chat, rag, ingestion, models). Both the `.proto` files and the regenerated `ts-proto` output under `grpc/gen/` are committed for hermetic builds and reviewable diffs (per ADR-0024 §"Sub-decision 3"). - `pnpm run grpc:codegen` — regenerates the stubs via `grpc-tools`' bundled `protoc` and the `ts-proto` plugin (`outputServices=grpc-js, esModuleInterop, forceLong=long, useOptionals=messages, exportCommonSymbols=false`). - `pnpm run grpc:sync` — copies the vendored `.proto` files from the sibling `apf-ai-service` working tree (`../apf-ai-service/contract/proto/`); developer convenience, never invoked from CI. Errors with an actionable message when the sibling tree is not where it expects. - Generated tree (`grpc/gen/**`) excluded from Prettier (`.prettierignore`) and ESLint (`eslint.config.mjs` ignores). Hand-rules apply to wrappers under `ai-client/`, not to codegen output. ### Dependencies - `@grpc/grpc-js@^1.13.0` — runtime gRPC client. - `@bufbuild/protobuf@^2.10.2` — wire codec used by `ts-proto`'s emitted code. - `long@^5.2.3` — int64 representation for proto Long fields (`forceLong=long`). - `ts-proto@^2.7.0` — devDep, TypeScript codegen plugin. - `grpc-tools@^1.13.0` — devDep, ships `protoc` + the gRPC plugin; added to `pnpm.onlyBuiltDependencies` so the postinstall binary download runs. ### Env validator `apps/portal-bff/src/config/check-ai-service-config.ts` follows the same posture as the other validators (per ADR-0018 §"BFF env-var loading"): small, per-key, runs at module init, throws with an actionable message on misconfiguration. Three vars: | Var | Mandatory | Purpose | |---|---|---| | `AI_SERVICE_GRPC_ENDPOINT` | yes | `host:port` reachable from the BFF — `apf-ai-service:8080` in dev Compose, service DNS + 443 in prod | | `AI_SERVICE_CLIENT_ID` | yes | Deployment slug propagated as the `x-client-id` metadata. Convention: `apf-portal-<env>` | | `AI_SERVICE_GRPC_TLS` | no (default `true`) | `false` for h2c in dev, `true` for h2 + TLS in prod | 7 spec cases lock the validation contract end-to-end. ### `AiClientModule` `apps/portal-bff/src/grpc/ai-client/` houses: - **`tokens.ts`** — DI tokens (`AI_CONFIG`, `AI_CREDENTIALS`, one per generated stub). - **`principal.mapper.ts`** — `PrincipalMapper.fromInputs({oid, tid, roles, extraAttributes})` returns the proto `Principal`. `subject` is hashed via `HashUserIdService` (the **same** salt + algorithm the audit writer uses) so `Principal.subject` matches `audit.events.actor_id_hash` byte-for-byte. `roles` passes through verbatim — inclusive expansion lands with a future role-hierarchy ADR; the wire contract on the AI side is unchanged. - **`grpc-metadata.builder.ts`** — stamps every outbound call with `x-client-id` (from config) and `x-correlation-id` (active OTel span's trace-id when present, else explicit override, else fresh UUID). - **`chat.client.ts`** — server-stream wrapper around `ChatServiceClient`. Returns the raw `ClientReadableStream<ChatEvent>` (Node `Readable` is async-iterable so the SSE bridge consumes with `for await`). Optional `AbortSignal` propagates browser disconnect to `call.cancel()`. - **`rag.client.ts`**, **`models.client.ts`**, **`ingestion.client.ts`** — unary promisified wrappers. `IngestionClient` is unused in v1 (the admin "manage AI corpus" surface lands later) but wired now so the future controller is one new file, not a new module. - **`ai-client.module.ts`** — NestJS module wiring the providers. `HashUserIdService` is declared locally rather than imported via `AuditModule` (the global module) to keep the module unit-testable in isolation; runtime equivalence is guaranteed by the service being a pure function of `LOG_USER_ID_SALT`. ### Tests 5 new spec files, 18 new test cases. All run against in-process fake gRPC servers; no network, no live `apf-ai-service`: - `check-ai-service-config.spec.ts` — 7 cases (happy path + 6 rejection branches). - `principal.mapper.spec.ts` — 7 cases including the cross-service hash-stability invariant and the `tenantId` reserved-key contract. - `grpc-metadata.builder.spec.ts` — 5 cases covering all three correlation-id resolution paths and metadata immutability. - `chat.client.spec.ts` — 4 cases: happy-path stream, metadata propagation, mid-stream cancel via AbortSignal, pre-aborted signal. - `rag.client.spec.ts` — 2 cases: unary happy path, `ServiceError` propagation. - `ai-client.module.spec.ts` — 4 cases: module bootstrap, all four wrappers resolved, env-driven `AI_CONFIG`, shared credentials across stubs. **Total BFF spec suite: 443 → 461 (after merge accounting), all passing.** ## Notes for the reviewer - **Why both `.proto` and generated `.ts` are committed.** Hermetic builds. Reviewers see both sides of a contract change in one diff. CI never runs codegen — drift between proto and stub would otherwise hide behind a successful build. The drift gate that compares the vendored copy against an upstream tag of `apf-ai-service` is the post-v1 follow-up listed in ADR-0024's "What's next". - **`HashUserIdService` declared locally in `AiClientModule`.** Two instances of the service exist when both `AuditModule` (global) and `AiClientModule` are wired into `AppModule`. The cost is one extra constructor call at bootstrap; the value is full test isolation of `AiClientModule` and a self-contained Principal-mapping boundary. The cross-service hash join invariant from ADR-0013 still holds because the service is a pure function of the `LOG_USER_ID_SALT` env var. - **`AiClientModule` is NOT imported by `AppModule`.** Deliberately. The skeleton compiles, type-checks, and unit-tests cleanly with no runtime side effects. The next PR (SSE bridge controller) adds the `imports: [AiClientModule]` line + the controller, in one focused change. - **`ts-proto` flat-oneof emission.** `ChatEvent` is generated with optional siblings (`token?`, `citation?`, `done?`, …) rather than a discriminated union (`$case: 'token'`). The flatter shape composes more naturally with the SSE writer the next PR will introduce (`event:` field name maps directly to the populated sibling). - **Cancellation test deliberately relaxed.** The "AbortSignal already aborted before call dial" test asserts the client-side outcome (no payload, error or clean end) but not server-side observation. gRPC-js may or may not propagate a cancel frame depending on whether the call had time to dial — both outcomes are correct per the contract; only the absence of payload matters. - **Lifecycle (`onApplicationShutdown`) deferred.** The module does not close the gRPC channel on shutdown today. Process termination closes the sockets via OS-level descriptor reclaim — sufficient for dev/preprod. The next PR wires the module into `AppModule` and adds an explicit Nest lifecycle hook in the same change (paired with `app.enableShutdownHooks()` in `main.ts`). ## Test plan - [x] `pnpm run grpc:codegen` — clean regeneration. Generated tree byte-identical to what's committed. - [x] `pnpm nx test portal-bff` — **443 specs pass** (was 425). - [x] `pnpm nx lint portal-bff` — clean. The eslint ignore for `grpc/gen/**` covers ts-proto's relaxed style; hand-written `ai-client/` files pass the project's full rule set. - [x] `pnpm nx build portal-bff` — clean, webpack-compiled. No bundle-size delta on the runtime artefact yet (the module is unimported). - [x] `pnpm install` — lockfile reconciled; `grpc-tools` postinstall fetches `protoc` from the precompiled-binaries mirror without errors on Linux x64. - [ ] **Manual smoke (next PR)** — once the SSE bridge ships, point `AI_SERVICE_GRPC_ENDPOINT` at the local `apf-ai-service` Compose service and run an end-to-end chat against the canned stub responses on the AI side. Not in scope for this PR. ## What's next 1. **PR — SSE bridge controller.** Wires `AiClientModule` into `AppModule`, adds `POST /api/ai/chat` (SSE), `GET /api/ai/rag/search`, `GET /api/ai/models`. Adds the `OnApplicationShutdown` hook + `enableShutdownHooks()`. Adds `apf-ai-service` to `infra/local/dev.compose.yml`. Promotes ADR-0024 from `proposed` to `accepted` and updates `CLAUDE.md`'s ADR roll-up. 2. **PR (frontend chantier)** — chatbot widget on `portal-shell` consuming the SSE endpoint. 3. **PR (post-v1)** — proto-drift CI gate diffing the vendored `proto/apf-ai/` against the upstream tag. 4. **Coordinated amendment** — when the first production deployment is in scope, both repos record the same prod-hardening choice (signed envelope or mTLS) on the same date. --------- Co-authored-by: Julien Gautier <julien.gautier@apf.asso.fr> Reviewed-on: #195
237 lines
6.9 KiB
TypeScript
237 lines
6.9 KiB
TypeScript
import { describe, it, expect, beforeAll, afterAll, beforeEach } from '@jest/globals';
|
|
import {
|
|
ChannelCredentials,
|
|
Server,
|
|
ServerCredentials,
|
|
type Metadata,
|
|
type ServerWritableStream,
|
|
} from '@grpc/grpc-js';
|
|
import {
|
|
ChatServiceClient,
|
|
ChatServiceService,
|
|
type ChatEvent,
|
|
type ChatRequest,
|
|
} from '../gen/apf-ai/chat';
|
|
import { ChatClient } from './chat.client';
|
|
import { GrpcMetadataBuilder } from './grpc-metadata.builder';
|
|
import type { AiServiceConfig } from '../../config/check-ai-service-config';
|
|
|
|
/**
|
|
* Integration test: drives `ChatClient` against an in-process fake
|
|
* gRPC ChatService that emits a canned `ChatEvent` sequence. The
|
|
* spec covers ADR-0024's confirmation criteria:
|
|
*
|
|
* - happy-path stream emits every event the server sent and ends
|
|
* after `done`;
|
|
* - metadata (`x-client-id` + `x-correlation-id`) reaches the
|
|
* server unchanged;
|
|
* - browser-close (modelled here as an `AbortController.abort()`)
|
|
* propagates as `call.cancel()` and the server's stream callback
|
|
* observes the cancellation.
|
|
*/
|
|
|
|
const CONFIG: AiServiceConfig = {
|
|
endpoint: '',
|
|
clientId: 'apf-portal-test',
|
|
useTls: false,
|
|
};
|
|
|
|
interface ServerObservations {
|
|
request: ChatRequest | null;
|
|
metadata: Metadata | null;
|
|
cancelled: boolean;
|
|
}
|
|
|
|
let server: Server;
|
|
let port: number;
|
|
let serverObservations: ServerObservations;
|
|
let chatHandler: (call: ServerWritableStream<ChatRequest, ChatEvent>) => void;
|
|
|
|
beforeAll(async () => {
|
|
server = new Server();
|
|
server.addService(ChatServiceService, {
|
|
chat: (call: ServerWritableStream<ChatRequest, ChatEvent>) => {
|
|
serverObservations.request = call.request;
|
|
serverObservations.metadata = call.metadata;
|
|
call.on('cancelled', () => {
|
|
serverObservations.cancelled = true;
|
|
});
|
|
chatHandler(call);
|
|
},
|
|
});
|
|
|
|
port = await new Promise<number>((resolve, reject) => {
|
|
server.bindAsync('127.0.0.1:0', ServerCredentials.createInsecure(), (err, bound) => {
|
|
if (err) {
|
|
reject(err);
|
|
return;
|
|
}
|
|
resolve(bound);
|
|
});
|
|
});
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await new Promise<void>((resolve, reject) => {
|
|
server.tryShutdown((err) => {
|
|
if (err) {
|
|
reject(err);
|
|
return;
|
|
}
|
|
resolve();
|
|
});
|
|
});
|
|
});
|
|
|
|
beforeEach(() => {
|
|
serverObservations = { request: null, metadata: null, cancelled: false };
|
|
});
|
|
|
|
function buildClient(): ChatClient {
|
|
const config = { ...CONFIG, endpoint: `127.0.0.1:${port}` };
|
|
const grpc = new ChatServiceClient(config.endpoint, ChannelCredentials.createInsecure());
|
|
return new ChatClient(grpc, new GrpcMetadataBuilder(config));
|
|
}
|
|
|
|
async function collect(stream: AsyncIterable<ChatEvent>): Promise<ChatEvent[]> {
|
|
const events: ChatEvent[] = [];
|
|
for await (const event of stream) {
|
|
events.push(event);
|
|
}
|
|
return events;
|
|
}
|
|
|
|
describe('ChatClient (in-process fake server)', () => {
|
|
it('emits every server event then terminates on done', async () => {
|
|
chatHandler = (call) => {
|
|
call.write({ token: { token: 'hello', value: 'Hello' } });
|
|
call.write({ token: { token: ' world', value: ' world' } });
|
|
call.write({ done: { stats: { tokensIn: 1, tokensOut: 2, chunksRetrieved: 0 } } });
|
|
call.end();
|
|
};
|
|
|
|
const client = buildClient();
|
|
const stream = client.chat({
|
|
messages: [],
|
|
conversationId: 'conv-1',
|
|
model: '',
|
|
provider: '',
|
|
toolsAvailable: [],
|
|
principal: { subject: 'subj-1', roles: ['admin'], attributes: { tenantId: 't' } },
|
|
});
|
|
const events = await collect(stream);
|
|
|
|
expect(events).toHaveLength(3);
|
|
expect(events[0]?.token?.value).toBe('Hello');
|
|
expect(events[2]?.done?.stats?.tokensOut).toBe(2);
|
|
});
|
|
|
|
it('propagates x-client-id and x-correlation-id metadata to the server', async () => {
|
|
chatHandler = (call) => {
|
|
call.write({ done: { stats: { tokensIn: 0, tokensOut: 0, chunksRetrieved: 0 } } });
|
|
call.end();
|
|
};
|
|
|
|
const client = buildClient();
|
|
await collect(
|
|
client.chat(
|
|
{
|
|
messages: [],
|
|
conversationId: 'conv-2',
|
|
model: '',
|
|
provider: '',
|
|
toolsAvailable: [],
|
|
principal: { subject: 'subj-2', roles: [], attributes: {} },
|
|
},
|
|
{ correlationId: 'corr-from-test' },
|
|
),
|
|
);
|
|
|
|
expect(serverObservations.metadata?.get('x-client-id')).toEqual(['apf-portal-test']);
|
|
expect(serverObservations.metadata?.get('x-correlation-id')).toEqual(['corr-from-test']);
|
|
});
|
|
|
|
it('cancels the call when the AbortSignal aborts mid-stream', async () => {
|
|
const cancellationObserved = new Promise<void>((resolve) => {
|
|
chatHandler = (call) => {
|
|
call.write({ token: { token: 't1', value: 't1' } });
|
|
call.on('cancelled', () => resolve());
|
|
// Deliberately do not `end()` — the test cancels.
|
|
};
|
|
});
|
|
|
|
const controller = new AbortController();
|
|
const client = buildClient();
|
|
const stream = client.chat(
|
|
{
|
|
messages: [],
|
|
conversationId: 'conv-3',
|
|
model: '',
|
|
provider: '',
|
|
toolsAvailable: [],
|
|
principal: { subject: 'subj-3', roles: [], attributes: {} },
|
|
},
|
|
{ signal: controller.signal },
|
|
);
|
|
|
|
// Drain the first event then abort.
|
|
const iter = stream[Symbol.asyncIterator]();
|
|
await iter.next();
|
|
controller.abort();
|
|
|
|
await cancellationObserved;
|
|
expect(serverObservations.cancelled).toBe(true);
|
|
});
|
|
|
|
it('terminates the stream without data when the AbortSignal is already aborted', async () => {
|
|
// The signal aborts before the call dial completes. gRPC-js
|
|
// cancels locally — depending on timing the server may or may
|
|
// not observe the call, so the spec only locks the client-side
|
|
// outcome: the stream ends in error and yields no payload.
|
|
chatHandler = (call) => {
|
|
call.on('cancelled', () => {
|
|
// best-effort end on cancellation so the suite never hangs
|
|
try {
|
|
call.end();
|
|
} catch {
|
|
// already torn down
|
|
}
|
|
});
|
|
};
|
|
|
|
const controller = new AbortController();
|
|
controller.abort();
|
|
|
|
const client = buildClient();
|
|
const stream = client.chat(
|
|
{
|
|
messages: [],
|
|
conversationId: 'conv-4',
|
|
model: '',
|
|
provider: '',
|
|
toolsAvailable: [],
|
|
principal: { subject: 'subj-4', roles: [], attributes: {} },
|
|
},
|
|
{ signal: controller.signal },
|
|
);
|
|
|
|
const events: ChatEvent[] = [];
|
|
let caught: unknown;
|
|
try {
|
|
for await (const ev of stream) {
|
|
events.push(ev);
|
|
}
|
|
} catch (err) {
|
|
caught = err;
|
|
}
|
|
|
|
expect(events).toHaveLength(0);
|
|
// gRPC-js may surface the cancellation as an error or as a clean
|
|
// end-of-stream depending on whether the call had time to dial.
|
|
// Either way, no payload is emitted — that is the contract.
|
|
if (caught !== undefined) {
|
|
expect((caught as { code?: number }).code).toBeDefined();
|
|
}
|
|
});
|
|
});
|