Repository navigation
Drop the polyfill for native workspace APIs - #30
Merged
Merged
Conversation
grouville
force-pushed
the
polyfill-removal
branch
3 times, most recently
from
August 14, 2026 07:52
38be4b1 to
90545b2
Compare
eunomie
force-pushed
the
polyfill-removal
branch
from
August 21, 2026 09:01
90545b2 to
f14895f
Compare
eunomie
added a commit
to grouville/python-sdk
that referenced
this pull request
Aug 21, 2026
Workspace.withNewDirectory replaces the directory it writes. The polyfill's fork.withDirectory layered onto what was already there, so swapping one for the other turned `dagger module init` into a delete: running it against a directory that already held files reported them all as removed, including the module's own engine-owned dagger.json. Layer the template onto the existing directory instead, and check it. The sdk-sdk contract suite already asserts init removes nothing, but it only ever inits into a fresh path, so it cannot see this. Found by the dagger/go-sdk#30 sibling, which hit the same substitution. Signed-off-by: Yves Brissaud <yves@dagger.io>
sdk-sdk#20 made every check group honour the `daggerCliVersion` setting and moved its default to 1.0.0-beta.10. Before that, the contract group built its target from mod-test's own default of beta.9, and a beta.9 CLI cannot see a function taking a required Workspace argument — so `initModule` was invisible and all four contract checks failed as "initModule should seed at least one SDK file", with `module:loads` failing the same way. The pinned e1747f4 predates that fix, which is why those seven checks are red on main today. Move the pin to sdk-sdk main; it also carries sdk-sdk's own polyfill removal, its switch to the native ModuleSource fields, and a new monorepo group covering workspace configs in a git subdirectory. Signed-off-by: Yves Brissaud <yves@dagger.io>
eunomie
force-pushed
the
polyfill-removal
branch
from
August 21, 2026 10:32
f14895f to
5f73ef7
Compare
TomChv
added a commit
to dagger/typescript-sdk
that referenced
this pull request
Aug 24, 2026
Workspace.withNewDirectory replaces the directory it writes, where the polyfill's fork.withDirectory merged onto it. Dropping the polyfill (#33) flipped that semantic under two call sites, so both deleted files they do not own. initModule wiped the destination: `dagger module init typescript hello` over a directory holding a foo.txt removed it, along with the module config the engine writes there before calling the SDK. It now layers the rendered starter onto existingDir(ws, modPath), as dagger/go-sdk#30 and dagger/python-sdk#14 did. generateClient / generateAllClient wiped hand-written files in the client package — the bug #9 fixed with an overlay, lost when the polyfill went away. Clients keep the user's files and still drop the *.gen.ts bindings of a module that has left the closure: the SDK owns that set, the user owns the rest. go-sdk and python-sdk keep plain replace for clients; this repo does not, given #9. Pruning has to happen twice, because withNewDirectory is not one operation. On a local-directory workspace it replaces what it writes; on a synthetic one — the shape a git-loaded workspace has, and how the Cloud checks runner loads this repo — it merges. Isolated, same engine, same call: local removed: main.ts, package.json, stale-dep.gen.ts synthetic removed: (nothing) So the bindings come out of the baseline, covering replace, and off the workspace with withoutFile, covering merge. The baseline still reads from the untouched workspace: reading it back out of the pruned one comes up empty on a local directory, which drops the user's files. Reported as dagger/dagger#13955. Three checks guard this, none of which pass without the fix: - init:init-over-existing-check inits over a fixture module and asserts removedPaths is empty. Before: dagger.json, index.ts, nested/. - client:generate-client-respects-existing-check gains main.ts (must survive) and stale-dep.gen.ts (must be pruned), so it fails under replace and under a plain overlay alike. - client:generate-client-on-synthetic-workspace-check covers the other half of the split, asserting the resulting tree rather than the removals, since removals are what diverge. It builds the workspace with ws.directory("/").asWorkspace, so the CI shape is reachable locally with no git pin. A config-file fixture cannot catch any of it: config-updator merges those files, so they survive a replace and the check passes anyway. Verified with `dagger check 'e-2-e*'` (31/31) on a v1.0.0-beta.10 engine and 60/60 in CI, plus an engine-driven `dagger module init typescript hello -y` over a directory holding an unrelated file, which now keeps it. Signed-off-by: Tom Chauveau <tom@dagger.io>
eunomie
marked this pull request as ready for review
August 24, 2026 13:49
The Go SDK used dagger/polyfill to discover managed modules, load module sources, stage generated files, and return only the changes made by one operation. The engine now provides those behaviors directly. Use currentModule.asSDK(workspace).modules for managed modules, load module sources and clients straight off the Workspace, and bump the engine requirement so returned paths are translated exactly once. Workspace.withNewDirectory replaces the directory it writes, where the polyfill's fork.withDirectory layered onto it. That is what generateClient wants — its input already carries the existing client dir, so a replace prunes stale generated files — but initModule and module generation must not remove anything, so both merge their input onto what is already there before writing it back. e2e covers init: initializing over an existing module now keeps that module's files. Module generation writes the generated context as a directory rather than threading a workspace through ModuleSource.generate. generate is the API meant for this, but on v1.0.0-beta.10 applying a changeset to a workspace and diffing it back loses the baseline: a module's entire generated context reports as added rather than only what generation changed. Deleting one generated file and regenerating returned six, including the engine-owned module config. A directory overlay diffs correctly, so Workspace.changes still does the cwd rooting and the outside-the-cwd guard. generateModuleCheck now commits the fixture's .gitattributes so a generator that returns its whole context is caught. moduleConfigFilenames had no reader left once discovery moved to the engine; mod()'s find-up ladder names both config files itself. Signed-off-by: Guillaume de Rouville <guillaume@dagger.io> Signed-off-by: Yves Brissaud <yves@dagger.io>
The document records a helper boundary that no longer exists: it directs generation and workspace changeset rooting through github.com/dagger/sdk-sdk/polyfill, and its flow descriptions call the polyfill wrapper at each step. Every gap it works around is now native engine API, so the doc's advice is to restore the indirection this repo just removed. Signed-off-by: Yves Brissaud <yves@dagger.io>
eunomie
force-pushed
the
polyfill-removal
branch
from
August 24, 2026 14:32
5f73ef7 to
7b5e9aa
Compare
TomChv
approved these changes
Aug 24, 2026
TomChv
added a commit
to dagger/typescript-sdk
that referenced
this pull request
Aug 24, 2026
Workspace.withNewDirectory replaces the directory it writes, where the polyfill's fork.withDirectory merged onto it. Dropping the polyfill (#33) flipped that semantic under two call sites, so both deleted files they do not own. initModule wiped the destination: `dagger module init typescript hello` over a directory holding a foo.txt removed it, along with the module config the engine writes there before calling the SDK. It now layers the rendered starter onto existingDir(ws, modPath), as dagger/go-sdk#30 and dagger/python-sdk#14 did. generateClient / generateAllClient wiped hand-written files in the client package — the bug #9 fixed with an overlay, lost when the polyfill went away. Clients keep the user's files and still drop the *.gen.ts bindings of a module that has left the closure: the SDK owns that set, the user owns the rest. go-sdk and python-sdk keep plain replace for clients; this repo does not, given #9. Pruning has to happen twice, because withNewDirectory is not one operation. On a local-directory workspace it replaces what it writes; on a synthetic one — the shape a git-loaded workspace has, and how the Cloud checks runner loads this repo — it merges. Isolated, same engine, same call: local removed: main.ts, package.json, stale-dep.gen.ts synthetic removed: (nothing) So the bindings come out of the baseline, covering replace, and off the workspace with withoutFile, covering merge. The baseline still reads from the untouched workspace: reading it back out of the pruned one comes up empty on a local directory, which drops the user's files. Reported as dagger/dagger#13955. Three checks guard this, none of which pass without the fix: - init:init-over-existing-check inits over a fixture module and asserts removedPaths is empty. Before: dagger.json, index.ts, nested/. - client:generate-client-respects-existing-check gains main.ts (must survive) and stale-dep.gen.ts (must be pruned), so it fails under replace and under a plain overlay alike. - client:generate-client-on-synthetic-workspace-check covers the other half of the split, asserting the resulting tree rather than the removals, since removals are what diverge. It builds the workspace with ws.directory("/").asWorkspace, so the CI shape is reachable locally with no git pin. A config-file fixture cannot catch any of it: config-updator merges those files, so they survive a replace and the check passes anyway. Verified with `dagger check 'e-2-e*'` (31/31) on a v1.0.0-beta.10 engine and 60/60 in CI, plus an engine-driven `dagger module init typescript hello -y` over a directory holding an unrelated file, which now keeps it. Signed-off-by: Tom Chauveau <tom@dagger.io>
eunomie
added a commit
to eunomie/python-sdk
that referenced
this pull request
Aug 25, 2026
Generation threaded the workspace through ModuleSource.generate and diffed it back with Workspace.changes. On v1.0.0-beta.10 that loses the baseline: the module's whole generated context comes back as added rather than only what generation changed. Every generate and generateAll claimed to add the module's own dagger.json, which the engine owns and had not touched, and once a module has generated output on disk, files codegen rewrites byte for byte are reported as added too. The engine defect is in applying a changeset, not in comparing workspaces: withChanges projects the structural before/after diff, which a fresh mtime alone puts a path into, rather than the content-based paths the changeset reports. dagger/dagger#13947 fixes it. Write the generated context as a directory instead, the shape go-sdk settled on in dagger/go-sdk#30. A directory overlay diffs correctly, so Workspace.changes still does the cwd rooting and the outside-the-cwd guard, and nothing here has to be reverted once the engine fix lands. generatedContextDirectory does not resolve the local dependency closure the way generate(ws) did, so generation stages it explicitly, and generateAll merges each module's changeset rather than threading one workspace through all of them. The generate fixture now commits the .gitattributes that generation emits, so e-2-e:generate-skips-existing-files-check catches a generator returning its whole context: every other file there is genuinely new on a fresh module. Signed-off-by: Yves Brissaud <yves@dagger.io>
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.
The Go SDK no longer needs
dagger/polyfill.Managed modules come from
currentModule.asSDK(workspace: ws).modules, using the registrations already present indagger.toml. Generation now keeps the workspace it received, applies each module's generated files to it, then returns only the difference:That explicit baseline matters during module initialization: the input already contains the new config and scaffold, so the SDK sees those files without returning them a second time. Paths are made cwd-relative once, by
Workspace.changes(from:).Init, clients, and config edits use the same pattern, and the polyfill dependency is removed.
Module generation writes a directory, not a workspace
ModuleSource.generate(workspace)is the API meant to replace the polyfill here, and its documentation promises the baseline survives:On
v1.0.0-beta.10it does not. Applying a changeset to a workspace and diffing it back reports the module's entire generated context as added rather than only what generation changed:Six files for a one-file change, including
dagger-module.toml, which the SDK must never touch. Reduced, the round-trip loses the baseline on its own:A directory overlay does not have this problem, so generation writes
generatedContextDirectoryinto the workspace instead. That context holds only generated files, so it is merged onto the module rather than replacing it, andWorkspace.changesis left to do the cwd rooting and the outside-the-cwd guard as intended.dagger generatereports1 file changedagain, from the workspace root and from inside the module directory alike.This should collapse to
moduleSource.generate(ws).changes(ws)once the engine's changeset baseline is fixed, and the merge step goes away onceWorkspacegrows a layeringwithDirectory.generateModuleChecknow commits the fixture's.gitattributesso a generator returning its whole context is caught: every other file in that fixture is genuinely new, which is why the suite stayed green through this.initModulelayers, it does not replaceWorkspace.withNewDirectoryreplaces the directory it writes, where the polyfill'sfork.withDirectorylayered onto it. That is whatgenerateClientwants — its input already carries the existing client directory, so replacing prunes stale generated files — butinitModulemust never remove anything. Running it over a directory that already held files removed that directory'sdagger.jsonand its subdirectories; it now layers the starter onto whatever is already at the destination.e-2-e:init-over-existing-checkcovers it.sdk-sdk pin
The pinned
e1747f4predates sdk-sdk#20, which made every check group honour thedaggerCliVersionsetting and moved its default to1.0.0-beta.10. Before that the contract group built its target from mod-test's own default of beta.9, and a beta.9 CLI cannot see a function taking a requiredWorkspaceargument — soinitModulewas invisible and the four contract checks failed as "initModule should seed at least one SDK file", withmodule:loadsfailing the same way. Those seven checks are red onmaintoday; the pin bump is a separate commit because it fixes them on its own.Test
dagger check53 checks, all passing: the
e-2-esuite plus thedagger/sdk-sdkconformance groups.Requires dagger/dagger#13854 and dagger/dagger#13855.