feat: add EnvironmentDocumentHandler for offline handlers without a filesystem - #299
Open
justin-masse wants to merge 1 commit into
Open
justin-masse wants to merge 1 commit into
justin-masse wants to merge 1 commit into
Conversation
…ilesystem Custom offline handlers are documented, but building one is only possible if the environment document comes from disk: the engine needs the API's JSON rehydrated by buildEnvironmentModel, which is not exported from the package root and cannot be deep-imported because the exports map has no subpaths. The only public route is LocalFileHandler. EnvironmentDocumentHandler takes an already-parsed environment document, so a handler can load it from anywhere - an object store, a cache, a database, or a document embedded in the deployment - and hand it straight to the SDK. LocalFileHandler now extends it and keeps its existing behaviour and public surface, so reading from a file is just the filesystem-shaped case of loading a document. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
justin-masse
requested review from
matthewelwell
and removed request for
a team
September 16, 2026 18:16
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
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.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #187
Custom offline handlers are documented — subclass
BaseOfflineHandler, return anEnvironmentModel— but in practice you can only build one if the environment document comes from disk.The engine needs the API's JSON rehydrated into its model graph by
buildEnvironmentModel. That function isn't exported from the package root, and it can't be deep-imported either, because theexportsmap has no subpaths:So
LocalFileHandleris the only public route to a usable handler. If your environment document lives anywhere else, you end up writing it to a temp file purely to read it back — which is what we're currently doing on AWS Lambda, where the document comes from DynamoDB and the filesystem round-trip is pure overhead.This adds
EnvironmentDocumentHandler, which takes an already-parsed environment document:LocalFileHandlernow extends it, so reading from a file becomes the filesystem-shaped case of loading a document rather than a separate implementation. Its behaviour and public surface are unchanged — same constructor, sameenvironmentproperty, still aninstanceof BaseOfflineHandler.This follows on from #187, which exported
FeatureModelfor the same reason: the models a handler returns are public, but the function that builds one from your API's response isn't, so the job can't quite be finished. Happy to take this in a different direction if you'd prefer — exportingbuildEnvironmentModeldirectly would also solve it, and I'm not attached to the class name.Notes
objectrather thanany, so a parsed document passes straight through while a stray string or path doesn't.How did you test this code?
npx vitest run— 188 passing, up from 185. Three new cases intests/sdk/offline-handlers.test.ts:EnvironmentDocumentHandlerbuilds a validEnvironmentModelfrom a document, asserting onapiKeythe same way the existingLocalFileHandlertest does.fs.readFileSyncis spied on and asserted not called.LocalFileHandleris aninstanceof EnvironmentDocumentHandler, covering the refactor.The existing
LocalFileHandlertest is unchanged and still passes, which is the compatibility check.Two pre-existing failures are unrelated to this change and reproduce identically on a clean checkout:
tests/engine/e2e/engine.test.ts(needs live credentials) and somepinotype errors duringtscthat come from resolving dependencies without yourpackage-lock.json.npx prettier --checkpasses on all three changed files.