From f9135ae7c311276929fceb81805c2ad7bb05a2c9 Mon Sep 17 00:00:00 2001 From: Koji AGAWA Date: Sun, 11 Oct 2026 22:28:20 +0900 Subject: [PATCH 1/2] feat(up): return resolved workspace folder in results Use the resolved container workspace folder as the default for successful up results when workspaceFolder is omitted. Preserve explicit folder, Git subpath, and CLI override precedence. This makes remoteWorkspaceFolder useful to scripts alongside the resolved containerId and remoteUser. Cover default/custom mount folders and precedence with unit tests and Docker JSON/openvscode E2E assertions. CLI lint, race tests, and both focused E2E cases pass. Co-authored-by: Codex --- cmd/workspace/up/up.go | 2 +- cmd/workspace/up/up_workdir_test.go | 107 ++++++++++++++++++++++++++++ e2e/tests/up/up_behaviors.go | 4 +- 3 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 cmd/workspace/up/up_workdir_test.go diff --git a/cmd/workspace/up/up.go b/cmd/workspace/up/up.go index c4574e8451..55155633cb 100644 --- a/cmd/workspace/up/up.go +++ b/cmd/workspace/up/up.go @@ -685,7 +685,7 @@ func (cmd *UpCmd) resolveWorkdir( result *config2.Result, client client2.BaseWorkspaceClient, ) string { - workdir := "" + workdir := result.SubstitutionContext.ContainerWorkspaceFolder if result.MergedConfig != nil && result.MergedConfig.WorkspaceFolder != "" { workdir = result.MergedConfig.WorkspaceFolder } diff --git a/cmd/workspace/up/up_workdir_test.go b/cmd/workspace/up/up_workdir_test.go new file mode 100644 index 0000000000..156c1b924b --- /dev/null +++ b/cmd/workspace/up/up_workdir_test.go @@ -0,0 +1,107 @@ +package up + +import ( + "bytes" + "encoding/json" + "testing" + + "github.com/devsy-org/devsy/cmd/flags" + devcconfig "github.com/devsy-org/devsy/pkg/devcontainer/config" + "github.com/devsy-org/devsy/pkg/provider" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type workspaceFolderCase struct { + name string + resolvedFolder string + configFolder string + subpath string + override string + noMergedConfig bool + want string +} + +func TestUpResultWorkspaceFolder(t *testing.T) { + const ( + defaultFolder = "/workspaces/project" + customFolder = "/src/project" + overrideFolder = "/override/project" + gitSubpath = "app" + ) + tests := []workspaceFolderCase{ + {name: "default folder", resolvedFolder: defaultFolder, want: defaultFolder}, + {name: "custom mount folder", resolvedFolder: customFolder, want: customFolder}, + { + name: "no merged config", + resolvedFolder: defaultFolder, + noMergedConfig: true, + want: defaultFolder, + }, + { + name: "explicit folder", + resolvedFolder: defaultFolder, + configFolder: customFolder, + want: customFolder, + }, + { + name: "git subpath", + resolvedFolder: defaultFolder, + subpath: gitSubpath, + want: defaultFolder + "/app", + }, + { + name: "git subpath with explicit folder", resolvedFolder: customFolder, + configFolder: customFolder, subpath: gitSubpath, want: customFolder + "/app", + }, + { + name: "CLI override", + resolvedFolder: defaultFolder, + override: overrideFolder, + want: overrideFolder, + }, + { + name: "CLI override takes precedence", + resolvedFolder: customFolder, + configFolder: customFolder, + subpath: gitSubpath, + override: overrideFolder, + want: overrideFolder, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assertUpResultFolder(t, tt) + }) + } +} + +func assertUpResultFolder(t *testing.T, test workspaceFolderCase) { + t.Helper() + result := &devcconfig.Result{ + SubstitutionContext: &devcconfig.SubstitutionContext{ + ContainerWorkspaceFolder: test.resolvedFolder, + }, + } + if !test.noMergedConfig { + result.MergedConfig = &devcconfig.MergedDevContainerConfig{ + DevContainerConfigBase: devcconfig.DevContainerConfigBase{ + WorkspaceFolder: test.configFolder, + }, + } + } + workspace := &provider.Workspace{ + Source: provider.WorkspaceSource{GitSubPath: test.subpath}, + } + client := prepareWorkspaceClient{workspaceConfig: workspace} + cmd := &UpCmd{GlobalFlags: &flags.GlobalFlags{}} + cmd.WorkspaceFolder = test.override + workdir := cmd.resolveWorkdir(result, client) + + var output bytes.Buffer + emitUpResult(&workspaceContext{result: result, workdir: workdir}, "", &output) + var envelope devcconfig.ResultEnvelope + require.NoError(t, json.Unmarshal(output.Bytes(), &envelope)) + assert.Equal(t, "success", envelope.Outcome) + assert.Equal(t, test.want, envelope.RemoteWorkspaceFolder) +} diff --git a/e2e/tests/up/up_behaviors.go b/e2e/tests/up/up_behaviors.go index 774955bfb6..16cd4ae4f1 100644 --- a/e2e/tests/up/up_behaviors.go +++ b/e2e/tests/up/up_behaviors.go @@ -160,6 +160,7 @@ var _ = ginkgo.Describe("up command behaviors", ginkgo.Label("up-behaviors"), fu framework.ExpectNoError(err) gomega.Expect(envelope.Outcome).To(gomega.Equal("success")) + gomega.Expect(envelope.RemoteWorkspaceFolder).NotTo(gomega.BeEmpty()) statusCount := 0 for _, line := range lines[:len(lines)-1] { var status struct { @@ -229,8 +230,7 @@ var _ = ginkgo.Describe("up command behaviors", ginkgo.Label("up-behaviors"), fu "expected exactly one success envelope in stdout, got %d", len(matched)) gomega.Expect(matched[0].ContainerID).NotTo(gomega.BeEmpty()) gomega.Expect(matched[0].RemoteUser).NotTo(gomega.BeEmpty()) - // RemoteWorkspaceFolder may legitimately be empty when the - // devcontainer.json doesn't set a workspaceFolder. + gomega.Expect(matched[0].RemoteWorkspaceFolder).NotTo(gomega.BeEmpty()) }, ginkgo.SpecTimeout(framework.TimeoutLong())) ginkgo.It("suppresses JSON envelope with --result-format plain", func(ctx context.Context) { From 91ecb26817b02aa475cc6ea96afd150255d2ae46 Mon Sep 17 00:00:00 2001 From: Koji AGAWA Date: Sun, 11 Oct 2026 23:07:22 +0900 Subject: [PATCH 2/2] fix(up): limit resolved folder fallback to JSON output Fill an empty remoteWorkspaceFolder from the substitution context only while emitting the success result. Restore the shared workdir resolver default so SSH configuration and tunnels keep their previous behavior. Verify JSON output and unchanged SSH workdir for default, custom and literal mount paths, and preserve explicit-folder/subpath/CLI override precedence. Cover absent result or substitution context. CLI lint, race tests and two focused Docker E2E specs pass. Co-authored-by: Codex --- cmd/workspace/up/up.go | 8 +++++-- cmd/workspace/up/up_workdir_test.go | 37 +++++++++++++++++++++++------ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/cmd/workspace/up/up.go b/cmd/workspace/up/up.go index 55155633cb..5192bba095 100644 --- a/cmd/workspace/up/up.go +++ b/cmd/workspace/up/up.go @@ -515,16 +515,20 @@ func reportErr(err error, _ bool, _ io.Writer) error { // emitUpResult writes the JSON result envelope for a completed `up` invocation. func emitUpResult(wctx *workspaceContext, ideURL string, out io.Writer) { containerID := config2.GetContainerID(wctx.result) + workdir := wctx.workdir var warnings []string recovery := false if wctx.result != nil { warnings = wctx.result.HostWarnings recovery = wctx.result.RecoveryContainer + if workdir == "" && wctx.result.SubstitutionContext != nil { + workdir = wctx.result.SubstitutionContext.ContainerWorkspaceFolder + } } _ = config2.WriteResultJSON(out, config2.ResultEnvelope{ ContainerID: containerID, RemoteUser: wctx.user, - RemoteWorkspaceFolder: wctx.workdir, + RemoteWorkspaceFolder: workdir, URL: ideURL, Warnings: warnings, Recovery: recovery, @@ -685,7 +689,7 @@ func (cmd *UpCmd) resolveWorkdir( result *config2.Result, client client2.BaseWorkspaceClient, ) string { - workdir := result.SubstitutionContext.ContainerWorkspaceFolder + workdir := "" if result.MergedConfig != nil && result.MergedConfig.WorkspaceFolder != "" { workdir = result.MergedConfig.WorkspaceFolder } diff --git a/cmd/workspace/up/up_workdir_test.go b/cmd/workspace/up/up_workdir_test.go index 156c1b924b..302b80c9d3 100644 --- a/cmd/workspace/up/up_workdir_test.go +++ b/cmd/workspace/up/up_workdir_test.go @@ -19,6 +19,7 @@ type workspaceFolderCase struct { subpath string override string noMergedConfig bool + wantWorkdir string want string } @@ -28,10 +29,12 @@ func TestUpResultWorkspaceFolder(t *testing.T) { customFolder = "/src/project" overrideFolder = "/override/project" gitSubpath = "app" + literalFolder = "/src/$(printf marker)" ) tests := []workspaceFolderCase{ {name: "default folder", resolvedFolder: defaultFolder, want: defaultFolder}, {name: "custom mount folder", resolvedFolder: customFolder, want: customFolder}, + {name: "literal mount folder", resolvedFolder: literalFolder, want: literalFolder}, { name: "no merged config", resolvedFolder: defaultFolder, @@ -42,23 +45,27 @@ func TestUpResultWorkspaceFolder(t *testing.T) { name: "explicit folder", resolvedFolder: defaultFolder, configFolder: customFolder, - want: customFolder, + wantWorkdir: customFolder, want: customFolder, }, { name: "git subpath", resolvedFolder: defaultFolder, subpath: gitSubpath, - want: defaultFolder + "/app", + wantWorkdir: defaultFolder + "/app", want: defaultFolder + "/app", }, { - name: "git subpath with explicit folder", resolvedFolder: customFolder, - configFolder: customFolder, subpath: gitSubpath, want: customFolder + "/app", + name: "git subpath with explicit folder", + resolvedFolder: customFolder, + configFolder: customFolder, + subpath: gitSubpath, + wantWorkdir: customFolder + "/app", + want: customFolder + "/app", }, { name: "CLI override", resolvedFolder: defaultFolder, override: overrideFolder, - want: overrideFolder, + wantWorkdir: overrideFolder, want: overrideFolder, }, { name: "CLI override takes precedence", @@ -66,7 +73,7 @@ func TestUpResultWorkspaceFolder(t *testing.T) { configFolder: customFolder, subpath: gitSubpath, override: overrideFolder, - want: overrideFolder, + wantWorkdir: overrideFolder, want: overrideFolder, }, } for _, tt := range tests { @@ -97,11 +104,27 @@ func assertUpResultFolder(t *testing.T, test workspaceFolderCase) { cmd := &UpCmd{GlobalFlags: &flags.GlobalFlags{}} cmd.WorkspaceFolder = test.override workdir := cmd.resolveWorkdir(result, client) + assert.Equal(t, test.wantWorkdir, workdir, "folder used by SSH") var output bytes.Buffer - emitUpResult(&workspaceContext{result: result, workdir: workdir}, "", &output) + wctx := &workspaceContext{result: result, workdir: workdir} + emitUpResult(wctx, "", &output) + assert.Equal(t, test.wantWorkdir, wctx.workdir, "JSON emission must preserve SSH folder") var envelope devcconfig.ResultEnvelope require.NoError(t, json.Unmarshal(output.Bytes(), &envelope)) assert.Equal(t, "success", envelope.Outcome) assert.Equal(t, test.want, envelope.RemoteWorkspaceFolder) } + +func TestEmitUpResultWithoutSubstitutionContext(t *testing.T) { + for _, result := range []*devcconfig.Result{nil, {}} { + for _, workdir := range []string{"", "/explicit"} { + var output bytes.Buffer + wctx := &workspaceContext{result: result, workdir: workdir} + emitUpResult(wctx, "", &output) + var envelope devcconfig.ResultEnvelope + require.NoError(t, json.Unmarshal(output.Bytes(), &envelope)) + assert.Equal(t, workdir, envelope.RemoteWorkspaceFolder) + } + } +}