refactor(core): extract runtime implementations behind extensions - #3212
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cd9e53507
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR refactors Veryfront core to keep provider/runtime-specific implementations behind explicit extension package boundaries. It extracts S3/GCS blob storage and Node ws WebSocket upgrades into dedicated extensions, adds lifecycle/ownership primitives to make registration and shutdown safe across generations, and refreshes the npm build pipeline plus generated API references and docs.
Changes:
- Introduces explicit extension contracts for blob storage (
BlobStorage) and Node WebSocket upgrades (NodeWebSocketServerProvider), with new first-party extensions for S3, GCS, andws. - Hardens runtime lifecycle: owned registrations in the project-scoped registry, abort-signal composition, WebSocket upgrade option validation, deferred WebSocket transport bridging, and Node listener startup ownership.
- Updates npm build metadata (engine gates, SBOM/supply-chain boundaries), docs, and generated API reference outputs.
Verification
- Not run in this review environment.
- PR description reports:
deno task verify:quick,deno task build:npm, runtime tests, script tests, audit, andgit diff --check.
Reviewed changes
Copilot reviewed 119 out of 122 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/workflow/blob/types.ts | Adds contract constant and richer BlobRef/options docs for provider-neutral blob storage. |
| src/workflow/blob/s3-storage.ts | Removes core S3 implementation (moved to extension). |
| src/workflow/blob/index.ts | Updates blob barrel exports, removes S3/GCS exports, adds blob-id helpers. |
| src/workflow/blob/gcs-storage.test.ts | Removes core GCS tests (moved to extension). |
| src/workflow/blob/blob-id.ts | Strengthens blob id validation to accept unknown and updates error text. |
| src/workflow/blob/blob-id.test.ts | Adds focused tests for blob id safety helpers. |
| src/server/service-server.ts | Routes Node server creation through startup-owned Node adapter and adds explicit WS provider option. |
| src/server/service-server.test.ts | Updates shutdown test to reflect hardened Node transport lifecycle behavior. |
| src/server/production-server.ts | Snapshots Node WS provider by bootstrap generation and passes it into serve options. |
| src/server/node-upgrade-lifecycle.ts | New lifecycle owner for Node upgrade listeners/sockets/WebSocket servers. |
| src/server/node-upgrade-lifecycle.test.ts | Tests for idempotent cleanup, failure aggregation, and retry behavior. |
| src/server/dev-server/server.ts | Captures/supplies the bootstrap-selected Node WS provider in dev server runtime. |
| src/server/bootstrap.ts | Captures an immutable Node WS provider generation during bootstrap. |
| src/server/bootstrap.test.ts | Tests that bootstrap snapshots an immutable provider generation. |
| src/release-assets/index.ts | Adds a release-assets API example to the public barrel docs. |
| src/release-assets/build-executor.ts | Switches to importing bundler extension via published package boundary. |
| src/registry/project-scoped-registry-manager.ts | Adds owned registrations with generation-safe disposal semantics. |
| src/registry/project-scoped-registry-manager.test.ts | Expands tests for owned registration lifecycle and transaction interactions. |
| src/react/jsx-runtime.d.ts | Adjusts JSX intrinsic element typing wiring for runtime compatibility. |
| src/platform/compat/http/websocket-upgrade-options.ts | New shared validation for portable WebSocket upgrade options/headers. |
| src/platform/adapters/runtime/shared/deferred-websocket.ts | New deferred WebSocket bridge during server upgrade handshake. |
| src/platform/adapters/runtime/node/types.ts | Re-exports Node WS types from the new websocket extension contract module; refines Node server close typing. |
| src/platform/adapters/base.ts | Hardens WebSocketUpgradeResponse detection against hostile objects and adds Node WS provider option. |
| src/extensions/websocket/node-websocket-server-provider.ts | New dependency-free Node WS provider contract + snapshot/capture helpers. |
| src/extensions/websocket/node-websocket-server-provider.test.ts | Tests contract snapshot/capture behavior and diagnostics. |
| src/extensions/websocket/index.ts | New websocket contract barrel export. |
| src/extensions/types.ts | Adds optional abort signal support to extension context. |
| src/extensions/recommendations.ts | Adds recommended package for the Node WS provider contract. |
| src/extensions/index.ts | Exports websocket contract helpers from the main extensions barrel. |
| src/extensions/builtin-schema-validator.ts | Moves zod adapter import to published extension package boundary. |
| src/extensions/builtin-extensions.ts | Switches builtin extension imports to published extension packages. |
| src/extensions/builtin-extensions.test.ts | Adds explicit assertion that Node WS extension is never auto-loaded; validates import boundaries. |
| src/extensions/abort-signal.ts | Adds composeAbortSignals helper (avoids AbortSignal.any). |
| src/extensions/abort-signal.test.ts | Tests abort-signal composition semantics and Node 18 compatibility. |
| src/errors/index.ts | Adds a public example for defineError usage. |
| src/agent/hosted/cloud-agent-provider-bootstrap.ts | Routes zod adapter loading through first-party extension module import helper. |
| scripts/test.deno.json | Adjusts script config lockfile location. |
| scripts/lint/test-typecheck-baseline.json | Removes deleted core GCS storage test from the typecheck baseline. |
| scripts/lint/extension-source-metadata.ts | Adds NodeWebSocketServerProviderName to known contract constants. |
| scripts/lint/audit-extension-contracts.test.ts | Extends lint test coverage to include websocket contract constants. |
| scripts/lint/audit-dependency-boundaries.test.ts | Adds Node WS boundary to sensitive extension dependency checks. |
| scripts/build/npm-package-metadata.ts | Marks AWS SDK + ws dependencies as extension-owned. |
| scripts/build/npm-package-metadata.test.ts | Adds npm lifecycle probe coverage for bundled MLflow exporter. |
| scripts/build/npm-extension-package-metadata.ts | Adds per-extension Node engine gating via manifest metadata. |
| scripts/build/npm-extension-package-metadata.test.ts | Tests Node engine gating and pinned dependency expectations (including S3 + Node>=20). |
| scripts/build/generate-sbom.ts | Adds SBOM sensitivity boundary for Node WS extension deps. |
| scripts/build/generate-sbom.test.ts | Tests SBOM sensitivity ratchet for Node WS boundary. |
| scripts/build/compile-binary.test.ts | Ensures compiled CLI does not implicitly embed Node WS extension. |
| scripts/build/build-npm-dnt.ts | Strengthens npm root import lifecycle probe to cover bundled MLflow lifecycle. |
| extensions/README.md | Updates extension catalog and contract guidance for blob + websocket boundaries. |
| extensions/ext-node-websocket-ws/src/package-boundary.test.ts | New extension boundary test for explicit activation and dependency ownership. |
| extensions/ext-node-websocket-ws/src/index.ts | New explicit ws-backed Node WS provider extension implementation. |
| extensions/ext-node-websocket-ws/src/index.test.ts | Tests extension setup/teardown behavior and provider surface. |
| extensions/ext-node-websocket-ws/README.md | New README for explicit Node WS extension composition. |
| extensions/ext-node-websocket-ws/deno.json | New extension manifest for Node WS extension (explicit activation, deps, tasks). |
| extensions/ext-blob-s3/src/package-boundary.test.ts | Boundary test ensuring extension imports only public Veryfront subpaths. |
| extensions/ext-blob-s3/src/multipart-upload.ts | New multipart uploader adapter for unknown-length Web streams. |
| extensions/ext-blob-s3/src/multipart-upload.test.ts | Tests multipart adapter cancellation/abort and protocol pathing. |
| extensions/ext-blob-s3/src/index.ts | New explicit S3 BlobStorage extension with lifecycle-safe setup/teardown. |
| extensions/ext-blob-s3/README.md | New README documenting S3 extension usage and migration from core. |
| extensions/ext-blob-s3/deno.json | New S3 extension manifest with Node>=20 engine requirement and AWS deps. |
| extensions/ext-blob-gcs/src/resumable-upload.ts | New resumable upload protocol for unknown-length streams to GCS. |
| extensions/ext-blob-gcs/src/resumable-upload.test.ts | Tests resumable protocol correctness, cancellation, and bounds. |
| extensions/ext-blob-gcs/src/package-boundary.test.ts | Boundary test ensuring extension imports only public Veryfront subpaths. |
| extensions/ext-blob-gcs/src/index.ts | New explicit GCS BlobStorage extension with lifecycle-safe setup/teardown. |
| extensions/ext-blob-gcs/README.md | New README documenting GCS extension usage and migration from core. |
| extensions/ext-blob-gcs/deno.json | New GCS extension manifest (explicit activation and network capabilities). |
| docs/guides/extensions.md | Documents explicitly enabling Node.js WebSocket upgrades via extension. |
| docs/architecture/20-support-matrix.md | Updates support matrix to include explicit Node WS provider + availability model. |
| docs/architecture/12-extension-system.md | Documents the Node WS transport boundary and its ownership model. |
| docs/api-reference/veryfront/workflow.md | Generated reference refresh including new veryfront/workflow/blob barrel. |
| docs/api-reference/veryfront/webhook.md | Generated nav order refresh. |
| docs/api-reference/veryfront/ui.md | Generated reference refresh (line/ordering updates). |
| docs/api-reference/veryfront/trigger.md | Generated nav order refresh. |
| docs/api-reference/veryfront/tool.md | Generated nav order refresh. |
| docs/api-reference/veryfront/testing.md | Generated reference refresh (timing source line updates). |
| docs/api-reference/veryfront/task.md | Generated nav order refresh. |
| docs/api-reference/veryfront/skill.md | Generated nav order refresh + adds getSkill to import example. |
| docs/api-reference/veryfront/server.md | Generated reference refresh reflecting server API shifts. |
| docs/api-reference/veryfront/schemas.md | Generated nav order refresh. |
| docs/api-reference/veryfront/schedule.md | Generated nav order refresh. |
| docs/api-reference/veryfront/sandbox.md | Generated nav order refresh. |
| docs/api-reference/veryfront/runs.md | Generated nav order refresh. |
| docs/api-reference/veryfront/router.md | Generated nav order refresh. |
| docs/api-reference/veryfront/resource.md | Generated nav order refresh. |
| docs/api-reference/veryfront/release-assets.md | Generated reference refresh + adds release asset URL example. |
| docs/api-reference/veryfront/provider.md | Generated nav order refresh. |
| docs/api-reference/veryfront/prompt.md | Generated nav order refresh. |
| docs/api-reference/veryfront/observability.md | Generated nav order refresh. |
| docs/api-reference/veryfront/oauth.md | Generated nav order refresh. |
| docs/api-reference/veryfront/middleware.md | Generated nav order refresh. |
| docs/api-reference/veryfront/metrics.md | Generated nav order refresh. |
| docs/api-reference/veryfront/mdx.md | Generated nav order refresh. |
| docs/api-reference/veryfront/mcp.md | Generated nav order refresh. |
| docs/api-reference/veryfront/markdown.md | Generated nav order refresh. |
| docs/api-reference/veryfront/knowledge.md | Generated nav order refresh. |
| docs/api-reference/veryfront/integrations.md | Generated nav order refresh. |
| docs/api-reference/veryfront/index.md | Generated reference refresh reflecting new exports and line shifts. |
| docs/api-reference/veryfront/index.client.md | Generated nav order refresh. |
| docs/api-reference/veryfront/head.md | Generated nav order refresh. |
| docs/api-reference/veryfront/fs.md | Generated reference refresh (path helper docs/line shifts). |
| docs/api-reference/veryfront/fonts.md | Generated nav order refresh. |
| docs/api-reference/veryfront/eval.md | Generated nav order refresh. |
| docs/api-reference/veryfront/chat.md | Generated reference refresh (line shifts and updated descriptions). |
| docs/api-reference/index.md | Generated index refresh (adds errors module link). |
| deno.json | Adds new exports and workspace entries; adjusts npm build task config; updates import map. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 119 out of 122 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/registry/project-scoped-registry-manager.ts:387
- The JSDoc has a grammatical error that makes the contract harder to read: "the registered value is exact generation" should be "the registered value's exact generation".
src/server/service-server.ts:625 stop()aborts the startup AbortController even after the Node listener is already ready. IncreateNodeServerInternal, aborting a ready signal triggers an additionalnodeServer.stop()(abortListener phase "ready"), so this can cause redundant shutdown work and the abort reason ("stopped before readiness") becomes misleading once the server is listening. Track startup readiness and only abort the startup controller while startup is still pending.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 123 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/workflow/blob/blob-id.test.ts:2
- This new test file is the only one in src/workflow/blob/ that does not preload the shared test setup or use the repo’s assertion helpers. Other tests in this directory import
#veryfront/schemas/_test-setup.tsand#veryfront/testing/assert.ts(for example src/workflow/blob/local-storage.test.ts:1-10). Using the shared helpers keeps behavior consistent across runtimes and avoids drifting assertion APIs.
extensions/ext-blob-gcs/src/index.ts:58 - In the setup rollback path,
generation.storage.close()can throw and replace the original setup error, making the primary failure harder to diagnose. ext-blob-s3 already preserves the original failure by aggregating rollback errors; ext-blob-gcs should do the same so callers reliably see the setup error that caused initialization to fail.
extensions/ext-blob-s3/src/index.ts:26 - This extension already imports
BlobStorageContractName, but thecontracts.provideslist uses a string literal. Using the shared constant avoids drift if the contract name ever changes and makes the contract boundary explicit.
extensions/ext-blob-gcs/src/index.ts:26 - This extension already imports
BlobStorageContractName, but thecontracts.provideslist uses a string literal. Using the shared constant avoids drift if the contract name ever changes and keeps the contract boundary consistent withctx.provide(BlobStorageContractName, ...).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 123 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/extensions/websocket/index.ts:2
- This new public barrel does not follow the established
src/extensions/*/index.tsconvention of declaring an explicit@module ...tag in the header JSDoc (for examplesrc/extensions/sandbox/index.ts:4). Keeping this consistent helps generated API docs and barrel-JSDoc linting remain stable.
src/extensions/websocket/node-websocket-server-provider.ts:7 - This new module header is missing an explicit
@module ...tag, which is consistently present acrosssrc/extensions/**modules (for examplesrc/extensions/auth/auth-provider.ts,src/extensions/schema/index.ts). Adding it keeps doc generation and module documentation consistent.
Summary
Compatibility and migration
require.resolve-backedimport.meta.resolveponyfill, which cannot resolve file URLsActivation safety
Validation
import.meta.resolveabsent and canonical absolute ext-schema-zod factory loading succeedsReview notes
The generated API-reference changes are an intentional refresh after the runtime and export changes. Extension registration and activation are explicit and fail closed; no workspace implementation paths, implicit global fallbacks, path-only identity fallback, or third-party core parser are introduced.