feat(app-admin): AI mode in the command palette - #5589
Open
adrians5j wants to merge 49 commits into
Open
Conversation
Adds incremental HTTP response delivery to Webiny, plus a File Manager "Re-enrich with AI" action that exercises it end to end. Core (@webiny/event-handler-core) Introduces `HttpStreamBody`, an explicit marker a route wraps its source in to opt into streaming. Additive: `IHttpResponse.body` was already `any`, so existing routes are untouched. Transports that can stream write chunks as produced; those that cannot call `collect()`. Self-hosted (@webiny/event-handler-server) Streams via `res.flushHeaders()` + per-chunk writes, honouring back-pressure and client disconnects. Also fixes a latent bug the streaming path made reachable: the error handler called `writeHead(500)` unconditionally, which throws ERR_HTTP_HEADERS_SENT once headers are out and masks the real error. AWS (@webiny/event-handler-aws) API Gateway cannot stream — it buffers the whole Lambda response regardless of how it was produced — so streaming requires a Lambda Function URL with `InvokeMode: RESPONSE_STREAM`, and a Lambda's handler entry is fixed per function. Hence a second function off the same bundle (`handler.streamHandler`) with its own transport: Function URL event type, translator, terminal handler writing to the response stream, and `createStreamLambdaHandler`. `streamifyResponse` is applied eagerly, because the runtime inspects the exported handler for the mark it attaches; a lazy wrap would silently fall back to buffered responses. The existing API Gateway translator now drains a streaming body instead of failing, so a streaming route still works over that transport as one buffered response. Auth Adds `x-webiny-authorization`, read ahead of `Authorization` by both the AWS and self-hosted extractors. Behind CloudFront with Origin Access Control, SigV4 occupies `Authorization`, so a viewer bearer token cannot survive to the origin. Using a separate header keeps the Function URL private (AWS_IAM + OAC) without depending on that interaction. Infra (@webiny/project-aws) `ApiGraphqlStream` (Lambda + Function URL, reusing the graphql IAM role, 300s timeout), a CloudFront OAC, a `/stream/*` cache behavior ordered first with `compress: false` — compression buffers chunks and defeats incremental delivery — and an invoke permission scoped to the distribution ARN. File Manager Extracts the AI enrichment logic out of `AiImageEnrichmentTask` into `Prepare`/`Apply` use cases shared with a new SSE route, so the task and the route cannot drift. The route resolves everything it can before opening the stream, so missing file / non-image / no provider / license come back as real status codes rather than buried in a 200. Frontend adds `ApiStreamClient` and a generic `readServerSentEvents` reader, with auth and tenant decorators mirroring the GraphQL client. Not yet verified against a real deployment: whether CloudFront passes chunks through unbuffered, and that `handler.streamHandler` resolves in the built bundle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The streaming client sends its auth token in `x-webiny-authorization`, but `SecureHeadersDecorator` never listed that header in `Access-Control-Allow-Headers`. The preflight returned 204 while failing the CORS check, so the browser blocked the actual request before sending it — surfacing as an opaque "Failed to fetch" with no response headers rather than a 4xx. Adds a test asserting every custom request header Webiny clients send is present in the allow-list, so the next one can't slip through the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects found by inspecting the built api artifact before deploying.
The first two would each have broken the response-streaming Lambda; the
third broke every non-Lambda import of the bundle.
1. rspack tree-shook `streamHandler` away
Nothing imports an entry's exports, so the unused one was dropped along
with every module reachable only from it — the bundle exported just
`handler` and contained no streaming code at all. Declaring the entry a
module library marks its exports as the public API and keeps them.
`handler` survived only by accident of being first.
2. The WCP telemetry wrapper re-exported only `handler`
`WcpInjectTelemetryClientAfterBuild` renames the bundle to `_handler.mjs`
and puts a downloaded telemetry wrapper in its place. That wrapper knows
only about `handler`, so `handler.streamHandler` — what the Pulumi config
points the streaming function at — did not exist in the deployed artifact.
The re-export is unwrapped: `streamHandler` carries the marker
`streamifyResponse` attaches and the runtime inspects the exported function
for it, so wrapping it would silently downgrade the function to buffered
responses. It goes through a namespace import rather than a named
re-export because this injection also runs for the self-hosted api build,
whose bundle has no `streamHandler`, and a named re-export of a missing
binding is a hard ESM error that took the whole handler down.
3. The streaming-runtime check tested the wrong thing
`@aws/lambda-invoke-store`, transitive via the AWS SDK, runs
`globalThis.awslambda = globalThis.awslambda || {}` at import time. In
Lambda that preserves the runtime's real global, but everywhere else it
leaves an EMPTY object — so checking the object's presence passed outside
Lambda and `streamifyResponse` then threw at module load, failing the
import of the entire bundle including the buffered handler. Checks for the
function now.
Verified on the built artifacts of both hosting types: AWS exports
`handler` + `streamHandler` (2.83 MB minified, under the 4.5 MB cap),
self-hosted exports `handler` with `streamHandler` undefined, and both
import cleanly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ront Three defects, all found by measuring a real deployment rather than reading the code. Response streaming now works end to end on AWS. 1. Missing IAM permission (403 on every request) CloudFront could not invoke the Function URL at all: Lambda's authorizer denied every signed request with AccessDeniedException and the function was never invoked. The OAC-for-Lambda docs require TWO permission statements — `lambda:InvokeFunctionUrl` AND `lambda:InvokeFunction` — and only the former was granted. 2. The prelude was never flushed (empty 200, no headers) The runtime emits the response prelude LAZILY, on the first write to the stream. A response that writes nothing — a CORS preflight is 204 with no body — therefore sent no prelude at all, and Lambda substituted a default 200 with `application/octet-stream` and none of the route's headers. It failed silently: status success, nothing logged, and a direct streaming invoke returned 0 bytes. Every path now guarantees one write, including a stream that yields no chunks. Verified against the deployed function: the same invoke now returns the prelude JSON plus its 8-byte delimiter. This is what made the preflight "succeed" while the browser still reported a CORS failure — it arrived header-less. 3. Legacy forwardedValues on the streaming behavior Replaced with Managed-CachingDisabled plus an origin request policy. The policy also forwards `Access-Control-Request-Method` and `Access-Control-Request-Headers`, without which the origin cannot build a correct preflight response. Also adds a loud warning when `streamifyResponse` is unavailable inside Lambda. That state is otherwise invisible: the handler is invoked buffered and returns header-less empty responses that look like a working stream. Note for future streaming routes: OAC does not sign the request body, so a route that POSTs a body needs the client to send `x-amz-content-sha256`. The enrich route sends no body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three conflicts, resolved as follows. `SecureHeadersDecorator` — next dropped `x-apollo-tracing` and `apollo-query-plan-experimental` from the CORS allow-list. Kept their removal and re-applied only `x-webiny-authorization` on top. `FileActions` — next added an `editImage` action, this branch added `reenrichWithAi`. Both kept. `createWebinyApiHandler` — next added `EventBridgeEventType` + `BulkActionsEventBridgeLambdaHandlerFeature`, removed `WebSocketLambdaHandler`, and swapped the auth decorator order so TENANT is established before IDENTITY (API-key auth resolves the key by tenant partition). All taken. Their inline root/request wiring stays in `registerWebinyApi.ts`, which already matched it line for line, so nothing of theirs was dropped. Also applied next's decorator ordering to `createWebinyStreamApiHandler`, which git could not conflict on because that file is new on this branch. Without it, API-key authentication would have been broken on streaming routes only.
…r response writing Review feedback on two files that had grown into grab-bags. `registerWebinyApi.ts` held two independent lifecycle steps plus their shared types. Split into `composition/registerWebinyApiRoot.ts`, `composition/registerWebinyApiRequest.ts`, `composition/types.ts`, and a barrel. Behavior unchanged; both composition roots import from the barrel. `createServerHandler` had the whole response-writing decision tree inline — a stream branch with its own back-pressure loop, four buffered-body branches, and an error path that has to know whether headers already went out. Extracted to `response/`: `writeHttpResponse` (dispatch), `writeStreamBody`, `writeBufferedBody`, `writeErrorResponse`. The request handler is now three lines and each piece is separately readable. Adds `ai-context/code-style/one-public-function-per-file.md` so this doesn't regress, with the narrow exception for a module of small pure helpers named for one concept — otherwise `extractRequestAuth.ts` would be a violation for no gain.
…a library
`createServerHandler` cast the awaited handler result, which required wrapping
the expression in parens. `createHandler` already returns `Promise<any>`, so the
cast was never needed — a type annotation does the job and reads left to right.
Adds `ai-context/code-style/prefer-type-annotation-over-cast.md`.
Expands the `library: { type: "module" }` comment in createRsbuildConfig with the
failure it prevents and how to verify it, since the reason isn't recoverable from
the code: removing that line silently drops `streamHandler` from the bundle and
still produces a green build.
Press space on an empty command palette query to enter AI mode and ask about the project's content in plain language. The assistant discovers content models and fields at call time, so it never needs model IDs or field names supplied. Backend: - `@webiny/api-ai-chat` adds `POST /ai/chat`. The agent loop runs server-side under the signed-in user's identity, so the browser holds no provider key and every tool call is checked against that user's own permissions. - Writes are gated. A tool that does not declare `readOnlyHint` is never run on the model's word alone: the loop pauses and returns the pending call for a human to approve. With no `WEBINY_API_AI_CHAT_APPROVAL_SECRET` configured, mutating tools are withheld entirely rather than gated on an approval that cannot be verified. - CMS tools (`api-headless-cms`): listContentModels, describeContentModel, queryEntries. Filters and sorts are accepted flat and split into the CMS's entry-meta vs `values` levels, since a flat field list is what describeContentModel hands the model. - Folder access tools (`api-aco`): listTeams, listFolders, and setFolderPermissions — the latter annotated destructive because it replaces rather than merges. - `IAiSdkTool` gains optional `title` and `annotations`; annotations drive the approval gate. Admin: - AI is a palette mode rather than a command detail view, so the input row stays in the same slot across modes. - Answers render through the existing `compileMarkdown`, with the tool chain shown beneath and a confirm block for proposed changes. Adds two code-style rules: routes delegate to use cases, and inject real dependencies rather than a container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Conflict was in packages/cli-core/files/references.json, which is generated by `yarn webiny sync-dependencies`. Resolved by taking next's version and regenerating rather than merging the JSON by hand; verified that every entry from next survived and that this branch's additions (ai in api-ai-chat, zod in api-aco) are present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🚓 Slop Cop ✅ Nothing worth flagging. The diff looks consistent with the PR's stated intent and the code-style rules. Large but coherent PR; footprint matches the stated (deliberately broad) intent, no secrets/artifacts/debug leftovers found, and no clear rule violations in the added code. Automated, non-blocking heads-up from an LLM. It can be wrong — use your judgment. Regenerates on every push. |
`@webiny/api-ai-chat` mixed the feature with the route that exposed it, so using the assistant anywhere else meant dragging in `event-handler-core`. Split in two, following how `ai-powerups` is laid out: - `@webiny/ai-chat` — the feature. `AiChatUseCase`, its config, the approval gating and the system prompt, under `src/api/`. Depends on `api-core`, `feature` and `ai`, and nothing about transport. Registering `AiChatFeature` puts the use case in the container; how it is reached is somebody else's problem. - `@webiny/ai-chat-http` — the adapter. Owns the `HttpRoute` for `POST /ai/chat` and registers the feature alongside it, so a host wires up one thing. This is the only package that knows about `event-handler-core`. A CLI or background task now registers `AiChatFeature` directly and skips the HTTP package entirely. Also corrects the public surface: `src/api/index.ts` exports the use case ABSTRACTION, not the implementation, matching how the rest of the codebase does it (see contentModel/ListModels). The old index exported the implementation under the same name, which shadowed the abstraction and would have broken any consumer trying to depend on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
/e2e |
|
Cypress E2E tests have been initiated (for more information, click here). ✨
|
Member
Author
|
/e2e |
|
Cypress E2E tests have been initiated (for more information, click here). ✨
|
`next` replaced `createHandler` with the DI-native `HandlerApp` (#5532) and renamed the `request` config key to `child` (#5582). Both land directly on the seam this branch is built on. Two conflicts, resolved by adopting the new contract: `createServerHandler` — kept this branch's extracted `writeHttpResponse` / `writeErrorResponse` split, moved onto `app.handle(req)` and `child`. Their inline body-writing branches are what the extraction replaced, so the resolution keeps the split rather than reinstating them; behavior is identical plus the streaming path. `createWebinyApiHandler` — `request:` becomes `child:`, still delegating to `registerWebinyApiRequest`. Their inline request-stack body already matches `composition/registerWebinyApiRequest.ts` line for line. Two more files had to change with no conflict to warn about, because both are new on this branch: - `createStreamLambdaHandler` called the now-deleted `createHandler`. Migrated to `HandlerApp.init` + `app.handle`, and its option renamed `request` → `child`. - `createWebinyStreamApiHandler` passed `request:`, silently ignored under the new contract — the per-request stack would never have been registered on streaming routes. Renamed to `child:`. Same class of trap as the decorator-ordering fix in the previous merge: the buffered path conflicts loudly, the streaming twin fails quietly.
…rver Review feedback. `registerWebinyApiRequest` becomes `registerWebinyApiChild` (file included), so the name matches the `child` lifecycle step of `HandlerConfig` it implements rather than the older `request` wording. Drops `ApiStreamRequestError` from `app/exports/admin.ts`. It was exported alongside the two symbols the file manager actually imports, with no consumer — that file is the public surface, so it shouldn't carry it until something needs it. The class stays exported from the feature module for internal use. Extracts `HttpServer` from `createServerHandler`: the class owns the Node server and the per-request bridge into the handler app, leaving `createServerHandler` as a composition root that wires the app and hands the server back.
`HttpServer` was the only consumer of `response/` — the writers aren't exported from the package index and the tests reach them through `createServerHandler` — so a separate top-level folder bought nothing. They now sit beside the class that uses them, with relative imports, under one `server/` barrel.
…lass `HttpServer` held no state beyond the server it created and exposed a single `getServer()`, so the class earned nothing. `createNodeHttpServer(app)` returns the `http.Server` directly and the accessor disappears. The per-request bridge is a private function in the same file. It also removes a name collision: `api-websockets-server` aliases `node:http`'s own `Server` as `HttpServer` in several files. Drops `server/index.ts` — nothing imported the barrel. `createServerHandler` imports the factory directly and the writers import each other relatively.
Review question on the `"GET" | "POST"` union: nothing ever passed GET. The only caller sent an explicit `method: "POST"`, which the client already defaulted to, so the option and its GET branch were speculative. Removed both — the client always POSTs and `hasBody` is just "is there a body". POST is also the right default to hardcode: a streaming route is an action, and keeping parameters in a body avoids a cacheable URL (CloudFront caches GET/HEAD). A future read-only stream can reintroduce the option. Documents `signal`, which stays because it is load-bearing: a streaming response lives as long as the producer, so the FM dialog aborts on close, on unmount, and before restarting. Without it the request stays open and the read loop keeps setting state on an unmounted component. Also replaces an inline conditional spread in the header build with `if` + `Object.assign`, per ai-context/code-style/no-inline-conditional-spreads.md — my own violation, spotted while in the file.
Follows the mechanism from #5537: the WCP license is now refreshed PRE-register (loadWcpLicense in registerApiRequestStack, before registerExtensions), and FeatureFlags.get() returns the effective flags — userFlag && license — so a register-time check is finally valid. That was not true when this route was written, which is why the gate started out at request time. `AiImageEnrichmentFeature` now reads `isEnabled("aiPowerups.fileManager.imageEnrichment")` and registers nothing when it is off, the same shape `AssetDeliveryFeature` uses for private files. Adds the missing `LICENSE_CHECKS` entry mapping that flag to `canUseAiImageEnrichment()` — without it the flag fell through to "any license grants it". Consequences: - The route's request-time `WcpContext.canUseAiImageEnrichment()` check and its 403 are gone. An unlicensed caller now gets a router 404, because the route genuinely does not exist. - `AiImageEnrichmentAfterCreateHandler` had already been migrated to FeatureFlags upstream, but its runtime check is now redundant — the handler is not registered at all when the flag is off — so it and the FeatureFlags dependency are removed. - A license change still takes effect on the next request: the child container re-registers per request. Swaps the 403 test for two tests on the registration gate itself.
Replaces `RequestContainer` + `container.resolve(...)` with declared constructor dependencies: `[PrepareImageEnrichmentUseCase, ApplyImageEnrichmentUseCase, Ai]`. The lazy resolution was a workaround copied from `AssetDeliveryRoute`, whose comment claims the file-manager use cases reach CMS tokens registered only during a CMS request. That is no longer true: `registerApiRequestStack` registers `HeadlessCmsFeature` unconditionally (line 100) before extensions (line 140), and route construction happens later still, when the terminal handler resolves `HttpRouter`. So everything the route needs exists by then and the workaround bought nothing here. Adds `ai-context/code-style/no-container-as-service-locator.md`. It notes the one legitimate use — `container.resolve` in a `createFeature` `resolve()` hook — and records that the remaining lazy routes are working around the documented eager route construction TODO on `HttpRouterImplClass`, not setting a precedent. The feature test now asserts the gate by spying on `register` instead of resolving: with injected dependencies, resolving would construct the route and pull `GetFileUseCase` and its whole graph, which is the request stack's business, not a flag test's.
The route was 158 lines, most of it not about enrichment. Now 96. Moved to `@webiny/event-handler-core` (any streaming route needs these): - `sseResponse(source)` — status, the SSE headers, and the HttpStreamBody wrap. The headers are the real payload here: `no-transform` and `x-accel-buffering` are easy to omit and omitting either yields a response that looks right and never arrives incrementally. That cost a production debugging session to find, so it belongs in one place rather than in every route. - `jsonResponse(statusCode, body)` — drops the repeated content-type literal. Moved next to the enrichment domain: - `buildEnrichmentAiRequest(prepared)` — the model/output/connection/messages payload was 21 byte-identical lines in the task and the route; only generateText vs streamText differed. One builder, typed `Ai.GenerateTextParams`, which streamText accepts too. - `readEnrichmentPartial(partial)` — normalises one partialOutputStream value, including the holes a partial array can contain. - `imageEnrichmentErrorStatusCode(error)` — error-to-status mapping, now beside the errors so a second entry point can't invent a different one. This also answers where `Output.object(...)` should live: not on `IPreparedImageEnrichment`, because the AI SDK's `Output` type cannot be named in emitted declarations (TS4023) and an exported interface would have to name it. It sits inside the builder's body instead, which is a single place and compiles. Adds tests for the two core helpers.
Clean merge, no conflicts. One follow-up needed: #5604 replaced `loadWcpLicense()` with the `WcpLicenseLoader` static class, so the comment in `AiImageEnrichmentFeature` explaining why a register-time license check is valid now names `WcpLicenseLoader.load()`. Nothing on this branch imported the old function directly — `registerApiRequestStack` owns that call. Also lands #5607, which gates api-aco folder-level permissions on FeatureFlags instead of WcpContext — the same migration this branch did for image enrichment.
Applies ai-context/code-style/no-nested-call-arguments.md, which this branch added and then broke in four places while extracting the shared helpers: - `generateText(buildEnrichmentAiRequest(prepared))` (task) - `streamText(buildEnrichmentAiRequest(prepared))` (route) - `jsonResponse(imageEnrichmentErrorStatusCode(error), ...)` (route) - `sseResponse(this.enrich(...))` (route) Each step now has a named const.
Integrates #5611, which gave routes an Express-style response builder. That is the response API now, so streaming had to join it rather than sit beside it. New: `IHttpResponseBuilder.sse(source)` — sets the SSE headers and wraps the source in an `HttpStreamBody`. The two headers that silently matter (`no-transform`, `x-accel-buffering`) now live in the builder, where any route writing a stream will find them, instead of being copied per route. That supersedes the two helpers this branch had just added: `jsonResponse` is `response.json()`, and `sseResponse` is `response.sse()`. Both files deleted, and the enrichment route now returns the builder. Three conflicts, all "their new feature + our streaming": - `httpResponseToApiGatewayResult` — theirs went sync and added cookies, ours was async to drain a stream. Kept async, kept both behaviours. - `createServerHandler` — theirs reinstated the inline writer to add cookies. Kept this branch's extraction and folded the cookie handling into `writeHttpResponse` instead. - the http barrel — export lists. Also closed a gap git could not flag: `FunctionUrlStreamRouterHandler` never passed `response.cookies` into the streaming prelude, so a streaming route setting a cookie would have lost it silently. The prelude type already had the field. Route tests now drive the route through `invokeHttpRoute`, the helper #5611 added, rather than calling `handle()` by hand — which would leave the builder undefined. Error assertions parse the body, since `json()` serializes eagerly now.
Answering where the per-use-case / shared line sits in streamEvents.ts. The event types stay: `tags`, `description`, `fileId`, `model` and the start/partial/done/error names are this feature's stream protocol. A shared event union would be either too loose to type-check or wrong for the next streaming route, which will want a different set. `toSseFrame` was the misplaced one. It only JSON-stringifies into a `data:` record — the SSE wire format, not enrichment — and it was needlessly typed to the enrichment union. Now in `@webiny/event-handler-core` beside `sse()` and taking `unknown`, which also makes it symmetric with `readServerSentEvents` in `@webiny/app`, the generic parser on the other end. Tests include a round-trip against the format that parser expects, since a mismatch there means events silently never parse.
…rigin timeout Review of the whole file turned up two things. The comment on the streaming origin request policy claimed the cache-policy swap is what fixed Lambda rejecting every signed CloudFront request with `AccessDeniedException`. It wasn't — that was a missing `lambda:InvokeFunction` permission. The swap is still right (legacy forwarded values are deprecated, and the policy has to carry the two CORS preflight headers), but the stated reason was the first wrong guess and would have misled the next reader. Also records why `AllViewer` must not be used here: it forwards `Host`, which breaks OAC signing. Sets `originReadTimeout: 60` on the streaming origin. CloudFront's default is 30s between response packets, which is shorter than a slow model's first token — it would abandon the stream while the Lambda (300s) kept working, and the client would see a truncated SSE stream with nothing wrong in the logs. 60s is the ceiling without an AWS quota increase; gaps beyond that need SSE heartbeat comments, noted inline. Also removes AiImageEnrichmentFeature.test.ts per review, and notes that `/stream/*` is deliberately `https-only` while other behaviors are `allow-all`.
It reads as a cap on total stream duration, which is what it was misread as in review. It is not: CloudFront applies it to time-to-first-byte and to the gap between two packets, so a stream that keeps emitting runs as long as the Lambda lives. Only silence longer than the timeout kills it.
…rompt `cachePolicyId: "4135ea2d-..."` was an unexplained constant. Verified against the API: it is AWS's `Managed-CachingDisabled`. Now a named constant, with a note on why it is hardcoded rather than resolved by name the way `createCloudFrontDefaultCacheBehaviorPolicies` does in this same package — the lookup needs `cloudfront:ListCachePolicies` on the deploy role, which is not worth requiring of every project for a value AWS documents as a global constant. Also sharpens the "do not use a managed origin request policy" warning. The real trap is not `Managed-AllViewer` but `Managed-AllViewerExceptHostHeader`, which this package already uses for the blue/green router: it fixes `Host` but still forwards `Authorization`, which OAC signing owns on this origin. WCP_STREAM_HANDLER_PROMPT.md is a temporary handoff for the WCP-side fix that will let us delete the streamHandler string-append; both go away together.
`execute` returned the bare DOM `Response` while its parameter was already `ApiStreamClient.Request`, so half the contract was named through the abstraction and half wasn't. Adds `ApiStreamClient.Response` and uses it at all three implementations — the fetch client and both decorators, not just the one flagged in review. Aliased as `globalThis.Response`: a plain `export type Response = Response` inside the namespace resolves to itself and is a circular reference. Also drops WCP_STREAM_HANDLER_PROMPT.md.
…t settled The comment explained why the append looks the way it does but read as if the approach were final. Notes that it is a known-fragile stopgap and that the agreed replacement is one line on the WCP side, so the next reader does not re-derive it.
The file had pure helpers both ways: `toFetchHeaders` and `joinUrl` as module functions, `toError` as a private method. None of the three read instance state, so the method was the odd one out. `toError` becomes `toRequestError` alongside the others, leaving the class with only the constructor and `execute`. Adds the general rule to ai-context/code-style, since the question will come up again: a private method that never reads `this` should be a module-level function.
…eway translator Keeps one translator per transport — splitting by buffered vs streaming would not work, since whether a body streams is a runtime property of the response, not a compile-time property of the route, so both halves would still have to return an ApiGatewayResult and the caller would have to dispatch on something it cannot see. What did belong in its own file is the streaming-specific part: collecting a stream into one value and deciding text vs bytes from the content type. `drainStreamBody` now owns that, along with the content-type regex and the case-insensitive header lookup that existed only to serve it. Also replaces the ternary-built result object and its nested body ternary with a named `encodeBody`, matching how the house style builds objects elsewhere.
Five useState calls, a ref, and the stream event branching all lived in the component. Now a MobX presenter following the pattern of the other presenters in this package: private state, commands, and a `vm` getter, with the component an `observer` holding no state of its own. The AbortController is explicitly annotated out of the observable map — nothing renders from it, and MobX needs the second type parameter before a private field may appear there at all. Event handling is now three named private actions rather than an if/else chain against setters, which also keeps the mutations after `await` inside actions.
Comes out of the ReenrichWithAi review: five useState calls and stream branching sat in a component when this codebase already has a presenter pattern for exactly that. Records where the line is (one visual useState is fine), the file shape to mirror, and the two MobX details that cost time — mutations after an await escape their action, and a private field needs the second type parameter before it can be annotated out of the observable map.
…h DI Team feedback on the new code-style rule: use `createReactiveComponent` rather than importing `observer` from mobx-react-lite directly, and get a presenter from `useFeature` rather than hand-instantiating it with useMemo. The presenter is now a registered implementation of a `ReenrichWithAiPresenter` abstraction with the gateway as a declared dependency, handed out by the feature's `resolve`, following `AiPowerUpsSettings`. The view model and presenter interfaces moved to abstractions.ts. `reenrichFile` is no longer exposed by the feature — the presenter is its only consumer. Registering the presenter as a singleton means it outlives the component, so `dispose()` now resets state instead of only aborting: a left-over `open` would have popped the dialog straight back up on the next mount. Rule doc updated to match, including that trap.
…uction
Every request to the deployed API failed with "No registration found for
FileModel", including OPTIONS preflights to /graphql, so admin could not start.
Cause: HttpRouterImplClass injected [HttpRoute, { multiple: true }], which
constructs every registered route while the router is being constructed.
RequestContextInitializerDecorator decorates HttpRouter, so the decoratee — and
therefore all routes — is built before route() runs and before the initializers
register FileModel. AiImageEnrichmentStreamRoute reaches FileModel through
GetFileUseCase, so its construction threw, and because construction ignores which
route matches, it took down every request on every path.
The router now takes the request container and resolves routes inside route(),
which runs after the initializers. This is the systemic fix the TODO on that class
asked for, and it is what makes declared route dependencies safe.
I introduced this in 261fea7 by replacing that route's lazy resolution with
declared dependencies. The commit message claimed the workaround "bought nothing
here" — wrong: it accounted for HeadlessCmsFeature being registered early but not
for FileModel, which a RequestContextInitializer registers per request.
AssetDeliveryRoute and WebsiteBuilderRedirectsRoute can now drop their lazy
resolution too; left alone here to keep this fix small.
Three conflicts, all in generated or list-style files: - `yarn.lock` and `packages/cli-core/files/references.json` — took #5533's version and regenerated via `yarn` + `yarn webiny sync-dependencies` rather than merging by hand. Verified nothing from either side was dropped: every name present on both branches survives, this branch's `ai-chat` and `ai-chat-http` entries are intact, and the one name that disappeared (`node-notifier`) was removed on #5533, not here. - `ai-context/code-style/README.md` — both branches added rules; kept the union. Dropped `inject-dependencies-not-the-container.md` in favour of #5533's `no-container-as-service-locator.md`. They state the same rule, and #5533's is the more accurate one: it records that `HttpRouter` now resolves routes inside `route()`, so the constraint that once justified injecting `RequestContainer` no longer exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three separate CI failures, none of the same cause: 1. Duplicate dependency versions. `ai-chat` and `ai-chat-http` pinned `ai@^7.0.58` and `vitest@^4.1.10` while the rest of the repo had moved to `^7.0.73` and `^4.1.11`, so `verify-dependencies` refused the tree. Aligned both to the versions the repo already uses. 2. `@webiny/ai-chat/api/index.js` did not resolve when bundling. With only `src/api/**` present, emit collapsed the common directory and produced `dist/index.js` instead of `dist/api/index.js`. TypeScript resolved the import through tsconfig `paths`, so `yarn check` passed and hid it — only the rspack bundle, which uses real node resolution, caught it. Added a root `src/index.ts` that anchors the emit root, and gave it a real job: the approval types that cross the wire, so the admin client can stop redeclaring them. 3. `CompressionDecorator.test.ts` (new on next) builds its own container and assumed `HttpRouter` takes routes as a constructor dependency. #5533 changed it to resolve routes inside `route()` from `RequestContainer`, so the test's container was missing that registration. Neither branch failed alone; only the combination did. Registered `RequestContainer` in the test container. Also merges current next. Verified the way CI does rather than by typecheck alone: `yarn build`, `yarn webiny build core`, `yarn webiny build api`, plus adio, lint and the event-handler-core and ai tool suites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`validate-webiny-package` failed because merging next brought new and changed `export:` files that the generated `webiny` package did not reflect — the check's own message predicts exactly this cause. Ran `yarn webiny-scripts generate-webiny-package`, then `yarn format`, since the generator emits unformatted output and `format:check` runs in the same job. The diff is entirely churn from next's export changes; nothing here relates to ai-chat, which declares no `export:` entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Space on an empty query is easy to miss, so make the key that already opens the palette get there too. mod+k now cycles closed -> commands -> AI -> closed: pressing it from AI mode still closes the palette, which is what mod+k does everywhere else. From a command's detail view, mod+k backs out to the command list rather than switching to AI mode — the detail view renders in place of the list, so a mode switch there would leave the user in a mode they cannot see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A multi-tool question takes tens of seconds, and writes made that the normal case. Answers now arrive token by token, tools appear as they run, and an approval request shows the moment it happens rather than after everything settles. Feature (`@webiny/ai-chat`): - `AiChatUseCase.stream()` yields `AiChatEvent`s — text, tool-call, tool-result, approval, done, error. A small purpose-built set rather than the AI SDK's UI-message protocol: the client is a command palette, the SDK protocol does not model our approval pause, and this keeps AI SDK code out of the admin bundle. - Events are transport-free. `execute()` and `stream()` share one `prepare()` so the identity check, write gating and model request cannot drift. - An auth failure in `stream()` arrives as an `error` EVENT, not a thrown error: by the time a transport iterates, it has committed to a 200 and there is no status code left to change. Transport (`@webiny/ai-chat-http`): - `POST /ai/chat/stream` frames those events as SSE via `HttpStreamBody`. A separate route from `/ai/chat` because the response contracts genuinely differ, and the buffered one stays for callers that cannot read a stream. - Extracted `parseChatBody` and `jsonResponse`, now shared by both routes. Admin: - The gateway streams through `ApiStreamClient` instead of raw `fetch`, so auth and tenant headers come from the platform's decorators. That matters: the streaming path authenticates with `x-webiny-authorization`, because on AWS SigV4 occupies `Authorization` — hand-rolling the header would have made every streamed request anonymous. - The stream is aborted when the palette unmounts; a stream stays open as long as the model runs. - `AiTurn` shows tool chips live, with the running one spinning, and drops the skeleton as soon as the first token lands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The assistant read `WEBINY_API_AI_CHAT_MODEL` and relied on the provider SDK's own environment variable for the key, which is inconsistent with how every other AI feature in the project is configured: AI Power-Ups already manages providers in the admin UI, per tenant, with keys encrypted at rest. - `AiChatProvider` abstraction in `@webiny/ai-chat` answers "which model, whose key". `EnvAiChatProvider` is the default so a project without AI Power-Ups keeps working from one environment variable, and a bare checkout stays zero-config. - `PowerUpsAiChatProvider` in `@webiny/ai-powerups` overrides it, reading the first configured preset and decrypting its key — the same source `CmsGenerateEntryContent` and `CmsCompareEntryRevisions` read. Registered after the default so it wins. - `AiChatConfig` no longer carries a model; it is limits only. An absent key stays meaningful: the provider's SDK factory then falls back to its own environment variable, so a provider row that exists without a key yet does not send an empty string the provider would reject. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…y stream The route worked; it was unreachable through a transport able to stream. On AWS, CloudFront has a dedicated behavior for `/stream/*` that targets the graphql Lambda's Function URL, with compression disabled because "CloudFront compression buffers small chunks, which defeats incremental delivery even though the origin streams". Everything else goes to API Gateway, which buffers the entire Lambda response no matter how the route produced it. `/ai/chat/stream` did not match that pattern, so it took the API Gateway path and arrived all at once — no error anywhere, just no streaming. Moved to `/stream/ai/chat`, matching the prefix `ApiStreamClient` documents. Nothing else was needed: the Function URL handler resolves the same `HttpRouter` as every other transport, and the origin is the same graphql Lambda, so the route was already registered there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every `/stream/*` request carrying a body was rejected on AWS with `InvalidSignatureException`, while bodyless ones succeeded. CloudFront's Origin Access Control signs the request with SigV4 but does not hash the body — it folds whatever `x-amz-content-sha256` says into the signature, and the Lambda Function URL's IAM authorizer then recomputes the hash from the body it received. With no header the two disagree. A bodyless POST worked because an empty payload hashes predictably, which is why image enrichment (its only parameter is a path segment) has always worked and nothing caught this: no streaming route had ever sent a body. `FetchApiStreamClient` now sets the header whenever there is one. Fixed here rather than in the caller so the next streaming route with a body does not have to rediscover it. Harmless on transports that do not sign. Confirmed against a deployed distribution: same request 403s without the header and returns `text/event-stream` with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…reflight Adding `x-amz-content-sha256` to streaming requests made the browser send a header the preflight did not permit, so it blocked the request before it left — "Failed to fetch", with no status code and nothing in the origin's logs. `ALLOWED_HEADERS` already carried the same note for `x-webiny-authorization`; this is the second header the streaming path needs for the same reason. Extended the existing preflight test to cover it, since the symptom points at the network rather than at a missing allowlist entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ended up as one commit rather than two — the merge resolution and the follow-up work were staged together. Contents: MERGE #5533's streaming work landed on next as #5622, so the apiStreamClient and AiImageEnrichment files exist on both sides. Took next's reviewed version for everything this branch had not touched, and re-applied its own additions on top: `x-amz-content-sha256` in `FetchApiStreamClient` and the CORS allowlist, which next does not have — no streaming route had sent a request body before, so the signing and preflight failures were still unfixed there. next also added `no-multiline-ternaries`, which this branch violated in eight places including one nested ternary; all rewritten as `if` statements or named values. `CompressionDecorator.test.ts` registered `RequestContainer` twice because next fixed it the same way this branch did; kept next's. FEATURE FLAG `aiChat` gates both sides. The routes return 404 rather than 403 when it is off, so a disabled project reveals nothing about what it could do. The palette hides AI mode, the space and second-mod+k shortcuts, the `Ask AI` command row, the footer hint and the no-results CTA. Gating one side only would either offer a mode the API refuses, or leave the endpoint open with no UI. PROVIDER RESOLUTION Registration order was wrong: a single resolve takes the LAST registration, and `AiChatHttpFeature` ran after extensions, so its environment-variable default won and the configured AI Power-Ups provider was silently ignored. Registered before extensions now. Route construction does not depend on the order — `HttpRouter` resolves routes inside `route()`, and `resolveAll(AiSdkTool)` is order-independent. `PowerUpsAiChatProvider` now resolves the same way `PrepareImageEnrichmentUseCase` does: first preset, decrypt its key. It errors when a provider exists without a model or key rather than falling back to the environment — that fallback is what made the previous behaviour hard to reason about, because settings said one thing and the request used another. Also removed two `as never` casts around `generateText`/`streamText`. They were an artifact of building the request as `Record<string, unknown>`; typing it as `Ai.GenerateTextParams` typechecks with no errors, so nothing was hidden. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…icense `aiChat: true` in `webiny.config.tsx` had no effect on an unlicensed install. The license decorator returned false for any flag not in `LICENSE_CHECKS` whenever no license was present, so a project could not turn on a flag it had declared itself — which the license has no business preventing. Now, for non-license-governed flags with no license present, resolution falls through to the base class: an explicitly configured `true` is honoured, and an unset flag stays off. Deliberately narrow. Eight flags are consulted in code but absent from LICENSE_CHECKS (`aiPowerups`, its six sub-flags, `remoteComponents`) and none are declared in a project's config, so they rely on "license present + unset → true" to ship enabled. Making the config authoritative outright would have switched all of AI Power-Ups off. That path is untouched. Adds the decorator's first tests — its behaviour was documented only in a comment, which is how the rules drifted from the base class in the first place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Press space on an empty command palette query to enter AI mode and ask about the project's content in plain language.
The assistant discovers what exists at call time — it is never given model IDs or field names, because those are project-specific:
How a request travels
AiChatGatewayposts to/ai/chatwith the admin session token and tenant — no API key, no provider key, no model name leaves the browser.AiChatUseCaseruns the agent loop over every registeredAiSdkTool, under that user's identity.Because tools call real Webiny use cases, permissions apply exactly as they do in the UI — a caller without CMS access gets
Not allowed to access content modelsrather than data.Writes are gated
A tool that does not declare
readOnlyHintis never executed on the model's word alone. The loop pauses, the pending call is returned, and the palette shows the exact arguments for approval. Resuming replays the paused messages plus a signed approval, so an approval issued for one call cannot be replayed against another.With no
WEBINY_API_AI_CHAT_APPROVAL_SECRETset, mutating tools are withheld from the toolset entirely rather than gated on an approval that cannot be verified. Default for an unannotated tool is requires approval, so an extension author who forgets to annotate gets a prompt rather than silent write access.Tools
listContentModelsapi-headless-cmsdescribeContentModelapi-headless-cmsqueryEntriesapi-headless-cmslistTeamsapi-acolistFoldersapi-acosetFolderPermissionsapi-acosetFolderPermissionsreplaces rather than merges, so omitting an existing rule revokes it — hence the destructive annotation and the explicit note in its schema.Filters and sorts are accepted flat and split into the CMS's two levels internally (entry meta at the top, model fields under
values).describeContentModelreturns a flat field list, so a flat filter is what a model will write; without the split every filter it produced failed.Configuration
Not in this PR
Ai.streamTexthas no consumers and Lambda response streaming is unmerged. A multi-tool question returns once, after a few seconds. Upgrading is a change to how the same payload arrives, not a redesign.ai-powerupsalready manages. The intended fix is anAiChatProviderabstraction with an env-var default, overridden whenai-powerupsis installed.values-splitting fixes it surfaced all remain.Testing
yarn test packages/api-headless-cms— 16 tests covering the where/sort splitting, the system-model filter, and the DI wiring of each tool.Verified by hand against a local server: the read-only chain end to end, the permission boundary, and the palette rendering answers as markdown.
Not verified: the approval round-trip against a live model. The riskiest part is whether replaying
responseMessagesplus atool-approval-responseresumes the loop and the HMAC verifies. A failure there surfaces as a 500 with a readable provider message, not a silent wrong write.Also carrying
as nevercasts wheregenerateTextgenerics meet a runtime-assembledToolSet; those hide real type errors and deserve a follow-up.🤖 Generated with Claude Code