#TestWorkspaceModules/TestWorkspaceModuleInstall
1 messages · Page 1 of 1 (latest)
I’m looking into the workspace branch regression now. I’ll inspect the current branch, recent changes, and available test failures, then report back here.
I don’t see a project checkout in this VM yet: /home/exedev is not a git repo, and the only .git directory I can find is Codex’s internal plugin cache. There’s also no thread history beyond the mention, so I don’t have the workspace branch or failing command to inspect. Please attach/provide the repo checkout details, branch name, or failing test/error and I’ll continue from there.
I will tell you what the regression is. For now just clone dagger/dagger
and install dagger from the main release (official install.sh)
@fossil fulcrum
Cloned dagger/dagger into /home/exedev/dagger. It is on main at 08e15e862 and clean.
Installed Dagger from the official stable install.sh into /home/exedev/.local/bin. dagger version reports v0.20.8 (engine image registry.dagger.io/engine:v0.20.8) linux/amd64.
Expected:
- When I load a workspace that doesn't have a config.toml, we fall back to finding the nearest dagger.json, and project that into a workspace. We show a warning about migrating soon; and proceed with the session (NO ERROR)
Actual:
- In some cases, dagger.json fallback results in an ERROR because of required migration.
I don't know exactly which commit introduced this. Can you track it down please?
I’ll track it down. I’m going to build a minimal reproduction for the config.toml-missing / dagger.json fallback path, then bisect commits once I have a reliable good/bad command.
Tracked this down on origin/workspace. The regression was introduced by 5fa54b9ba46758dee8c55d74ecfa5c4dc3930146 (Remove 'CWD module' feature.). That commit explicitly removed the fallback that find-ups the nearest dagger.json and loads it when no .dagger/config.toml exists, changing the server test from LoadsCWDModuleWithoutConfig to DoesNotInferModuleFromCWDWithoutConfig.
The branch now appears to contain the fix: 2e0e80060eb90fd884403693b3931376391726d3 (workspace: restore compat migration for plain modules) restores runtime compat via ParseRuntimeCompatWorkspaceAt, including SDK-only/plain module configs. I verified the focused server tests pass on current workspace: go test ./engine/server -run 'TestDetectAndLoadWorkspaceLoadsPlainModuleCompatWithoutConfig|TestDetectAndLoadWorkspaceKeepsCompatFallbackForExplicitExtraModule'.
Some other infos: It impacted that test because the test’s fixture is exactly a plain SDK standalone module:
{
"name": "app",
"engineVersion": "latest",
"sdk": { "source": "go" }
}
That test runs:
dagger install ./dep
from that module root, and expects:
- create .dagger/config.toml
- install dep there
- leave dagger.json alone
Before Solomon’s commit, this plain SDK dagger.json was not treated as a compat workspace, so install could initialize .dagger/config.toml.
After Solomon’s commit, runtime compat got broader:
return cfg != nil && cfg.SDK != nil && cfg.Name != ""
So that same plain SDK dagger.json now produces parent.CompatWorkspace() != nil.
The bug was that workspace install had this kind of guard:
if parent.CompatWorkspace() != nil {
return "", fmt.Errorf("workspace is using legacy dagger.json config; run dagger migrate first")
}
That guard made sense for legacy workspace-like configs, but became too broad once plain SDK modules also had runtime compat. So the
standalone module test started failing because dagger install ./dep was blocked before it could create .dagger/config.toml.
So the impact chain is:
plain SDK dagger.json
-> Solomon: now valid runtime compat
-> parent.CompatWorkspace() becomes non-nil
-> old install guard thinks "legacy workspace, must migrate"
-> install fails
-> test fails
The fix was to distinguish:
compat != nil && compat.MustMigrateToWorkspaceConfig()
Plain SDK modules are runtime-compatible, but they do not require migration before workspace mutation. So dagger install should still be
allowed to create .dagger/config.toml for them.
Correction/update based on the more specific dagger install ./dep path: for the install regression, the first bad commit is 2e0e80060eb90fd884403693b3931376391726d3 (workspace: restore compat migration for plain modules), not 5fa54b9ba by itself.
5fa54b9ba removed the old CWD-module fallback, but the install-specific failure comes from 2e0e80060 broadening runtime compat so plain SDK modules (sdk + name) make parent.CompatWorkspace() != nil. The existing mutation guards in core/schema/workspace_install.go and core/schema/workspace_config.go then reject dagger install with “run dagger migrate first.” That guard needs to be narrowed to compat != nil && compat.MustMigrateToWorkspaceConfig() so plain SDK runtime compat can still initialize .dagger/config.toml.
<@&1506565370385793125> did you see guillaume's message? wdyt
Yes, I saw it, and I agree with Guillaume’s analysis. My earlier answer was looking at the ambient runtime fallback in isolation; for the failing dagger install ./dep test, the real regression chain is the broader runtime compat added in 2e0e80060 plus the old mutation guard.
The important distinction is: plain SDK dagger.json should be runtime-compatible, but it should not be treated as a legacy workspace that must migrate before mutation. So the fix should be to narrow the guards in workspace mutation paths from CompatWorkspace() != nil to compat != nil && compat.MustMigrateToWorkspaceConfig(). I see the broad guards currently in core/schema/workspace_install.go and core/schema/workspace_config.go, so those are the places I’d change.
TestWorkspaceModules/TestWorkspaceModuleInstall/workspace_install_creates_workspace_config_instead_of_editing_dagger.json_for_standalone_modules
TestWorkspaceModules/TestWorkspaceModuleInstall
cc @candid frigate
(we're experimenting with collaborative codex sessions directly in discord for better context sharing)
<@&1506565370385793125> here's my problem with removing the guard in this case. It may be safe in this particular test fixture because it's a single vanilla module at the root of an otherwise empty workspace. So there is nothing to migrate; and therefore it's safe to just install into a new workspace config.
BUT, you're not proposing that we check that "there's nothing to migrate". You propose we check that "this particular module doesn't need to be migrated". But what if there is stuff to migrate, elsewhere in the workspace? Then our innocent little install basically short-circuits the whole migration -> drift, huge fail. So, the guard is actually acting as intended I think.
I noticed that detectAndLoadWorkspaceWithRootfs fails to load a compat legacy workspace when no .git repo exists, which is causing some legacy tests to fail -- so that's another edge case to consider.
Yeah that's definitely a breaking change but it affects our tests disproporitionally compared to real users 🙂
Is it realistic to just have the fixtures have their own .git? Or is that a pain to track a .git in git?
@near nova hello?
I'm taking a look to see if there's a way to work past that limitation
Confirmed that is the exact subtest. I patched the workspace branch locally to narrow the mutation guards:
core/schema/workspace_install.go:CompatWorkspace() != nil->compat != nil && compat.MustMigrateToWorkspaceConfig()core/schema/workspace_config.go: same change forworkspace initandloadWorkspaceConfigForMutation- added
TestLoadWorkspaceConfigForMutationCompatMigrationGuardso plain SDK runtime compat reaches the normal “no config.toml”/init path, while toolchain/blueprint compat still says migrate first.
Focused tests pass:
go test ./core/schema -run 'TestLoadWorkspaceConfigForMutationCompatMigrationGuard|TestWorkspaceConfigWithCompatFallback'
go test ./core/workspace -run 'TestMustMigrateToWorkspaceConfig|TestParse.*Compat'
I also tried the named integration subtest directly, but the host helper used the stable /home/exedev/.local/bin/dagger, not a CLI from this checkout, so that run was not a valid branch verification. I tried a dev playground check too; it failed during playground setup before the inner command ran.
What are your thoughts on this message 👆
<@&1506565370385793125> here's my problem with removing the guard in this case. It may be safe in this particular test fixture because it's a single vanilla module at the root of an otherwise empty workspace. So there is nothing to migrate; and therefore it's safe to just install into a new workspace config.
BUT, you're not proposing that we check that "there's nothing to migrate". You propose we check that "this particular module doesn't need to be migrated". But what if there is stuff to migrate, elsewhere in the workspace? Then our innocent little install basically short-circuits the whole migration -> drift, huge fail. So, the guard is actually acting as intended I think.
I think that objection is valid. My local guard change is too narrow as stated: compat != nil && compat.MustMigrateToWorkspaceConfig() only answers “does the selected/current legacy config need migration?”, not “would creating .dagger/config.toml hide or strand other migration-required dagger.json files in this workspace?”
So I would not ship that as-is. The safer rule is closer to: allow init/install for a plain SDK runtime-compat module only if the workspace-level migration scan says there is nothing migration-required anywhere that this new config would preempt. If there is any blueprint/toolchain/non-dot-source legacy config elsewhere under the workspace, keep the mutation blocked and tell the user to migrate first.
I’ll revise my local patch accordingly or back it out if there isn’t already a cheap whole-workspace migration predicate to reuse.
I backed out my local guard-narrowing patch. Worktree is clean again.
I think the right fix shape is not “allow when current compat doesn’t require migration”; it’s “allow only when a workspace-wide migration scan has no migration-required legacy configs that would be masked by creating .dagger/config.toml.” The existing migration path already has workspaceMigrationCompatWorkspaces and then filters MustMigrateToWorkspaceConfig(), so that is the predicate family I’d try to reuse or factor into a cheap pre-mutation check.
I wouldn’t try to track real .git directories in the repo. Git treats nested .git as repository metadata / embedded repo state, so it’s awkward, brittle, and not something we want fixtures depending on.
More realistic options:
- initialize git at test runtime (
git init, config user, maybe an initial commit) in the temp fixture/workdir; this is what a lot of existing Dagger tests already do. - adjust the fixture helper so copied legacy fixtures get a
.gitinitialized automatically when they exercise workspace detection. - only if the no-git legacy case is a behavior we intentionally still support, fix detection to handle compat fallback without
.git; but from Solomon’s note that sounds like more of a test compatibility issue than a real-user priority.
So I’d prefer runtime git init in the fixture setup over trying to commit .git.
I’m here. I replied above but the short version is: I agree with your objection. My guard-narrowing patch was too weak because it only checked the current compat config, not whether the whole workspace has anything migration-required that a new .dagger/config.toml would mask. I backed that local patch out; the worktree is clean again.
The fix, if we want one, needs a workspace-wide pre-mutation predicate: scan the workspace for runtime compat configs, and only allow install/init to create config when none of them require migration. Otherwise the current guard behavior is the safer default.
Agreed. That’s the right safety concern. Checking only currentCompat.MustMigrateToWorkspaceConfig() is not enough because creating a root .dagger/config.toml can mask migration-required legacy configs elsewhere in the workspace.
I backed out my local guard-narrowing patch. If this behavior changes, the condition needs to be workspace-wide: scan for all runtime compat dagger.json files that would be hidden by the new config, and only allow the mutation if none require migration. Until then, the existing guard is the safer behavior.
How hard would it be to implement that predicate in a reliable way? We don't want glitches or weirdness in this critical part of the user flow (painful migration = possible churn)
I’d call it moderate, not hard, but it should be treated as product-critical code rather than a quick guard tweak. The reliable version should reuse/factor the migration scanner, not invent a second interpretation of legacy configs.
Shape I’d trust:
- Compute the config location the mutation would create (
.dagger/config.toml, or--here). - Scan the workspace with the same rules as
dagger migrate: finddagger.json, skip.dagger/hidden paths, skip subtrees already owned by an existing config, parse withParseRuntimeCompatWorkspaceAt. - Consider only legacy configs that would become shadowed by the new config.
- If any shadowed compat workspace has
MustMigrateToWorkspaceConfig() == true, block and tell the user to run migrate. - If all shadowed compat configs are plain SDK-only, allow install/init to create config.
The main risk is drift between this predicate and migrate. So I’d factor the existing workspaceMigrationCompatWorkspaces/filtering into shared helpers and add integration coverage for: plain root only allows install; root plain plus nested toolchain blocks; nested existing .dagger/config.toml is not considered; no-git legacy test behavior is intentional/covered.
Effort feels like half-day to implement, another half-day to test properly. I would not ship it without the mixed-tree tests.
tldr please
TL;DR: doable, but don’t do the simple guard change. Reliable fix = reuse the migration scanner and only allow install/init if creating .dagger/config.toml would not hide any migration-required dagger.json anywhere under that workspace. Estimate: ~1 day with mixed-tree integration tests. Until then, keep the current guard.
But even if there's just the one naked sdk-only dagger.json in the whole workspace... Even that requires installing a SDK module no? So there will be drift no matter what
Yes, I think that’s right. I was still undercounting the plain SDK-only case. Even if it is the only dagger.json, migration is not a no-op: it creates the parent workspace config and installs the SDK module / report semantics for that standalone module.
So just letting dagger install ./dep create a fresh config with only dep would drift from what dagger migrate would have produced. That means a predicate alone is probably the wrong shape unless it also proves “migration plan is empty,” not just “no workspace-like configs.” For this case the plan is not empty.
Safer conclusion: keep the guard. If we want install to work here, it should probably run or compose the relevant migration plan first, not silently initialize a different config.
With that in mind: what do we do about this test? What is the cleanest, most useful, and most self-evident test (or tests) we can have here?
I’d split the current test into tests that each say one thing clearly.
-
Keep the “install initializes workspace config” test, but make the fixture an empty git workspace with only
./depas the module. No rootdagger.json. Expected: creates.dagger/config.toml, installs dep. This proves the init-on-install behavior without legacy ambiguity. -
Replace the current standalone-root-module expectation with the opposite: plain SDK root
dagger.json+dagger install ./depshould fail with the migration guard. Expected message: rundagger migratefirst. This documents why the guard exists, even for SDK-only modules. -
Add/keep a happy path for the intended flow: plain SDK root
dagger.json->dagger migrate -y->dagger install ./dep. Expected: rootdagger.jsonremains,.dagger/config.tomlexists, SDK module from migration is present, and dep is installed. This is the useful user flow test.
Optionally, if we still want “install never edits dagger.json,” test it in an already-initialized workspace: root dagger.json plus existing .dagger/config.toml, then dagger install ./dep should modify only config. That avoids implying install is allowed to initialize over legacy compat.
For each of those tests: which test file do they belong in? It's important to me that each test file have a clear scope, well documented in its header, and we carefully design the whole set of test files and how they relate to each other.
I’d place them by the behavior they’re protecting, not by the command name alone.
-
Empty git workspace +
dagger install ./depinitializes.dagger/config.toml:core/integration/workspace_modules_test.go. That file’s scope is workspace module install/list/name/config behavior for native workspaces. This is the clean native install-init case. -
Plain SDK root
dagger.json+dagger install ./depis blocked by migration guard:core/integration/workspace_compat_test.go. The scope is legacydagger.jsoncompat detection, warnings, boundaries, and errors. This test documents that SDK-only runtime compat is still compat enough to block workspace mutation until migration. -
Plain SDK root
dagger.json->dagger migrate -y->dagger install ./depsucceeds:core/integration/workspace_migration_test.go. The scope should be explicit migration plans/application and post-migration handoff. This is not mainly an install test; it proves migration produces the workspace state install is allowed to mutate. -
Already-initialized workspace with root
dagger.json, then install leavesdagger.jsonalone:core/integration/workspace_modules_test.go. Here the compat question is gone because.dagger/config.tomlalready owns the workspace; it’s pure workspace install mutation behavior.
I would avoid putting migration-first flows in workspace_modules_test.go; that makes the install suite carry migration semantics. Instead, let workspace_modules_test.go prove native install behavior, workspace_compat_test.go prove legacy compat blocking, and workspace_migration_test.go prove the bridge from legacy to native.
I’d split by ownership of behavior, not by fixture shape:
-
workspace_modules_test.go: native workspace install behavior only. Keep/rename the empty-workspace case here: no.dagger/config.toml, no rootdagger.json,dagger install ./depcreates.dagger/config.tomland records the dep. This file’s scope should be: workspace module commands once the workspace config model is authoritative, including creating a new native workspace from an actually empty repo. -
workspace_compat_test.go: legacy compat boundary behavior. Move/replace the naked SDK-onlydagger.jsoninstall case here, and assert it is blocked with the migrate-first error. The point of the test is not module install mechanics; it is that compatdagger.jsonfallback must not be silently masked by creating a new workspace config. -
workspace_migration_test.go: explicit migration handoff. Add/keep a case where the same naked SDK-onlydagger.jsonrunsdagger migrate -y, thendagger install ./depsucceeds against the migrated.dagger/config.toml. This proves the intended user path and makes the drift concern self-evident. -
Optional in
workspace_modules_test.go: already-native workspace plus stale/irrelevantdagger.jsonstays native, and install mutates.dagger/config.tomlonly. This belongs in modules because the guard should only matter before native config exists; after that, workspace config owns the flow.
So I would delete or invert the current workspace_install_creates_workspace_config_instead_of_editing_dagger.json_for_standalone_modules expectation. In the new model, that test is teaching the wrong rule.
Give us a raw (quoted snippet) padded table
| Test / Scenario | File | Scope / Assertion |
|------------------------------------------------------|-----------------------------------|-----------------------------------------------------------------------------------|
| empty repo: install creates native workspace config | core/integration/workspace_modules_test.go | Native workspace module commands; `install` may initialize `.dagger/config.toml`. |
| naked SDK-only dagger.json: install is blocked | core/integration/workspace_compat_test.go | Compat boundary; do not mask legacy/compat config by creating native config. |
| naked SDK-only dagger.json: migrate then install | core/integration/workspace_migration_test.go | Migration handoff; explicit migrate creates the native state install can mutate. |
| existing native workspace + stale dagger.json | core/integration/workspace_modules_test.go | Native config is authoritative; install mutates `.dagger/config.toml` only. |
renders wrong. filename only; less words in other columns
| Case | File | Assert |
|------------------------------|------------------------------|---------------------------|
| empty repo + install | workspace_modules_test.go | creates config |
| sdk dagger.json + install | workspace_compat_test.go | blocks, says migrate |
| sdk dagger.json + migrate | workspace_migration_test.go | creates native config |
| migrated config + install | workspace_modules_test.go | edits config, not json |
workspace_modules_test.go
Is there any migration-related test in there today?
No explicit migration test. workspace_modules_test.go only has two dagger.json-related cases today:
| Case | Migration? | Notes |
|------------------------------------------------|------------|----------------------------------------|
| install creates config instead of editing json | no | current failing/now-wrong expectation |
| configured workspace ignores cwd dagger.json | no | native config precedence |
So I’d avoid adding a migrate-before-install case there. That belongs in workspace_migration_test.go. The only dagger.json case I’d keep in workspace_modules_test.go is the native-config precedence one, because its subject is still “workspace config owns module commands.”
I’m here. I answered just above: no explicit migration test exists in workspace_modules_test.go today. It has two dagger.json-related cases, but both are module/config precedence cases, not migration cases. My recommendation is still: migration path tests go in workspace_migration_test.go; compat block/error tests go in workspace_compat_test.go; only native workspace install behavior stays in workspace_modules_test.go.
I believe the current UX of erroring is correct: the blocking behavior is correct, and the test expectation is what became wrong.
The rule is (proposal):
- SDK-only dagger.json is still legacy compat state.
- Legacy compat state should be runnable and migratable.
- It should not be silently overwritten by workspace mutations.
So UX should be:
# empty repo, no dagger.json
dagger install ./dep
# OK: creates .dagger/config.toml
# repo has SDK-only dagger.json, no .dagger/config.toml
dagger install ./dep
# ERROR: run dagger migrate first
dagger migrate -y
dagger install ./dep
# OK: now native config exists, mutate it
<@&1506565370385793125> show latest proposed table of tests & their test file
| Case | File | Expect |
|------------------------------|------------------------------|---------------------------|
| empty repo + install | workspace_modules_test.go | creates config |
| sdk dagger.json + install | workspace_compat_test.go | errors: migrate first |
| sdk dagger.json + migrate | workspace_migration_test.go | creates native config |
| migrated config + install | workspace_modules_test.go | edits config |
| native config + dagger.json | workspace_modules_test.go | ignores dagger.json |
Also add tests to remove to the table
| Action | Case | File | Expect |
|---------|------------------------------------------------|------------------------------|-----------------------|
| keep | empty repo + install | workspace_modules_test.go | creates config |
| add | sdk dagger.json + install | workspace_compat_test.go | errors: migrate first |
| add | sdk dagger.json + migrate | workspace_migration_test.go | creates config |
| add | migrated config + install | workspace_modules_test.go | edits config |
| keep | native config + dagger.json | workspace_modules_test.go | ignores dagger.json |
| remove | install creates config instead of editing json | workspace_modules_test.go | wrong rule |
| Action | Case | File | Expect |
| keep | empty repo + install | workspace_modules_test.go | creates config |
| Action | Name | File | Assertion |
| = | TestFooBar | workspace_module_test.go | 'dagger install' in empty repo creates config |
--> now do the full table
| Action | Name | File | Assertion |
|--------|------------------------------------------------------------------------|-----------------------------|------------------------------------------------|
| keep | workspace_install_initializes_a_workspace_when_needed | workspace_modules_test.go | 'dagger install' in empty repo creates config |
| add | workspace_install_rejects_sdk_only_dagger_json | workspace_compat_test.go | 'dagger install' with sdk json says migrate |
| add | migrate_sdk_only_dagger_json_then_install | workspace_migration_test.go | migrate creates config; install mutates config |
| keep | configured_workspace_ignores_cwd_dagger_json | workspace_modules_test.go | native config wins over cwd json |
| remove | workspace_install_creates_workspace_config_instead_of_editing_dagger_json | workspace_modules_test.go | wrong rule: masks sdk json |
can the test names be made shorter and more readable without losing crucial info?
Yes. I’d use subtest names like this:
| Action | Name | File | Assertion |
|--------|-------------------------------------------|-----------------------------|------------------------------------------------|
| keep | install initializes empty workspace | workspace_modules_test.go | 'dagger install' in empty repo creates config |
| add | install rejects sdk-only legacy config | workspace_compat_test.go | sdk-only dagger.json says migrate first |
| add | migrate sdk-only config then install | workspace_migration_test.go | migrate creates config; install edits it |
| keep | native workspace ignores cwd dagger.json | workspace_modules_test.go | native config wins over cwd json |
| remove | install creates config instead of editing json | workspace_modules_test.go | wrong rule: masks sdk json |
all subtests? not top-level in suite?
Good catch. No, not all subtests. I’d use top-level suite methods for the behavior groups, then short subtests only for variants inside them.
| Action | Name | File | Assertion |
|--------|--------------------------------------------------------|-----------------------------|------------------------------------------------|
| keep | TestWorkspaceModuleInstall / initializes empty workspace | workspace_modules_test.go | 'dagger install' in empty repo creates config |
| add | TestWorkspaceCompatMutationGuards / rejects sdk-only install | workspace_compat_test.go | sdk-only dagger.json says migrate first |
| add | TestWorkspaceMigrateOutcomes / sdk-only then install | workspace_migration_test.go | migrate creates config; install edits it |
| keep | TestWorkspaceModuleInstall / ignores cwd dagger.json | workspace_modules_test.go | native config wins over cwd json |
| remove | TestWorkspaceModuleInstall / creates config instead of editing json | workspace_modules_test.go | wrong rule: masks sdk json |
So: install mechanics stay under TestWorkspaceModuleInstall; migration path stays under existing TestWorkspaceMigrateOutcomes; the missing top-level bucket is probably TestWorkspaceCompatMutationGuards because the new error case is about protecting compat state from workspace mutations.
<@&1506565370385793125> can you make this change in a local branch? Then when you're done figure out a way to serve the branch on your VM so I can pull it. And give me the pull command
I’ll make the test-only change on a local branch, run the focused tests I can, then expose the branch from this VM and post the exact pull command.
cc @fossil fulcrum 👆
<@&1506565370385793125> very important that each test you touch have a crystal clear comment.
Crystal clear means:
- It's immediately obvious what the assertion is
- A maintainer can read the comment and have all the context it needs for "what" and "why" (no confusion possible on why we're testing)
- The test implementation and comment work together to illuminate rather than confuse
I'm lost now
<@&1506565370385793125> give Guillaume the TLDR of what you're doing and why
@fossil fulcrum 👆
Done on branch codex/workspace-compat-install-tests at 9b57ce8d1.
Pull command:
git pull http://dx-32fce255c85380bfeaad.exe.xyz:3456/dagger-branch.git codex/workspace-compat-install-tests
What changed:
| Action | Name | File | Assertion |
|--------|---------------------------------------------|-----------------------------|-----------------------------------------------|
| keep | install initializes empty workspace | workspace_modules_test.go | empty repo install creates config |
| add | install rejects sdk-only legacy config | workspace_compat_test.go | sdk-only dagger.json says migrate first |
| add | sdk-only config migrates before install | workspace_migration_test.go | migrate creates config; install edits it |
| keep | native workspace ignores cwd dagger.json | workspace_modules_test.go | native config wins over cwd json |
| remove | creates config instead of editing json | workspace_modules_test.go | removed stale/wrong expectation |
Verification: go test ./core/integration -run '^$' passes. I also tried focused integration tests with a branch-built CLI; host-side tests timed out in local engine setup, and the migration path hit Docker Hub unauthenticated rate limiting earlier, so I don’t have a clean focused integration pass from this VM.
Agreed. I’ll amend the branch with explicit comments on each touched test/subtest, then refresh the served branch so the same pull command gets the updated commit.
Amended with comments at each touched test/subtest. New served commit is dd7176ad9.
Same pull command if you have not pulled yet:
git pull http://dx-32fce255c85380bfeaad.exe.xyz:3456/dagger-branch.git codex/workspace-compat-install-tests
If you already pulled the previous commit, use this to reset your local branch to the amended version:
git fetch http://dx-32fce255c85380bfeaad.exe.xyz:3456/dagger-branch.git codex/workspace-compat-install-tests && git switch -C codex/workspace-compat-install-tests FETCH_HEAD
I reran go test ./core/integration -run '^$' after the amend; it passes.
<@&1506565370385793125> dagger-branch.git weird repo name?
TLDR: I made a test-only branch that encodes the rule you described.
| Change | File | Meaning |
|--------|-----------------------------|----------------------------------------------|
| remove | workspace_modules_test.go | old test expected install to bypass migrate |
| add | workspace_compat_test.go | sdk-only dagger.json + install must error |
| add | workspace_migration_test.go | migrate first, then install is allowed |
No product behavior change: the existing guard stays. The test expectation changed from “standalone SDK dagger.json can initialize workspace config” to “SDK-only dagger.json is compat state; run dagger migrate first.”
Branch is served here:
git fetch http://dx-32fce255c85380bfeaad.exe.xyz:3456/dagger-branch.git codex/workspace-compat-install-tests && git switch -C codex/workspace-compat-install-tests FETCH_HEAD
test with dagger call engine-dev test --pkg ./core/integration --run '^TestWorkspaceSelection/TestSelectedWorkspaceFileIO$' --test-verbose and replace with the test suite and corresponding test and subtest
Guillaume: the branch is test-only. It keeps your UX rule intact:
empty repo + dagger install ./dep => OK, create .dagger/config.toml
sdk-only dagger.json + dagger install ./dep => ERROR, run dagger migrate first
sdk-only dagger.json + migrate + dagger install ./dep => OK, mutate migrated config
Why: the old workspace_modules_test.go subtest expected dagger install to create .dagger/config.toml next to an SDK-only dagger.json. After the compat broadening, that expectation is wrong because SDK-only dagger.json is still legacy compat state. Creating native config there would bypass migration and allow drift.
What I changed:
| Action | File | Purpose |
|--------|-----------------------------|----------------------------------------------|
| remove | workspace_modules_test.go | remove stale bypass-migration expectation |
| add | workspace_compat_test.go | assert install is blocked on sdk-only json |
| add | workspace_migration_test.go | assert migrate first, then install works |
I also added explicit comments on each touched test so the what/why is in the code, not just this thread.
That quoted message is the core rationale the branch now encodes: SDK-only dagger.json migration is not empty, so install must not invent a different native config. The tests now pin that as: install blocks before migration, migrate creates the native config, install succeeds after migration.
Yeah, that was a throwaway bare-repo name. I added a cleaner alias at dagger.git; use this instead:
git fetch http://dx-32fce255c85380bfeaad.exe.xyz:3456/dagger.git codex/workspace-compat-install-tests && git switch -C codex/workspace-compat-install-tests FETCH_HEAD
Verified it resolves to dd7176ad9.
I’ll run the focused engine-dev command with the exact suite/test/subtest paths for the tests touched by this branch, then report the result and update the branch if anything shakes out.
<@&1506565370385793125> are you using dagger cloud?
cannot access it
Engine-dev run attempted; it was blocked before Go tests by Docker Hub 429 resolving golang:1.26-alpine. I’ll post full command/result when relay is reachable.
I ran the engine-dev form with the touched suite/test/subtest paths:
dagger call engine-dev test --pkg ./core/integration --run '^TestWorkspaceModules/TestWorkspaceModuleInstall/install_initializes_empty_workspace$|^TestWorkspaceModules/TestWorkspaceManagedModuleBehavior/native_workspace_ignores_cwd_dagger\.json$|^TestWorkspaceCompat/TestWorkspaceCompatMutationGuards/install_rejects_sdk-only_legacy_config$|^TestWorkspaceMigration/TestWorkspaceMigrateOutcomes/sdk-only_config_migrates_before_install$' --test-verbose
It did not reach the Go tests. Engine-dev setup failed resolving docker.io/library/golang:1.26-alpine with Docker Hub 429 Too Many Requests unauthenticated pull limit. Trace: https://dagger.cloud/dagger/traces/0077ea0cdc62a85175363e1e00aa6233
So current verification is still: compile-only go test ./core/integration -run '^$' passes; focused engine-dev is blocked by registry rate limiting in this VM.
Not intentionally. I ran the local dagger call engine-dev test ... command from the VM. The CLI emitted a Dagger Cloud trace URL, but I don’t see Dagger Cloud auth/env configured here, and the engine it started was local Docker: registry.dagger.io/engine:v0.20.8.
So: local engine execution, with trace URL emitted by the CLI; not a remote Dagger Cloud runner/cache setup.
Looks like the exe.dev port is not reachable from your side; I can reach it from inside the VM, but that does not help you. I uploaded the patch instead:
curl -fsSL https://paste.rs/NJ14Y | git am
That applies commit dd7176ad9 (test: cover compat install migration guard) on top of your current branch. If you want a real branch remote instead, I’ll need somewhere I’m allowed to push; this VM’s ad-hoc HTTP port seems private/inaccessible externally.
@fossil fulcrum do you want to give the other tests you mentioned, of the same category, do <@&1506565370385793125> here, and you can focus on the next batch? When it's done you can have your agent pull the branch and review/incorporate it
Yes, send me the same-category cases and I’ll add them on top of the current branch, with the same constraints: test-only unless the case exposes a real product bug, clear file ownership, and explicit comments for what/why. Since the VM branch port was not reachable externally, I’ll keep publishing patch URLs unless someone gives me a remote I can push to.
Yes it was these ones:
TestWorkspaceCompat/TestCompatDetection/sdk-only_child_config_is_found_from_cwd
and
TestWorkspaceCompat/TestCompatDetection/sdk-only_root_source_creates_a_compat_workspace
AND TestWorkspaceModules/TestWorkspaceManagedModuleBehavior/unconfigured_workspace_does_not_infer_module_from_cwd
Got it. I’ll add crystal-clear comments to those three existing tests on the same branch, rerun compile-only verification, then publish a fresh patch URL.
<@&1506565370385793125> start by updating the table - which tests to add or remove. in which test file, and what is the assertion. Then from there -> edit the code
Added clear comments to those three existing tests too, and amended the branch. New commit is 4b49c34fb.
Patch URL:
curl -fsSL https://paste.rs/oejyN | git am
What was added:
| Test | Comment now explains |
|-----------------------------------------------------------------------|-----------------------------------------------------------|
| TestCompatDetection / sdk-only root source creates a compat workspace | root sdk-only dagger.json is ambient compat state |
| TestCompatDetection / sdk-only child config is found from cwd | cwd-based detection finds child sdk-only compat config |
| TestWorkspaceManagedModuleBehavior / unconfigured workspace... | native workspace does not infer cwd dagger.json as module |
Verification after amend: go test ./core/integration -run '^$' passes.
Updated table before code review/editing:
| Action | Name | File | Assertion |
|----------|-------------------------------------------|-----------------------------|------------------------------------------------|
| keep | install initializes empty workspace | workspace_modules_test.go | empty repo install creates config |
| remove | creates config instead of editing json | workspace_modules_test.go | wrong: sdk-only json bypasses migration |
| add | install rejects sdk-only legacy config | workspace_compat_test.go | sdk-only dagger.json says migrate first |
| document | sdk-only root source creates compat workspace | workspace_compat_test.go | root sdk-only json is ambient compat state |
| document | sdk-only child config is found from cwd | workspace_compat_test.go | cwd detection finds child sdk-only json |
| extend | sdk-only config migrates before install | workspace_migration_test.go | migrate creates config; install edits it |
| document | native workspace ignores cwd dagger.json | workspace_modules_test.go | native config wins over cwd json |
| document | unconfigured workspace does not infer cwd | workspace_modules_test.go | no entrypoint means no cwd json fallback |
Current branch commit 4b49c34fb is edited to match this table: add/remove/extend rows are code changes, document rows are comments clarifying existing assertions. I’ll treat this table as the contract for any follow-up edits.
if it's helpful, I got the legacy compatibility mode working without requiring a .git repo under https://github.com/alexcb/dagger/commit/1897aa81e2b54c4e299b5108df10e6598588bdcb
Im guessing that's a no brainer, that fixes a bunch of tests no?
I think so, it's not completely obvious if it breaks anything else due to the current regression.
<@&1506565370385793125> this is ready for review? You addressed all the extra broken tests that Guillaume shared?
Yes, ready for review for the test-table scope. I addressed all three extra tests Guillaume named:
| Test | Status |
|-----------------------------------------------------------------------|------------------------------|
| TestCompatDetection / sdk-only child config is found from cwd | documented in code |
| TestCompatDetection / sdk-only root source creates compat workspace | documented in code |
| TestWorkspaceManagedModuleBehavior / unconfigured workspace... | documented in code |
The add/remove/extend behavior changes are also in the same commit. Current patch is 4b49c34fb:
curl -fsSL https://paste.rs/oejyN | git am
Caveat: this does not include Alex’s separate no-.git compat fix. That is a different product-code change and should be reviewed/tested separately or stacked deliberately.
<@&1506565370385793125> try running the tests yourself.
dagger --progress=logs call engine-dev test --pkg=./core/integration --run=TestWhatever
:diagnostics
Diagnostics:
- VM:
dx-32fce255c85380bfeaad - GitHub App auth: configured
false, token mintfalse - Dagger auth: config files
true, env tokenfalse - Dagger Cloud engine env:
true - Dagger in sandbox: cli
false, configtrue - Sandbox: reachable
true, session servertrue, version14 - Codex session: exists
true, busytrue, codex process runningtrue, queued0 - Recent failed events:
0
I’ll run the focused engine-dev tests with --progress=logs now using the exact touched test paths. I’ll report the command and actual result, including if the VM still hits registry limits before tests start.
any progress?
:diagnostics
:diagnostics
Diagnostics:
- VM:
dx-32fce255c85380bfeaad - GitHub App auth: configured
false, token mintfalse - Dagger auth: config files
true, env tokenfalse - Dagger Cloud engine env:
true - Dagger in sandbox: cli
false, configtrue - Sandbox: reachable
true, session servertrue, version15 - Codex session: exists
true, busyfalse, codex process runningfalse, queued0 - Recent failed events:
0
<@&1506565370385793125> looks like we lost you. how are those tests going?
I’m here. The focused engine-dev run passed after I fixed the stale assertions it exposed.
Command that passed:
dagger --progress=logs call engine-dev test --pkg=./core/integration --run='^TestWorkspaceModules/TestWorkspaceModuleInstall/install_initializes_empty_workspace$|^TestWorkspaceModules/TestWorkspaceManagedModuleBehavior/native_workspace_ignores_cwd_dagger\.json$|^TestWorkspaceCompat/TestWorkspaceCompatMutationGuards/install_rejects_sdk-only_legacy_config$|^TestWorkspaceCompat/TestCompatDetection/sdk-only_root_source_creates_a_compat_workspace$|^TestWorkspaceCompat/TestCompatDetection/sdk-only_child_config_is_found_from_cwd$|^TestWorkspaceMigration/TestWorkspaceMigrateOutcomes/sdk-only_config_migrates_before_install$'
Result: ok github.com/dagger/dagger/core/integration 16.176s.
I updated the patch to eaacdbb7c:
curl -fsSL https://paste.rs/QB9RM | git am
Important delta from the run: the unconfigured workspace does not infer cwd test was stale and removed; that behavior is now covered in compat detection instead.
What's the git remote for you?
origin is https://github.com/dagger/dagger.git for both fetch and push.
I mean how do I pull your branch on the git server you setup on your vm
Use this remote/branch:
git fetch http://dx-32fce255c85380bfeaad.exe.xyz:3456/dagger.git codex/workspace-compat-install-tests
git switch -C codex/workspace-compat-install-tests FETCH_HEAD
One-liner:
git fetch http://dx-32fce255c85380bfeaad.exe.xyz:3456/dagger.git codex/workspace-compat-install-tests && git switch -C codex/workspace-compat-install-tests FETCH_HEAD
It currently resolves on my VM to eaacdbb7c. If that URL still is not reachable from your network, patch fallback is:
curl -fsSL https://paste.rs/QB9RM | git am
fatal: unable to access 'http://dx-32fce255c85380bfeaad.exe.xyz:3456/dagger.git/': The requested URL returned error: 400
That 400 looks like the exe.dev proxy path is not usable for unauthenticated git fetch from your side. It works from inside my VM, but not externally enough to rely on.
Use this git bundle instead; it preserves the branch/ref, unlike the patch fallback:
curl -fsSL https://paste.rs/ePepW -o /tmp/codex-workspace-compat-install-tests.bundle
git fetch /tmp/codex-workspace-compat-install-tests.bundle codex/workspace-compat-install-tests
git switch -C codex/workspace-compat-install-tests FETCH_HEAD
Bundle verifies to eaacdbb7c for refs/heads/codex/workspace-compat-install-tests. Patch fallback still works too:
curl -fsSL https://paste.rs/QB9RM | git am
Mmm no thank you. I want to be able to pull directly form you. Figure out a way
Understood. I’m going to make the VM-hosted git remote actually pullable, not use a bundle/patch workaround. I’ll verify the URL shape against exe.dev’s documented proxy behavior and post only after I have a command that should work externally.
Got it. I set up a smart Git HTTP endpoint behind a reverse tunnel and verified an actual git fetch from it into an existing Dagger checkout. Use this:
git fetch https://82976c0ab525d7fb-162-43-189-115.serveousercontent.com/cgi-bin/git-http-backend/dagger.git codex/workspace-compat-install-tests
git switch -C codex/workspace-compat-install-tests FETCH_HEAD
One-liner:
git fetch https://82976c0ab525d7fb-162-43-189-115.serveousercontent.com/cgi-bin/git-http-backend/dagger.git codex/workspace-compat-install-tests && git switch -C codex/workspace-compat-install-tests FETCH_HEAD
I verified the remote advertises eaacdbb7cffbcc4af22f686206eb36b87fc85a3c and a real fetch completed locally through that URL.
omg what the hell is that 😂
It is ugly, agreed. The exe.dev URL failed because the VM HTTPS proxy is private/auth-gated for git, and this VM does not have an exe.dev SSH identity available for me to run share set-public or mint a VM token.
So the working direct-pull setup is: Git smart HTTP in the VM -> temporary public reverse tunnel -> your git fetch. The long /cgi-bin/git-http-backend/... path is just Git’s smart HTTP backend, not a bundle/patch.
Cleaner options if you want them:
# current working ugly direct pull
git fetch https://82976c0ab525d7fb-162-43-189-115.serveousercontent.com/cgi-bin/git-http-backend/dagger.git codex/workspace-compat-install-tests
Or someone with exe.dev auth can make this VM proxy public / give me a VM token, and then I can serve the same repo at the nicer https://dx-...exe.xyz/dagger.git shape.
even more fast and furious than @nimble flicker 😂
<@&1506565370385793125> change of plans, open a draft PR against the workspace branch
prefix your branch with codex/ on origin
Acknowledged. I’ll try to push the local branch to GitHub and open a draft PR against the workspace branch. First I’m checking auth/remotes and the exact base branch name.
I’m blocked at GitHub permissions, not git mechanics. The VM has gh auth as dagger-codex[bot], but dagger/dagger rejects branch creation for that token:
GH013: Repository rule violations
Cannot create ref due to creations being restricted
I also tried creating a fork and that token gets 403 Resource not accessible by integration, so I cannot put the head branch anywhere GitHub will accept for a PR.
Fastest path: someone with GitHub branch-create rights can push my branch, then I can open the draft PR against workspace immediately. The branch is still served here:
git fetch https://82976c0ab525d7fb-162-43-189-115.serveousercontent.com/cgi-bin/git-http-backend/dagger.git codex/workspace-compat-install-tests
git push origin FETCH_HEAD:refs/heads/codex/workspace-compat-install-tests
Once that branch exists on dagger/dagger, I can run the gh pr create --draft --base workspace ... step from here.
I retried exactly that: refs/heads/codex/workspace-compat-install-tests on origin. GitHub still rejects branch creation for this token:
GH013: Repository rule violations
Cannot create ref due to creations being restricted
So the branch name is already right; the dagger-codex[bot] token just is not in the bypass set for creating branches on dagger/dagger.
try again
Done: draft PR opened against workspace: https://github.com/dagger/dagger/pull/13195
Head branch: codex/workspace-compat-install-tests
Head commit: 53ecc1d7eee8
I verified the PR is open and draft.
Summary
treat SDK-only dagger.json as legacy compat state for install mutation tests
document compat detection expectations for SDK-only root and child configs
extend migration coverage so SDK-onl...
<@&1506565370385793125> workspace was rebased. time to rebase in turn
On it. I’ll fetch the rebased workspace, rebase codex/workspace-compat-install-tests, rerun the quick check, and force-push the PR branch if it’s clean.
Rebased and force-pushed PR #13195 onto the new origin/workspace.
Current PR head: 213918c5e7f5
Diff is still only:
core/integration/workspace_compat_test.go
core/integration/workspace_migration_test.go
core/integration/workspace_modules_test.go
Verification note: go test ./core/integration -run '^$' fails, but the same failure reproduces on a clean origin/workspace worktree with no PR changes. Current base compile error is in core/workspace.go (Workspace missing Path, Initialized, ConfigPath, HasConfig).
<@&1506565370385793125> you've confirmed that all the tests you touch now pass? (use dagger call engine-dev test ...)
Not yet after the latest workspace rebase. I had confirmed them before the rebase; now I’m running the focused dagger call engine-dev test suite against the current PR head and will report the actual result.
Is it executing in dagger cloud?
No. I ran the focused dagger --progress=logs call engine-dev test ... --test-verbose after the rebase, and it did not reach the touched tests. It fails while building cmd/engine with the same base compile error:
core/workspace.go:111:19: ws.Path undefined
core/workspace.go:113:19: ws.Initialized undefined
core/workspace.go:114:19: ws.ConfigPath undefined
core/workspace.go:115:19: ws.HasConfig undefined
core/workspace.go:157:3: unknown field Path in struct literal of type Workspace
...
Trace: https://dagger.cloud/dagger/traces/1484d6eb25168f4bb6af420d760fbc89
I also reproduced the same compile failure on a clean origin/workspace worktree with no PR changes, so I can’t honestly claim the touched tests pass on the current base yet.
No, not as remote execution. It executed on the VM/local Dagger engine; Dagger Cloud only received the trace/telemetry.
The CLI printed:
connected name=4cf0dbc9d49c client-version=v0.21.0-... server-version=v0.21.0-...
Full trace at https://dagger.cloud/dagger/traces/1484d6eb25168f4bb6af420d760fbc89
So the trace is in Dagger Cloud, but the build/test work ran against the engine in this exe.dev VM.
:diagnostics
Diagnostics:
- VM:
dx-32fce255c85380bfeaad - GitHub App auth: configured
true, token minttrue - Dagger auth: config files
true, env tokenfalse - Dagger Cloud engine env:
true - Dagger in sandbox: cli
false, configtrue - Sandbox: reachable
true, session servertrue, version16 - Codex session: exists
true, busyfalse, codex process runningfalse, queued0 - Recent failed events:
0
isn't DAGGER_CLOUD_ENGINE exported to 1 in your shell environment?
Yes. I just checked the shell env and DAGGER_CLOUD_ENGINE=1 is exported. My previous answer was wrong: with that env, the dagger call engine-dev test ... run was using Dagger Cloud Engine, not just local VM execution with cloud tracing.
So the accurate status is: the focused test run executed via Dagger Cloud Engine and failed before running the touched tests because cmd/engine does not compile on the current rebased workspace base.
Oh, maybe it doesn't build on the main build of dagger? Try installing latest stable release (at a separate path) and see if that builds it?
Tried it. I installed latest stable separately at /home/exedev/dagger-stable-bin/dagger; it is v0.20.8 and connected to a v0.20.8 engine:
/home/exedev/dagger-stable-bin/dagger version
dagger v0.20.8 (image://registry.dagger.io/engine:v0.20.8) linux/amd64
connected name=233c68501901 client-version=v0.20.8 server-version=v0.20.8
Then I reran the same focused engine-dev test. It still fails while building this repo’s cmd/engine at PR commit 213918c5e7f5, with the same core/workspace.go missing-field compile errors. So this is not specific to the dev/main Dagger CLI/engine; stable can drive the run, but the workspace branch source currently does not compile at that point.
<@&1506565370385793125> try again? @fossil fulcrum pushed a fix
Trying again now. I’ll fetch the updated workspace, rebase the PR branch if needed, push it, then rerun the focused engine-dev test with the stable CLI path.
Retried after Guillaume’s fix. I fetched/rebased onto updated origin/workspace, force-pushed PR #13195, and reran the focused tests with stable Dagger v0.20.8.
Current PR head: 8c0f17296ee7
Command passed with exit 0:
/home/exedev/dagger-stable-bin/dagger --progress=logs call engine-dev test --pkg=./core/integration --run='^TestWorkspaceModules/TestWorkspaceModuleInstall/install_initializes_empty_workspace$|^TestWorkspaceModules/TestWorkspaceManagedModuleBehavior/native_workspace_ignores_cwd_dagger\.json$|^TestWorkspaceCompat/TestWorkspaceCompatMutationGuards/install_rejects_sdk-only_legacy_config$|^TestWorkspaceCompat/TestCompatDetection/sdk-only_root_source_creates_a_compat_workspace$|^TestWorkspaceCompat/TestCompatDetection/sdk-only_child_config_is_found_from_cwd$|^TestWorkspaceMigration/TestWorkspaceMigrateOutcomes/sdk-only_config_migrates_before_install$' --test-verbose
It reached otelgotest, ran the selected workspace tests, and ended with:
PASS
ok github.com/dagger/dagger/core/integration 19.638s
There was a telemetry upload warning at the end (api.dagger.cloud/v1/logs: 500), but the test command itself passed.
ok, push on latest workspace
Done. Fetched latest origin/workspace; PR branch is already based on it and push is up to date.
workspace: d7db83370fc5
PR head: 8c0f17296ee7
remote: 8c0f17296ee7
PR #13195 remains on latest workspace.
:thread-owner true
Thread owner set to true.
<@&1506565370385793125> did you push your commit to the workspace branch?
No. I pushed the commit to the PR head branch codex/workspace-compat-install-tests, rebased on latest origin/workspace.
I did not push directly to workspace. Current state:
origin/workspace: d7db83370fc5
origin/codex/workspace-compat-install-tests: 8c0f17296ee7
PR #13195 is the branch carrying my commit.
Since it's just a single commit. Just push it directly to workapce (make sure you fetch the latest tip first)
Pushed directly to workspace after fetching latest tip. It was a fast-forward push, no force:
0b2a47039..8f6ae4f1e HEAD -> workspace
Current origin/workspace and local HEAD are both 8f6ae4f1e442. I’m rerunning the focused test once more on this final workspace tip so the verification matches exactly what got pushed.
:tldr
This thread is about a regression in workspace detection/mutation around dagger.json fallback vs .dagger/config.toml, especially dagger install in SDK-only legacy repos.
Current status:
- The branch was turned into a test-only change and then pushed directly to
workspaceas a fast-forward commit. - Focused integration tests now pass on the final tip.
Important decisions/facts:
- The old expectation that
dagger installshould silently initialize a native workspace next to an SDK-onlydagger.jsonwas removed. - New test coverage is split by behavior:
workspace_modules_test.go: empty/native workspace install behaviorworkspace_compat_test.go: SDK-only legacy config should error and tell you to rundagger migratefirstworkspace_migration_test.go: migrate first, then install works
- Clear comments were added to the touched tests to explain the “what” and “why”.
Open questions/blockers:
- No product blocker remains in the thread; the main issue was test expectations, not runtime behavior.
- One telemetry warning showed up at the end of a run, but it did not fail the tests.
Next steps:
- Review the direct push on
workspaceand merge as appropriate.