Keep existing files when init and client codegen write a directory - #35
Merged
Conversation
Member
Author
|
Filed the engine half as dagger/dagger#13955 — |
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>
TomChv
force-pushed
the
fix/init-and-client-keep-existing-files
branch
from
August 24, 2026 13:34
82243a2 to
0bc76ad
Compare
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.
Workspace.withNewDirectoryreplaces the directory it writes, where thepolyfill's
fork.withDirectorymerged onto it. Dropping the polyfill (#33)flipped that semantic under two call sites, and both started deleting files they
do not own.
initModulewiped the destinationRunning
dagger module init typescript helloover a directory that already helda
foo.txtremoved it — along with the module config the engine writes at thatpath just before calling the SDK. It now layers the rendered starter onto
existingDir(ws, modPath), the same fix as dagger/go-sdk#30 anddagger/python-sdk#14.
generateClient/generateAllClientwiped hand-written client filesThe same bug, and one this repo had already fixed once: #9 overlaid the generated
client onto the existing directory, and that overlay was lost with the polyfill
(the fixture guarding it,
client/existing/main.ts, went with it). Confirmedbefore the fix:
Clients now keep the user's files and still lose the
*.gen.tsbindings of amodule that has left the closure. That ownership split — the SDK owns
*.gen.ts,the user owns everything else — is deliberate for this repo, given #9; go-sdk and
python-sdk keep plain replace for clients.
withNewDirectorymeans two different thingsThe first attempt at that split pruned by leaving the bindings out of the
directory being written. It passed locally and failed in CI, twice. The cause is
that
withNewDirectoryis not one operation:withNewDirectoryat an existing pathIsolated, same engine, same call, same target —
changes.removedPathswasmain.ts, package.json, stale-dep.gen.tsagainst the local workspace and emptyagainst
ws.directory("/").asWorkspace.So pruning cannot ride on replace. The bindings are removed from the workspace
with
withoutFileas well, which prunes identically under both. The baselinestill reads from the untouched workspace — reading it back out of the pruned one
comes up empty on a local directory, which would drop the user's files.
This divergence looks worth an engine issue on its own; nothing in the API
suggests the two workspace kinds differ here.
Guards
init:init-over-existing-check— inits over a fixture module, assertsremovedPathsis empty. Before the fix:dagger.json,index.ts,nested/.client:generate-client-respects-existing-check— gainsmain.ts(mustsurvive) and
stale-dep.gen.ts(must be pruned), so it fails under replaceand under a plain overlay. It reports the paths it saw, so a future
divergence names itself.
client:generate-client-on-synthetic-workspace-check— the same guarantees ona synthetic workspace, asserting the resulting tree rather than the removals,
since removals are the thing that diverges. Built with
ws.directory("/").asWorkspace, so the CI shape is reachable locally without agit pin.
A config-file fixture cannot catch any of this: config-updator merges
package.json/tsconfig.json/deno.json, so those survive a replace and thecheck passes anyway. The guard only bites on a file the SDK does not write.
Test
dagger check 'e-2-e*'31/31 locally on a
v1.0.0-beta.10engine, 60/60 in CI. Also verifiedengine-driven against the working tree:
dagger module init typescript hello -yover a directory holding an unrelated
test.txtscaffolds the module and keepsthe file.