Skip to content

core/schema: don't prefix main workspace module checks#12951

Draft
eunomie wants to merge 1 commit intodagger:mainfrom
eunomie:fix-prefix-nodes
Draft

core/schema: don't prefix main workspace module checks#12951
eunomie wants to merge 1 commit intodagger:mainfrom
eunomie:fix-prefix-nodes

Conversation

@eunomie
Copy link
Copy Markdown
Member

@eunomie eunomie commented Apr 10, 2026

reparentWorkspaceTreeRoot was unconditionally wrapping every primary workspace module's tree root with a synthetic parent named after the module, so every check/generator/service path was rendered as <module>:<item> — including the workspace's own root module.

As a result, dagger check -l on this repo showed dagger-dev:generated instead of just generated, contradicting the documented behavior (docs/current_docs/introduction/core-concepts/checks.mdx) where only toolchain items are namespaced. The integration contract is already asserted strictly for toolchains by TestChecksAsToolchain but only loosely for main modules by TestChecksDirectSDK / TestChecksAsBlueprint (via Contains, which accepts either form).

Fix: have SchemaBuilder.PrimaryMods also return the set of "main" (entrypoint-proxied) module names, and skip the reparent for those modules inside reparentWorkspaceTreeRoot. Main modules are the ones installed with Entrypoint — the workspace root module or a blueprint — whose main-object methods are proxied onto the Query root. Toolchain modules remain prefixed with their module name.

Also strengthen TestChecksDirectSDK and TestChecksAsBlueprint with NotContains assertions on the prefixed form, so a future regression surfaces immediately.

`reparentWorkspaceTreeRoot` was unconditionally wrapping every primary
workspace module's tree root with a synthetic parent named after the
module, so every check/generator/service path was rendered as
`<module>:<item>` — including the workspace's own root module.

As a result, `dagger check -l` on this repo showed
`dagger-dev:generated` instead of just `generated`, contradicting the
documented behavior (docs/current_docs/introduction/core-concepts/checks.mdx)
where only toolchain items are namespaced. The integration contract is
already asserted strictly for toolchains by `TestChecksAsToolchain` but
only loosely for main modules by `TestChecksDirectSDK` /
`TestChecksAsBlueprint` (via `Contains`, which accepts either form).

Fix: have `SchemaBuilder.PrimaryMods` also return the set of "main"
(entrypoint-proxied) module names, and skip the reparent for those
modules inside `reparentWorkspaceTreeRoot`. Main modules are the ones
installed with `Entrypoint` — the workspace root module or a blueprint —
whose main-object methods are proxied onto the Query root. Toolchain
modules remain prefixed with their module name.

Also strengthen `TestChecksDirectSDK` and `TestChecksAsBlueprint` with
`NotContains` assertions on the prefixed form, so a future regression
surfaces immediately.

Signed-off-by: Yves Brissaud <yves@dagger.io>
@shykes
Copy link
Copy Markdown
Contributor

shykes commented Apr 10, 2026

I think maybe we shouldn't merge this (or any other attempt to patch the prefix change).

Rationale:

  • The current behavior doesn't actually break users that I know of. dagger check -l OLD_PATH and dagger check -l NEW_PATH both list the same check. dagger check OLD_PATH and dagger check NEW_PATH both execute the same path. Besides the visual difference, there is no known breakage at the moment. So there is no urgency to fix.

  • This change feels like adding duct tape, in a highly sensitive area of the code - workspace/module compat just went through significant changes, and its about to go through even more changes. One of the main people who was involved (@vito) is currently AFK for 2 weeks.

TLDR: big risk, small reward. I recommend just leaving it as is for now.

NOTE: this assumes I'm correct that nothing breaks. If I am wrong please tell me!

@eunomie eunomie marked this pull request as draft April 11, 2026 05:39
@github-actions
Copy link
Copy Markdown
Contributor

This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants