Skip to content

Drop the polyfill for native workspace APIs - #4

Draft
grouville wants to merge 3 commits into
dagger:mainfrom
grouville:polyfill-removal
Draft

Drop the polyfill for native workspace APIs#4
grouville wants to merge 3 commits into
dagger:mainfrom
grouville:polyfill-removal

Conversation

@grouville

@grouville grouville commented Aug 7, 2026 •
edited by eunomie
Loading

Copy link
Copy Markdown
Member

The PHP SDK wrapped every workspace with dagger/polyfill before selecting
modules and returning generated files. Those behaviors are engine APIs now
(dagger/dagger#13854,
#13855), so the dependency goes
away.

Module selection asks the engine: currentModule.asSDK(workspace: ws).modules
returns the modules registered to this SDK that matter for the client's cwd —
everything at or below it, plus the nearest enclosing one when the cwd itself
isn't managed. Membership and cwd policy are both engine-owned, so the config
scan and the hand-rolled path rebasing that fed it are gone.

Changesets use the baseline-diff shape those APIs were built for: stage onto
the workspace value already in hand, then diff against it. No workspace fork
anywhere, because none is needed.

Review notes

The useful review is that generation from a parent directory and from inside a
module still selects the same work, and that init still leaves alone whatever
it didn't put there.

Three native APIs do not behave like the polyfill helpers they replace:

  • ModuleSource.generate + Workspace.changes(from:) does not hold its
    baseline on v1.0.0-beta.10.     Once a module's generated files already exist,
    it reports the whole generated context as added rather than only what
    generation changed. Deleting a single generated file and regenerating
    returned 279 paths instead of 1, including the engine-owned
    dagger-module.toml that an SDK must never write. Only regenerates after the
    first are affected, which is why freshly-generated fixtures never see it. So
    generation stages the local dependency closure itself, takes the module's
    generatedContextChangeset, and re-roots it at the caller's cwd explicitly —
    the shape the polyfill produced. Mod.atCwd carries the reason and the
    engine version and should go once the engine holds the baseline.

  • Workspace.withNewDirectory writes a fresh directory; the polyfill's
    fork.withDirectory merged.     On a host-backed workspace the edit lands on
    an empty delta base and is then diffed against the host tree, so anything
    already at that path is reported as removed — dagger module init into a
    directory a user already has files in deletes them. init now layers the
    template onto the existing contents. sdk-sdk's
    contract:does-not-remove-existing-files only ever inits into an empty path
    and stayed green throughout, so e-2-e:init-layering-check covers it.

  • Generated files are rooted at the caller's cwd, not the workspace root.
    A changeset taken straight from generatedContextChangeset is measured from
    the module source's context directory, so returning it unrooted writes the
    client one module path deeper than it belongs —
    mymod/mymod/sdk/generated/. Every generation fixture ran the Dang runtime
    and emitted no files, so the suite could not see it.
    e-2-e:generate-rooting-check runs real PHP codegen from inside a module and
    asserts the added paths are cwd-relative. The same gap exists in sdk-sdk's
    shared generation:respects-cwd, which only asserts the sibling module is
    untouched — worth hardening there so every SDK is covered, after which this
    PHP-only check can retire.

Module paths are anchored at / before they cross into the workspace, since a
relative path resolves from ws.cwd. The engine hands initModule a
root-anchored workspace, so this only shows up on a direct call from a
subdirectory, and no check covers it — it was verified by hand.

Not addressed here, but noticed while reviewing: Mod.path computes its
cwd-relative path by prefix stripping, which returns a workspace-root-relative
path instead of a ../-relative one when mod() resolves a module outside the
cwd cone (dagger call php-sdk mod --path ../lookup/app path from a sibling
directory). It predates this PR and only affects the path-driven mod()
lookup, never the engine-scoped modules() list, so it's left for its own
change.

Test

dagger check

33/33 against a v1.0.0-beta.10 engine, including the monorepo group added in
dagger/sdk-sdk#17. dagger module init php and dagger generate were also
driven by hand end to end — into a fresh path, into a directory that already
had files, from inside the new module's directory, and regenerating after
deleting a generated file — to confirm nothing is deleted, that a regenerate
reports only what actually changed, and that the generated PHP client lands
where it belongs.

@grouville grouville mentioned this pull request Aug 7, 2026
Open
@eunomie
eunomie force-pushed the polyfill-removal branch 3 times, most recently from 897ba97 to 87b6c33 Compare August 21, 2026 10:29
grouville and others added 3 commits August 21, 2026 12:39
@grouville @eunomie
Drop the polyfill for native workspace APIs
3e2a425 
The behaviors this module leaned on `dagger/polyfill` for now live in the
engine: dagger/dagger#13854 added the native workspace and module-source
APIs, and #13855 gave `Workspace.changes` an explicit baseline.

Module discovery goes through `currentModule.asSDK(workspace: ws).modules`,
which returns the modules registered to this SDK that are relevant to the
client's cwd. The engine owns both membership and the cwd policy, so the SDK
no longer scans for config files or rebases discovery results by hand — the
`findConfigDirs` walk and its `workspacePath` helper go away with it.

Changesets follow the baseline-diff shape the native APIs were designed for:
stage onto the workspace value already in hand, then diff against it. No
workspace fork is involved, because none is needed. `generateAll` folds the
per-module changesets together with `changeset.withChangesets`.

Generation would read as `ModuleSource.generate(ws).changes(ws)`, which is
what #13855 built the pair for. On v1.0.0-beta.10 that pair does not hold its
baseline: once a module's generated files already exist, it reports the whole
generated context as added rather than only what generation changed —
deleting one file and regenerating returned 279 paths instead of 1, including
the engine-owned dagger-module.toml. So generation stages the local dependency
closure itself, takes the module's `generatedContextChangeset`, and re-roots
that at the caller's cwd explicitly, which is the shape the polyfill produced.
`Mod.atCwd` carries the reason and the engine version, so it can go once the
engine holds the baseline.

Staging paths anchor at "/" because the native workspace resolves a relative
path from the cwd, where the polyfill's took workspace-root paths.

`Workspace.withNewDirectory` is not a drop-in for the polyfill fork's
`withDirectory`: on a host-backed workspace the edit lands on an empty delta
base and is then diffed against the host tree, so whatever already sat at
that path is reported as removed. The polyfill's merged instead. init has to
layer the template onto the existing contents itself, or `dagger module init`
into a directory a user already has files in deletes them. sdk-sdk's
`contract:does-not-remove-existing-files` only inits into an empty path and
cannot catch that, so `e-2-e:init-layering-check` covers it here.

The engineVersion bump and the dependency removal land together: the version
gate scopes the engine's new changeset rooting to migrated modules.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>
Signed-off-by: Yves Brissaud <yves@dagger.io>
@eunomie
Check that generated changesets stay rooted at the caller's cwd
a665463 
Nothing in the suite noticed where a generated client actually lands. Every
generation fixture runs the Dang runtime, which emits no files, so the
existing cwd check compares two empty changesets and passes either way.

That leaves a real trap uncovered. `ModuleSource.generatedContextChangeset`
measures its paths from the module source's context directory, not from the
client's cwd — reaching for it directly instead of going through
`ModuleSource.generate` writes the generated client one module path deeper
than it belongs, e.g. `mymod/mymod/sdk/generated/`. The engine says as much
where the field is defined: a caller that needs cwd-relative paths applies
the generated files to a workspace and diffs it against the workspace it
started from.

Catching that needs a module whose codegen emits something, so this fixture
runs the PHP runtime. It carries only a module config: codegen produces the
composer scaffolding and the client itself, which is exactly what the check
inspects, so there is nothing to commit alongside it.

Signed-off-by: Yves Brissaud <yves@dagger.io>
@eunomie
chore: pin sdk-sdk to 3344489
9f3ed90 
The committed lock was still in the v1 format, whose `float` entry the current
CLI no longer honors: every run silently re-resolved sdk-sdk to whatever main
pointed at and rewrote the file. Recording it in v2 makes the revision the
checks actually run against explicit again.

Pinning it here also matters for what it contains. The old entry named
`e1747f4`, which still declared `daggerCliVersion = "1.0.0-beta.9"`, and a
beta.9 CLI cannot drive a beta.10 engine — beta.10 moved registering a
function's required `Workspace` argument as a CLI flag out of the engine and
into the CLI, so the older CLI never sees `initModule` and the contract checks
fail with `unknown command "init-module"`. dagger/sdk-sdk#14 fixed that pin,
and dagger/sdk-sdk#17 then added the `monorepo` group, which drives a workspace
whose dagger.toml sits in a git subdirectory — the layout where module paths
cross the engine/SDK boundary root-relative while the selected config reads
them relative to itself. That is exactly what this module's path anchoring has
to get right, so it is worth being on.

Signed-off-by: Yves Brissaud <yves@dagger.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@grouville @eunomie