feat(event-handler): HTTP response streaming on Lambda and self-hosted - #5533
Closed
adrians5j wants to merge 47 commits into
Closed
feat(event-handler): HTTP response streaming on Lambda and self-hosted#5533adrians5j wants to merge 47 commits into
adrians5j wants to merge 47 commits into
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>
adrians5j
marked this pull request as draft
August 4, 2026 14:42
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.
|
🚓 Slop Cop Large, coherent PR whose diff matches its stated scope (HTTP response streaming feature); no integrity red flags found, and only a minor style nit around console.error in a library file. 📏 Code-style rule checks 🟡 Low — console.error in backend streaming code packages/event-handler-aws/src/createStreamLambdaHandler.ts adds a 🟡 Low — console.log removed but worth noting no logger used elsewhere The old console.log("error", error.message) in AiImageEnrichmentTask.ts was removed (good), but no equivalent DI Logger usage was introduced to replace debug visibility — not a violation, just noting the cleanup is consistent with no-console-in-backend.md. Automated, non-blocking heads-up from an LLM. It can be wrong — use your judgment. Regenerates on every push. |
…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.
`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.
adrians5j
added a commit
that referenced
this pull request
Aug 29, 2026
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>
… width Option A. The actions row moves out of the left panel and above both of them, so it is bounded by the drawer rather than by half of it. "Re-enrich with AI" was being clipped to "Re-e", and any further action would have clipped the one before. A Separator under the row is load-bearing: the panels' vertical divider starts where the panels start, so without a horizontal rule to meet, its top end hangs in mid-drawer. Same toolbar-then-Separator structure as ListViewHeader.
adrians5j
added a commit
that referenced
this pull request
Aug 31, 2026
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>
…open drawer Three things. Enrichment now REPLACES tags as well as the description. Tags were merged into the file's existing ones while the description was overwritten — an odd split that made re-running accumulate tags forever, so a wrong tag from an earlier run could never be dropped by re-running. `existingTags` is now unused and gone from the prepared result and the apply params. The open drawer's form now takes the persisted values. The websocket handler patches the files LIST cache, which does nothing for an already-open drawer: its form is built once in loadFile. The fields kept showing pre-enrichment values, and because the form still held them, pressing Update wrote them straight back and undid the enrichment. Applied silently so the form does not become dirty. The re-enrich dialog gets bottom padding. DialogBody applies only horizontal padding; vertical space at the bottom normally comes from DialogFooter, and this dialog has no actions, so the description sat flush against the dialog edge.
The streaming route no longer writes. It streams the model's output and stops, so the dialog shows a proposal with Cancel and Save in the footer, and nothing touches the file until Save. Cancel discards, which previously was not possible — the write had already happened before the dialog appeared. Save puts the values into the file details form and submits it, so enrichment persists through exactly the path a manual edit takes: one write path, one set of permissions and validation. `setValueSilent` keeps the form from going dirty, and still feeds `getData()`, so the submit carries the values through. The upload-time task keeps applying automatically — there is nobody to ask at that point — so ApplyImageEnrichmentUseCase stays, just no longer called by the route. Also drops the dialog's bottom padding again: the footer now provides that spacing.
Pressing Save blocked the whole drawer behind a "Saving changes..." overlay while the write ran. Now the values go on screen, the dialog closes, and the request finishes in the background — the user carries on immediately. saveFile takes `showLoader`, defaulting to true so the drawer's own Update button is unchanged; only the background save skips the overlay. Rollback is the part that makes this honest. The form values are captured before applying, and restored if the save fails, with a warning notification since the dialog is gone by then. `persist()` treats a thrown save the same as a rejected one: updateFileUseCase.execute has no catch, so a network error propagates rather than returning false, and without that the failure would leave the UI claiming a save that never happened. Drops the now-dead "saving" status and vm.saving — the dialog closes instead.
Drops the background save and its rollback. Save now only drops the values into the file details form and closes the dialog; the file is written when the user presses Update in the drawer, the same as for a hand-typed edit. One save button in the drawer instead of two competing ones, and no optimistic state to unwind. applyEnrichment therefore uses setValue rather than setValueSilent, and leaves `this.file` alone: nothing is persisted at that point, so the form must go dirty and Update has to be what writes. `save()` becomes `accept()` — it no longer saves anything — and is synchronous, so the Notifications dependency and the `showLoader` option on saveFile are both gone again.
…oducing An em dash stood in for tags and description until content arrived, which reads as "this file has none" rather than "still coming". Skeleton bars now fill the gap while the stream runs, per field, so each one swaps to real content the moment its first partial lands. The dash stays for the case it actually describes: the stream finished and the field came back empty. Bar widths are uneven because equal-length bars read as a table rather than as text still arriving.
Chips visibly reshuffled mid-stream. The model does not emit the tag array as a growing prefix — it revises it, so successive partials can list the same tags in a different order, and assigning each value straight to state made the rendered chips jump around under the pointer. Tags are now held in the order first seen, so a tag only ever appears and never moves. The `done` value reconciles: anything the model dropped on the way is removed, so what gets saved is its actual answer rather than the union of every intermediate guess. Nothing here changes what the model returns or the order it returns it in. The schema already asks for tags before the description.
Regression from the previous commit. The model streams the tag array character by character, so "webiny" arrives as "web", "webi", "webiny". Keeping every tag ever seen therefore produced a chip per keystroke — web, webiny, inf, infographic — plus a blank one. Back to replacing the array each partial, with the trailing entry dropped while the stream is still running, since that entry is the half-written one. A chip now appears only once its text has settled, and the `done` value is used whole. readEnrichmentPartial also drops empty strings, not just non-strings: mid-stream the array can hold "" for an entry the model has opened but not written into, which is where the blank chip came from. Covered by a test. The reshuffling the previous commit set out to fix was almost certainly separate runs being compared, not reordering within one stream.
… resizing
The dialog grew and shrank as the model streamed, sliding the footer buttons out
from under the pointer.
Each value box now reserves its space — two rows of chips, four lines of text,
which is what this prompt ("up to 5 tags and one short sentence") produces. On the
value boxes rather than once on the dialog body: tags and description grow
independently, so a single body minimum would still let wrapping tags shove the
description down. Past the reservation the dialog grows, which beats clipping.
The description uses `4lh` — the line-height unit — so four lines stays four lines
if the type scale changes. Browsers without it ignore the minimum and behave as
before.
The dialog implied the work was done. Its button said "Save" while it saves nothing, and once it closed there was no sign the file was still unsaved — the drawer gives no unsaved-changes cue either. Three changes, covering before and after the click: - The button is "Apply", because that is what it does: fills the form in. - The ready message says both halves — "Applying fills in the form below. Press Update to save the file." - Applying queues a notification reminding the user to press Update. `notify` rather than `success`, since nothing has been saved and a green tick would claim otherwise.
… dirty state Update looked identical whether or not the drawer held unsaved changes, so nothing on screen distinguished "saved" from "about to be lost" — which is why an applied AI suggestion read as already saved. It is now disabled until the form is dirty, so the button itself carries that signal, for hand-typed edits as much as for enrichment. Reactivity comes free: the vm getter builds `form: this.form.vm`, which reads `isDirty` eagerly, so the observer tracks it. An existing test already pins isDirty=false after loadFile, so the button starts correctly disabled. Adds coverage that applyEnrichment goes through setValue rather than setValueSilent, since the binding depends on it. The mock form's `field()` returned undefined, so it now returns a usable field — loadFile also calls it to disable fields for a read-only user, a path no test had reached.
…state No other form in the admin disables its submit button this way, so doing it here alone makes the file details drawer the odd one out. Worth doing as a deliberate pass across forms rather than as a side effect of an AI-enrichment change. Keeps the two things from that commit that stand on their own: the test that applyEnrichment goes through setValue (the values are pending edits, not saved ones), and the mock form's `field()` returning a usable field instead of undefined — loadFile calls it to disable fields for a read-only user, a path no test reached.
It was an AI-enrichment-shaped method on a presenter that has nothing to do with
AI enrichment, so every future feature filling in fields would have added another
one. Removed from the class and the interface.
The enrichment presenter now uses the form's own generic API:
`vm.form.setData({ tags, description }, { dirty: true })`. `setData` merges — it
only touches the keys passed — so Name and Access Control keep whatever the user
typed, and `dirty: true` skips the baseline snapshot so Update still has something
to save. No new surface anywhere; the drawer stays unaware the feature exists.
A decorator would not have helped here: decorators implement the same abstraction,
so adding a method still means declaring it on IFileDetailsPresenter first, which
is the part worth avoiding.
Test mock fix that this exposed: the mock form's view model had inert getData and
setData stubs while the model kept its own data, so nothing reaching the form
through `vm.form` could be exercised. Both now read and write the same object.
This was referenced Aug 31, 2026
Member
Author
|
Closing — split into two reviewable PRs, no work dropped.
Together they are exactly the content of this branch — the split is at The branch Every comment raised on this PR was addressed before the split; the changes travelled into whichever of the two PRs owns that code. |
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.
Adds incremental HTTP response delivery to Webiny — on self-hosted Node and on AWS Lambda — plus a File Manager "Re-enrich with AI" action that exercises it end to end.
Verified working on both hosting types: self-hosted via
webiny-server watch, and on a real AWS deployment through CloudFront with chunks arriving incrementally.Core primitive
HttpStreamBody(@webiny/event-handler-core) is an explicit marker a route wraps its source in to opt into streaming. Additive —IHttpResponse.bodywas alreadyany, so no existing route changes. Transports that can stream write chunks as they are produced; transports that cannot callcollect().Duck-typing
Symbol.asyncIteratorwas rejected deliberately: a plain object body could satisfy it accidentally, andReadableStream's async-iterator support isn't in the DOM types.Self-hosted
createServerHandlerflushes headers, writes per chunk, honours back-pressure, and stops pulling when the client disconnects.Also fixes a latent bug the streaming path made reachable: the error handler called
writeHead(500)unconditionally, which throwsERR_HTTP_HEADERS_SENTonce headers are out and masks the real error.AWS
API Gateway buffers the entire Lambda response regardless of how it's produced, so it cannot stream at all. Streaming needs 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), fronted by a Function URL instead of API Gateway.New transport: Function URL event type, translator (cookies arrive as an array, not a header; no stage prefix to strip), a terminal handler that writes to the response stream, and
createStreamLambdaHandler.The existing API Gateway translator now drains a streaming body instead of failing, so a streaming route still works over that transport as a single buffered response.
Auth
Adds
x-webiny-authorization, read ahead ofAuthorizationby both the AWS and self-hosted extractors. Behind CloudFront with Origin Access Control, SigV4 occupiesAuthorization, so a viewer bearer token can't survive to the origin. A separate header keeps the Function URL private (AWS_IAM+ OAC) without depending on that interaction.Infra
ApiGraphqlStream(Lambda + Function URL, reusing the graphql IAM role, 300s timeout), a CloudFront OAC, and a/stream/*behavior ordered first withcompress: false— CloudFront compression buffers chunks and defeats incremental delivery.File Manager
The AI enrichment logic is extracted out of
AiImageEnrichmentTaskintoPrepare/Applyuse cases shared with the new SSE route, so the task and the route can't 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
ApiStreamClientand a genericreadServerSentEventsreader, with auth and tenant decorators mirroring the GraphQL client.GraphQLClientcouldn't carry this:execute(): Promise<TResult>is a buffered contract by type, and graphql-js 16 has no incremental delivery.Four bugs found by deploying
Each of these failed silently and would have shipped a non-streaming endpoint that looked fine:
streamHandleraway. Nothing imports an entry's exports, so the unused one went along with every module reachable only from it.handlersurvived by accident of being first. Fixed by declaring the entry a module library.handler. It renames the bundle to_handler.mjsand substitutes a downloaded wrapper, sohandler.streamHandlerdidn't exist in the deployed artifact. Re-exported unwrapped (thestreamifyResponsemarker must survive) via a namespace import (the self-hosted bundle has no such export, and a named re-export of a missing binding is a hard ESM error).lambda:InvokeFunctionUrlandlambda:InvokeFunction; with only the first, Lambda denied every signed CloudFront request and the function was never invoked.application/octet-streamwith all headers dropped. A direct streaming invoke returned 0 bytes. Every path now guarantees one write.Tests
79 new tests across
event-handler-core,event-handler-server,event-handler-aws,app, andai-powerups. The server test gates its producer on the client having read the first chunk, so a buffering transport deadlocks rather than passing.Reviewer notes
_onBeforeFirstWrite, which isn't public API. Tests can only assert we callwriteonce; if AWS changes that hook it regresses to the silent empty-200.api.webiny.com/clients/latest.mjs. If that file's shape changes, this breaks.library: { type: "module" }is in sharedbuild-toolsand affects every function bundle on both hosting types. Both were verified to build and import, but it has the widest blast radius here./stream/*route is a new public surface; authorization leans on the WCP gate plusUpdateFileUseCase. No route-level permission checks were added.x-amz-content-sha256. The enrich route sends none.🤖 Generated with Claude Code