Skip to content

Keep existing files when init and client codegen write a directory - #35

Merged
TomChv merged 1 commit into
mainfrom
fix/init-and-client-keep-existing-files
Aug 24, 2026
Merged

Keep existing files when init and client codegen write a directory#35
TomChv merged 1 commit into
mainfrom
fix/init-and-client-keep-existing-files

Conversation

@TomChv

@TomChv TomChv commented Aug 24, 2026

Copy link
Copy Markdown
Member

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, and both started deleting files they
do not own.

initModule wiped the destination

Running dagger module init typescript hello over a directory that already held
a foo.txt removed it — along with the module config the engine writes at that
path just before calling the SDK. It now layers the rendered starter onto
existingDir(ws, modPath), the same fix as dagger/go-sdk#30 and
dagger/python-sdk#14.

generateClient / generateAllClient wiped hand-written client files

The 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). Confirmed
before the fix:

$ dagger call typescript-sdk generate-client \
    --module .dagger/modules/e2e/fixtures/client/app \
    --path .dagger/modules/e2e/fixtures/client/existing removed-paths

.dagger/modules/e2e/fixtures/client/existing/main.ts

Clients now keep the user's files and still lose the *.gen.ts bindings of a
module 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.

withNewDirectory means two different things

The 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 withNewDirectory is not one operation:

workspace kind withNewDirectory at an existing path
local directory replaces it
synthetic / git-loaded (how the Cloud checks runner loads this repo) merges into it

Isolated, same engine, same call, same target — changes.removedPaths was
main.ts, package.json, stale-dep.gen.ts against the local workspace and empty
against ws.directory("/").asWorkspace.

So pruning cannot ride on replace. The bindings are removed from the workspace
with withoutFile as well, which prunes identically under both. The baseline
still 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, asserts
    removedPaths is empty. Before the fix: 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. It reports the paths it saw, so a future
    divergence names itself.
  • client:generate-client-on-synthetic-workspace-check — the same guarantees on
    a 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 a
    git 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 the
check 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.10 engine, 60/60 in CI. Also verified
engine-driven against the working tree: dagger module init typescript hello -y
over a directory holding an unrelated test.txt scaffolds the module and keeps
the file.

@TomChv

TomChv commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

Filed the engine half as dagger/dagger#13955Workspace.withNewDirectory replaces on a host-backed workspace and merges on a synthetic or git-loaded one, with a four-file repro. The workarounds in this PR stand either way, but if that lands as merge, withoutClientBindings becomes the only thing prunes stale bindings and the directory-level strip in existingClientBase can go.

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
TomChv force-pushed the fix/init-and-client-keep-existing-files branch from 82243a2 to 0bc76ad Compare August 24, 2026 13:34
@TomChv
TomChv merged commit ff9fd61 into main Aug 24, 2026
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant