Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/agent-bff/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"test": "jest"
},
"dependencies": {
"@forestadmin/agent-client": "1.9.0",
"@forestadmin/forestadmin-client": "1.40.3",
"@koa/bodyparser": "^6.1.0",
"jsonwebtoken": "^9.0.3",
Expand Down
15 changes: 15 additions & 0 deletions packages/agent-bff/src/adapters/console-metrics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import type { Logger } from '../ports/logger-port';
import type { MetricTags, Metrics } from '../ports/metrics-port';

import createConsoleLogger from './console-logger';

export default function createConsoleMetrics(logger: Logger = createConsoleLogger()): Metrics {
return {
increment(name: string, tags?: MetricTags): void {
logger('Info', 'metric.increment', { metric: name, ...(tags ?? {}) });
},
gauge(name: string, value: number, tags?: MetricTags): void {
logger('Info', 'metric.gauge', { metric: name, value, ...(tags ?? {}) });
},
};
}
6 changes: 6 additions & 0 deletions packages/agent-bff/src/ports/metrics-port.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export type MetricTags = Record<string, string | number>;

export interface Metrics {
increment(name: string, tags?: MetricTags): void;
gauge(name: string, value: number, tags?: MetricTags): void;
}
55 changes: 55 additions & 0 deletions packages/agent-bff/src/read-model/action-endpoint-resolver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import type ReadModel from './read-model';
import type { Metrics } from '../ports/metrics-port';
import type { ActionEndpointsByCollection } from '@forestadmin/agent-client';

export const ACTION_ENDPOINT_MISS = 'action_endpoint_miss';
export const ACTION_ENDPOINT_ERROR = 'action_endpoint_error';

export type ActionEndpointInfo = ActionEndpointsByCollection[string][string];

export type ReadModelProvider = () => Promise<ReadModel>;

export interface ResolveActionContext {
rendering: string | number;
}

/**
* Resolves an action to its endpoint against the current read-model. Never throws: an absent
* mapping emits the miss counter, a failure to obtain the read-model emits the error counter.
* Both counters carry `rendering`, `collection`, and `action` tags. In normal flow the action
* allow-list already excludes endpoint-less actions, so the miss path is a defensive guard.
*/
export default class ActionEndpointResolver {
constructor(
private readonly getReadModel: ReadModelProvider,
private readonly metrics: Metrics,
) {}

async resolve(
collection: string,
action: string,
{ rendering }: ResolveActionContext,
): Promise<ActionEndpointInfo | undefined> {
const tags = { rendering, collection, action };

let readModel: ReadModel;

try {
readModel = await this.getReadModel();
} catch {
this.metrics.increment(ACTION_ENDPOINT_ERROR, tags);

return undefined;
}

const info = readModel.getActionEndpoints()[collection]?.[action];

if (!info) {
this.metrics.increment(ACTION_ENDPOINT_MISS, tags);

return undefined;
}

return info;
}
}
21 changes: 21 additions & 0 deletions packages/agent-bff/src/read-model/agent-capabilities-fetcher.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import type { CapabilitiesFetcher } from './capabilities-cache';

import { createRemoteAgentClient } from '@forestadmin/agent-client';

export interface AgentCapabilitiesFetcherOptions {
agentUrl: string;
token: string;
}

/**
* Builds a capabilities fetcher bound to a request's agent token. The cache calls it only on a
* miss, so the token of whichever request first populates a collection is the one used.
*/
export default function createAgentCapabilitiesFetcher({
agentUrl,
token,
}: AgentCapabilitiesFetcherOptions): CapabilitiesFetcher {
const client = createRemoteAgentClient({ url: agentUrl, token });

return collection => client.collection(collection).capabilities();
}
68 changes: 68 additions & 0 deletions packages/agent-bff/src/read-model/capabilities-cache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { ONE_DAY_MS } from './schema-cache';

export interface CapabilitiesResult {
fields: { name: string; type: string; operators: string[] }[];
}

export type CapabilitiesFetcher = (collection: string) => Promise<CapabilitiesResult>;

export interface CapabilitiesCacheOptions {
now?: () => number;
ttlMs?: number;
}

interface CacheEntry {
result: CapabilitiesResult;
fetchedAt: number;
}

/**
* Per-collection capabilities cache (24h). The fetcher is passed per call so it can be bound to
* the caller's request token. Invalidated together with the schema via `clear()`.
*/
export default class CapabilitiesCache {
private readonly now: () => number;
private readonly ttlMs: number;

private readonly entries = new Map<string, CacheEntry>();
private readonly inFlight = new Map<string, Promise<CapabilitiesResult>>();
private generation = 0;

constructor({ now = Date.now, ttlMs = ONE_DAY_MS }: CapabilitiesCacheOptions = {}) {
this.now = now;
this.ttlMs = ttlMs;
}

async get(collection: string, fetcher: CapabilitiesFetcher): Promise<CapabilitiesResult> {
const entry = this.entries.get(collection);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache is keyed by collection only while the token is bound in the fetcher, so the first caller result is served to every token for 24h. Can we confirm /capabilities is not token-scoped server-side? If it is, key by token.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed not token-scoped. /forest/_internal/capabilities (packages/agent/src/routes/capabilities.ts, fetchCapabilities) derives operators from each field's filterOperators on the datasource schema — it does not read the caller / rendering / permissions. It's a PrivateRoute (needs a valid JWT) but the response is identical for every caller, so process-wide keying by collection is correct. Kept as-is.

if (entry && this.now() - entry.fetchedAt < this.ttlMs) return entry.result;

const pending = this.inFlight.get(collection);
if (pending) return pending;

const { generation } = this;
const fetch = fetcher(collection)
.then(result => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike SchemaCache.doRefresh, this has no .catch: a failing capabilities() propagates raw with no error counter and no last-good fallback. What about mirroring the schema cache here (emit a counter, serve stale)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept as-is, on purpose:

  1. The ticket's counter contract only names the schema + action-endpoint counters — a capabilities counter would be new contract.
  2. "Serve stale" is unsafe for capabilities specifically: stale operators would make Slice 4 validate filters against the wrong operator set. And there's no last-good on the first fetch anyway.
  3. The current behavior already avoids poisoning — the rejection propagates, the in-flight entry is cleared, and the next read re-attempts. The failure belongs to the request-scoped consumer (Slice 3/4), which has the context to map it to HTTP.

Happy to add a capabilities failure counter in Slice 4 if ops wants the signal.

// Skip the write if a clear() (schema refresh) happened while this fetch was in flight —
// its result belongs to the old schema generation and must not repopulate the cache.
if (this.generation === generation) {
this.entries.set(collection, { result, fetchedAt: this.now() });
}

return result;
})
.finally(() => {
if (this.inFlight.get(collection) === fetch) this.inFlight.delete(collection);
});

this.inFlight.set(collection, fetch);

return fetch;
}

clear(): void {
this.generation += 1;
this.entries.clear();
this.inFlight.clear();
}
}
42 changes: 42 additions & 0 deletions packages/agent-bff/src/read-model/create-read-model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import type { Logger } from '../ports/logger-port';
import type { Metrics } from '../ports/metrics-port';

import ActionEndpointResolver from './action-endpoint-resolver';
import CapabilitiesCache from './capabilities-cache';
import ForestSchemaClient from './forest-schema-client';
import ReadModelStore from './read-model-store';
import SchemaCache from './schema-cache';
import createConsoleMetrics from '../adapters/console-metrics';

export interface CreateReadModelOptions {
forestServerUrl: string;
envSecret: string;
metrics?: Metrics;
logger?: Logger;
now?: () => number;
}

export interface ReadModelBundle {
store: ReadModelStore;
actionEndpointResolver: ActionEndpointResolver;
}

export default function createReadModel({
forestServerUrl,
envSecret,
metrics,
logger,
now,
}: CreateReadModelOptions): ReadModelBundle {
const resolvedMetrics = metrics ?? createConsoleMetrics(logger);
const fetcher = new ForestSchemaClient({ forestServerUrl, envSecret });
const schemaCache = new SchemaCache({ fetcher, metrics: resolvedMetrics, now });
const capabilitiesCache = new CapabilitiesCache({ now });
const store = new ReadModelStore(schemaCache, capabilitiesCache);
const actionEndpointResolver = new ActionEndpointResolver(
() => store.getReadModel(),
resolvedMetrics,
);

return { store, actionEndpointResolver };
}
9 changes: 9 additions & 0 deletions packages/agent-bff/src/read-model/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export default class SchemaUnavailableError extends Error {
readonly cause?: unknown;

constructor(cause?: unknown) {
super('The agent schema is unavailable');
this.name = 'SchemaUnavailableError';
this.cause = cause;
}
}
24 changes: 24 additions & 0 deletions packages/agent-bff/src/read-model/forest-schema-client.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type { ForestSchemaCollection } from '@forestadmin/forestadmin-client';

import { ForestHttpApi, SchemaService } from '@forestadmin/forestadmin-client';

export interface ForestSchemaClientOptions {
forestServerUrl: string;
envSecret: string;
}

export interface SchemaFetcher {
fetchSchema(): Promise<ForestSchemaCollection[]>;
}

export default class ForestSchemaClient implements SchemaFetcher {
private readonly schemaService: SchemaService;

constructor({ forestServerUrl, envSecret }: ForestSchemaClientOptions) {
this.schemaService = new SchemaService(new ForestHttpApi(), { forestServerUrl, envSecret });
}

async fetchSchema(): Promise<ForestSchemaCollection[]> {
return this.schemaService.getSchema();
}
}
54 changes: 54 additions & 0 deletions packages/agent-bff/src/read-model/read-model-store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import type CapabilitiesCache from './capabilities-cache';
import type { CapabilitiesFetcher, CapabilitiesResult } from './capabilities-cache';
import type SchemaCache from './schema-cache';

import ReadModel from './read-model';

/**
* Single owner of the coupled schema + capabilities lifecycle. A successful schema refresh (a bump
* of the cache `revision`) rebuilds the read-model and clears capabilities atomically, so the
* allow-list and capabilities never split-brain across schema generations.
*/
export default class ReadModelStore {
private readonly schemaCache: SchemaCache;
private readonly capabilitiesCache: CapabilitiesCache;

private builtRevision = -1;
private readModel: ReadModel | null = null;

constructor(schemaCache: SchemaCache, capabilitiesCache: CapabilitiesCache) {
this.schemaCache = schemaCache;
this.capabilitiesCache = capabilitiesCache;
}

async getReadModel(): Promise<ReadModel> {
const collections = await this.schemaCache.get();

if (this.schemaCache.revision !== this.builtRevision || !this.readModel) {
this.readModel = new ReadModel(collections);
this.builtRevision = this.schemaCache.revision;
this.capabilitiesCache.clear();
}

return this.readModel;
}

async getCapabilities(
collection: string,
fetcher: CapabilitiesFetcher,
): Promise<CapabilitiesResult> {
// Ensure any pending schema refresh (and its capabilities invalidation) runs first.
await this.getReadModel();

// TODO(wiring): possible TOCTOU once this is called from request handling. A concurrent schema
// refresh can clear capabilities while this fetch is in flight, so the caller could receive
// capabilities from the previous schema generation alongside the new allow-list. When wiring
// the data endpoints, re-check `schemaCache.revision` after the fetch resolves and retry on a
// mismatch so capabilities and schema stay atomically coupled.
return this.capabilitiesCache.get(collection, fetcher);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium read-model/read-model-store.ts:43

getCapabilities() can return capabilities from the wrong schema generation. await this.getReadModel() only checks the revision before the capabilities fetch starts — if another request triggers a schema refresh and capabilitiesCache.clear() runs while this.capabilitiesCache.get(collection, fetcher) is still in flight, the caller receives capabilities tied to the old schema alongside the new read-model/allow-list. Consider re-checking schemaCache.revision after the capabilities fetch resolves and retrying when it has changed, so capabilities and schema stay atomically coupled.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/agent-bff/src/read-model/read-model-store.ts around line 43:

`getCapabilities()` can return capabilities from the wrong schema generation. `await this.getReadModel()` only checks the revision before the capabilities fetch starts — if another request triggers a schema refresh and `capabilitiesCache.clear()` runs while `this.capabilitiesCache.get(collection, fetcher)` is still in flight, the caller receives capabilities tied to the old schema alongside the new read-model/allow-list. Consider re-checking `schemaCache.revision` after the capabilities fetch resolves and retrying when it has changed, so capabilities and schema stay atomically coupled.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid TOCTOU. Deferring the actual fix to the data-endpoint wiring rather than adding it here, for three reasons:

  1. This method is not called from request handling yet (that's the data-endpoints slice).
  2. Cache poisoning is already prevented: a generation guard means a fetch that started before a clear() never writes to the cache, so no stale entry is served from cache after a refresh (the ticket's coupling AC).
  3. The residual window — a single caller receiving previous-generation capabilities while a refresh happens mid-flight — is near-nil impact: it needs a concurrent refresh exactly at the 24h boundary during an in-flight capabilities fetch AND the operators to actually differ between the two schema generations.

I've added a code note at the call site to re-check schemaCache.revision after the fetch and retry on a mismatch when the endpoints are wired, so the fix lands where the method is actually consumed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed reasoning—makes sense to defer given the method isn't called from production yet and the cache generation guard already prevents stale entries. Would you like me to implement the retry-on-revision-mismatch logic now in getCapabilities(), or are you handling it when you wire the endpoints?

}

ageSeconds(): number | undefined {
return this.schemaCache.ageSeconds();
}
}
Loading
Loading