From e95eabd934182fa854e150c5ba8ccf50e5c9078c Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 22 Jun 2026 13:17:18 -0500 Subject: [PATCH 1/5] fix(docker): bust Docker Desktop stale bind-mount inode on up/delete/up Workspace `up` failed intermittently on Docker Desktop after an up -> delete -> up cycle: the daemon rejected the workspace bind mount with "bind source path does not exist: /host_mnt/..." even though the host path existed. Root cause is Docker Desktop's file-sharing layer caching a stale inode for the workspace's recreated parent directory. `workspace delete` does os.RemoveAll on .../workspaces/, and the next `up` recreates the same .../agent/ path with a new host inode; the VM still maps the old one and cannot resolve children created under it. Retrying the same mount in place never succeeds. PR #512's content- leaf did not help because the poisoned directory is the parent agent/, which the lookup must traverse regardless of the leaf name. Fix: add bind-create-src=true to the workspace bind mount on runtimes that parse it (Docker only). This forces the daemon to re-resolve the source through the file share, busting the stale inode; because the directory really exists it binds the real content. The option is gated on an os.Stat existence check so a genuinely missing source still fails loudly (exit 125) instead of being silently created as an empty dir. Also drop the now-pointless content- suffix back to a plain "content" path, since it never delivered the protection it was added for and bind-create-src is the real fix. Co-Authored-By: Claude Opus 4.8 --- cmd/internal/agentworkspace/up.go | 1 - pkg/agent/agent.go | 7 ++----- pkg/agent/workspace.go | 7 ++----- pkg/compose/helper_test.go | 1 + pkg/docker/runtime.go | 4 ++++ pkg/driver/docker/docker.go | 21 +++++++++++++++++++++ pkg/driver/docker/docker_test.go | 21 +++++++++++++++++++++ 7 files changed, 51 insertions(+), 11 deletions(-) diff --git a/cmd/internal/agentworkspace/up.go b/cmd/internal/agentworkspace/up.go index 28ed2a1b5..d52e2680e 100644 --- a/cmd/internal/agentworkspace/up.go +++ b/cmd/internal/agentworkspace/up.go @@ -544,7 +544,6 @@ func prepareWorkspace(params prepareWorkspaceParams) error { params.workspaceInfo.Workspace.Source.LocalFolder != "" { params.workspaceInfo.ContentFolder = agent.GetAgentWorkspaceContentDir( params.workspaceInfo.Origin, - params.workspaceInfo.Workspace.UID, ) } diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 85fda2fce..149e27c75 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -246,7 +246,7 @@ func handleStaleWorkspace( return "", fmt.Errorf("recreate workspace dir: %w", err) } - // Drop the old UID's path so resolveContentFolder picks up the new one. + // Clear so resolveContentFolder recomputes against the recreated dir. workspaceInfo.ContentFolder = "" return newDir, nil } @@ -265,10 +265,7 @@ func resolveContentFolder( } } if workspaceInfo.ContentFolder == "" { - workspaceInfo.ContentFolder = GetAgentWorkspaceContentDir( - workspaceDir, - workspaceInfo.Workspace.UID, - ) + workspaceInfo.ContentFolder = GetAgentWorkspaceContentDir(workspaceDir) } } diff --git a/pkg/agent/workspace.go b/pkg/agent/workspace.go index 5b4ba88e0..38e241786 100644 --- a/pkg/agent/workspace.go +++ b/pkg/agent/workspace.go @@ -215,11 +215,8 @@ echo Devsy return true, nil } -// GetAgentWorkspaceContentDir returns the bind-mount source for a workspace. -// The UID suffix ensures a delete-then-recreate cycle never reuses the same -// host path, avoiding Docker Desktop's stale /host_mnt inode cache. -func GetAgentWorkspaceContentDir(workspaceDir, uid string) string { - return filepath.Join(workspaceDir, "content-"+uid) +func GetAgentWorkspaceContentDir(workspaceDir string) string { + return filepath.Join(workspaceDir, "content") } func GetAgentBinariesDirFromWorkspaceDir(workspaceDir string) (string, error) { diff --git a/pkg/compose/helper_test.go b/pkg/compose/helper_test.go index 92668b0b1..696f4a91b 100644 --- a/pkg/compose/helper_test.go +++ b/pkg/compose/helper_test.go @@ -177,6 +177,7 @@ func (r stubRuntime) Name() docker.RuntimeName { return r.name } func (r stubRuntime) SupportsInternalBuildKit() bool { return false } func (r stubRuntime) SupportsSignalProxy() bool { return false } func (r stubRuntime) SupportsMountConsistency() bool { return false } +func (r stubRuntime) SupportsBindCreateSrc() bool { return false } func (r stubRuntime) NeedsUserNamespaceArgs() bool { return false } func (r stubRuntime) GPUAvailable(_ context.Context, _ *docker.DockerHelper) (bool, error) { return false, nil diff --git a/pkg/docker/runtime.go b/pkg/docker/runtime.go index 376d6476a..cef1150a4 100644 --- a/pkg/docker/runtime.go +++ b/pkg/docker/runtime.go @@ -26,6 +26,7 @@ type ContainerRuntime interface { SupportsInternalBuildKit() bool SupportsSignalProxy() bool SupportsMountConsistency() bool + SupportsBindCreateSrc() bool NeedsUserNamespaceArgs() bool GPUAvailable(ctx context.Context, helper *DockerHelper) (bool, error) } @@ -36,6 +37,7 @@ func (dockerRuntime) Name() RuntimeName { return RuntimeDocker } func (dockerRuntime) SupportsInternalBuildKit() bool { return true } func (dockerRuntime) SupportsSignalProxy() bool { return true } func (dockerRuntime) SupportsMountConsistency() bool { return true } +func (dockerRuntime) SupportsBindCreateSrc() bool { return true } func (dockerRuntime) NeedsUserNamespaceArgs() bool { return false } func (dockerRuntime) GPUAvailable(ctx context.Context, h *DockerHelper) (bool, error) { @@ -52,6 +54,7 @@ func (podmanRuntime) Name() RuntimeName { return RuntimePodman } func (podmanRuntime) SupportsInternalBuildKit() bool { return false } func (podmanRuntime) SupportsSignalProxy() bool { return true } func (podmanRuntime) SupportsMountConsistency() bool { return true } +func (podmanRuntime) SupportsBindCreateSrc() bool { return false } func (podmanRuntime) NeedsUserNamespaceArgs() bool { return true } func (podmanRuntime) GPUAvailable(ctx context.Context, h *DockerHelper) (bool, error) { @@ -68,6 +71,7 @@ func (nerdctlRuntime) Name() RuntimeName { return RuntimeNerdctl } func (nerdctlRuntime) SupportsInternalBuildKit() bool { return true } func (nerdctlRuntime) SupportsSignalProxy() bool { return false } func (nerdctlRuntime) SupportsMountConsistency() bool { return false } +func (nerdctlRuntime) SupportsBindCreateSrc() bool { return false } func (nerdctlRuntime) NeedsUserNamespaceArgs() bool { return false } func (nerdctlRuntime) GPUAvailable(ctx context.Context, h *DockerHelper) (bool, error) { diff --git a/pkg/driver/docker/docker.go b/pkg/driver/docker/docker.go index 5526b2fe6..011a33996 100644 --- a/pkg/driver/docker/docker.go +++ b/pkg/driver/docker/docker.go @@ -660,11 +660,32 @@ func (d *dockerDriver) addWorkspaceMountArgs( if !helper.GetRuntime().SupportsMountConsistency() { mountPath = stripMountConsistency(mountPath) } + if helper.GetRuntime().SupportsBindCreateSrc() { + mountPath = withBindCreateSrc(mountPath) + } args = append(args, "--mount", mountPath) } return args } +// withBindCreateSrc adds bind-create-src=true to a bind --mount whose source +// exists, forcing Docker Desktop to resolve the path. +func withBindCreateSrc(mountPath string) string { + m := config.ParseMount(mountPath) + if m.Type != "bind" || m.Source == "" { + return mountPath + } + for part := range strings.SplitSeq(mountPath, ",") { + if strings.HasPrefix(part, "bind-create-src=") { + return mountPath + } + } + if _, err := os.Stat(m.Source); err != nil { + return mountPath + } + return mountPath + ",bind-create-src=true" +} + func stripMountConsistency(mount string) string { var parts []string for part := range strings.SplitSeq(mount, ",") { diff --git a/pkg/driver/docker/docker_test.go b/pkg/driver/docker/docker_test.go index 89d2d6003..15889bc5f 100644 --- a/pkg/driver/docker/docker_test.go +++ b/pkg/driver/docker/docker_test.go @@ -265,6 +265,27 @@ func (s *DockerDriverTestSuite) TestStripMountConsistency() { } } +func (s *DockerDriverTestSuite) TestWithBindCreateSrc() { + existing := s.T().TempDir() + + // Source exists: bind-create-src is appended to bust a stale file-share + // inode while still binding the real (present) directory. + withExisting := "type=bind,src=" + existing + ",dst=/b" + s.Equal(withExisting+",bind-create-src=true", withBindCreateSrc(withExisting)) + + // Source missing: spec is left untouched so docker fails loudly instead of + // silently materializing an empty placeholder directory. + s.Equal(testBindMount, withBindCreateSrc(testBindMount)) + + // Idempotent: an existing bind-create-src is not duplicated. + already := withExisting + ",bind-create-src=true" + s.Equal(already, withBindCreateSrc(already)) + + // Non-bind mounts are ignored. + vol := "type=volume,src=myvol,dst=/b" + s.Equal(vol, withBindCreateSrc(vol)) +} + func (s *DockerDriverTestSuite) TestAddRunPlatform_SetAppendsFlag() { b := &runArgsBuilder{ args: []string{testRunArg}, From d2525144cfe5f32dd84ed26efc1c2a293dc7c134 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Mon, 22 Jun 2026 14:31:58 -0500 Subject: [PATCH 2/5] fix(docker): gate bind-create-src to Docker Desktop + CLI >= v29 The bind-create-src mount option is only parsed by the docker CLI v29+, and only relevant on Docker Desktop. Emitting it unconditionally broke every `up` on Linux CI (docker 28.0.4), which rejects the unknown mount key at parse time: invalid argument ... unexpected key 'bind-create-src' Gate it three ways instead of the previous runtime-name-only capability: - Docker runtime (Podman/nerdctl reject the option) - Docker Desktop only (GOOS != linux); native Linux binds the host path directly and never hits the file-share inode cache - docker CLI >= v29, detected via `docker version`; older clients error out, which would turn the intermittent bug into a hard failure Adds DockerHelper.ClientVersion and replaces SupportsBindCreateSrc() with shouldBustBindCache()/dockerMajorAtLeast() in the driver. Co-Authored-By: Claude Opus 4.8 --- pkg/compose/helper_test.go | 1 - pkg/docker/helper.go | 13 +++++++++ pkg/docker/runtime.go | 4 --- pkg/driver/docker/docker.go | 47 ++++++++++++++++++++++++++++++-- pkg/driver/docker/docker_test.go | 19 +++++++++++++ 5 files changed, 77 insertions(+), 7 deletions(-) diff --git a/pkg/compose/helper_test.go b/pkg/compose/helper_test.go index 696f4a91b..92668b0b1 100644 --- a/pkg/compose/helper_test.go +++ b/pkg/compose/helper_test.go @@ -177,7 +177,6 @@ func (r stubRuntime) Name() docker.RuntimeName { return r.name } func (r stubRuntime) SupportsInternalBuildKit() bool { return false } func (r stubRuntime) SupportsSignalProxy() bool { return false } func (r stubRuntime) SupportsMountConsistency() bool { return false } -func (r stubRuntime) SupportsBindCreateSrc() bool { return false } func (r stubRuntime) NeedsUserNamespaceArgs() bool { return false } func (r stubRuntime) GPUAvailable(_ context.Context, _ *docker.DockerHelper) (bool, error) { return false, nil diff --git a/pkg/docker/helper.go b/pkg/docker/helper.go index e72c3a020..fcc8b9973 100644 --- a/pkg/docker/helper.go +++ b/pkg/docker/helper.go @@ -75,6 +75,19 @@ func (r *DockerHelper) GetRuntime() ContainerRuntime { return DetectRuntime(r.DockerCommand) } +// ClientVersion returns the docker CLI version (e.g. "29.5.3"), or "" if it +// cannot be determined. It queries the client only, so it succeeds even when +// no daemon is reachable. +func (r *DockerHelper) ClientVersion(ctx context.Context) string { + cctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + out, err := r.buildCmd(cctx, "version", "--format", "{{.Client.Version}}").Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + func (r *DockerHelper) FindDevContainer( ctx context.Context, labels []string, diff --git a/pkg/docker/runtime.go b/pkg/docker/runtime.go index cef1150a4..376d6476a 100644 --- a/pkg/docker/runtime.go +++ b/pkg/docker/runtime.go @@ -26,7 +26,6 @@ type ContainerRuntime interface { SupportsInternalBuildKit() bool SupportsSignalProxy() bool SupportsMountConsistency() bool - SupportsBindCreateSrc() bool NeedsUserNamespaceArgs() bool GPUAvailable(ctx context.Context, helper *DockerHelper) (bool, error) } @@ -37,7 +36,6 @@ func (dockerRuntime) Name() RuntimeName { return RuntimeDocker } func (dockerRuntime) SupportsInternalBuildKit() bool { return true } func (dockerRuntime) SupportsSignalProxy() bool { return true } func (dockerRuntime) SupportsMountConsistency() bool { return true } -func (dockerRuntime) SupportsBindCreateSrc() bool { return true } func (dockerRuntime) NeedsUserNamespaceArgs() bool { return false } func (dockerRuntime) GPUAvailable(ctx context.Context, h *DockerHelper) (bool, error) { @@ -54,7 +52,6 @@ func (podmanRuntime) Name() RuntimeName { return RuntimePodman } func (podmanRuntime) SupportsInternalBuildKit() bool { return false } func (podmanRuntime) SupportsSignalProxy() bool { return true } func (podmanRuntime) SupportsMountConsistency() bool { return true } -func (podmanRuntime) SupportsBindCreateSrc() bool { return false } func (podmanRuntime) NeedsUserNamespaceArgs() bool { return true } func (podmanRuntime) GPUAvailable(ctx context.Context, h *DockerHelper) (bool, error) { @@ -71,7 +68,6 @@ func (nerdctlRuntime) Name() RuntimeName { return RuntimeNerdctl } func (nerdctlRuntime) SupportsInternalBuildKit() bool { return true } func (nerdctlRuntime) SupportsSignalProxy() bool { return false } func (nerdctlRuntime) SupportsMountConsistency() bool { return false } -func (nerdctlRuntime) SupportsBindCreateSrc() bool { return false } func (nerdctlRuntime) NeedsUserNamespaceArgs() bool { return false } func (nerdctlRuntime) GPUAvailable(ctx context.Context, h *DockerHelper) (bool, error) { diff --git a/pkg/driver/docker/docker.go b/pkg/driver/docker/docker.go index 011a33996..418221086 100644 --- a/pkg/driver/docker/docker.go +++ b/pkg/driver/docker/docker.go @@ -660,7 +660,7 @@ func (d *dockerDriver) addWorkspaceMountArgs( if !helper.GetRuntime().SupportsMountConsistency() { mountPath = stripMountConsistency(mountPath) } - if helper.GetRuntime().SupportsBindCreateSrc() { + if shouldBustBindCache(helper) { mountPath = withBindCreateSrc(mountPath) } args = append(args, "--mount", mountPath) @@ -668,8 +668,51 @@ func (d *dockerDriver) addWorkspaceMountArgs( return args } +// minBindCreateSrcMajor is the docker CLI major version that introduced the +// bind-create-src mount option. Older clients reject it at parse time. +const minBindCreateSrcMajor = 29 + +// shouldBustBindCache reports whether the workspace bind mount should carry +// bind-create-src=true to work around Docker Desktop's stale file-share inode +// cache (the up -> delete -> up failure mode). It is gated three ways: +// - Docker runtime only (Podman/nerdctl reject the option). +// - Docker Desktop only (GOOS != linux); native Linux binds the host path +// directly and never hits the file-share cache. +// - docker CLI >= v29, which introduced the option; older clients error out. +func shouldBustBindCache(helper *docker.DockerHelper) bool { + if helper.GetRuntime().Name() != docker.RuntimeDocker { + return false + } + if runtime.GOOS == "linux" { + return false + } + return dockerMajorAtLeast(helper.ClientVersion(context.Background()), minBindCreateSrcMajor) +} + +// dockerMajorAtLeast reports whether a docker version string like "29.5.3" has +// a major component >= min. An unparseable version returns false so we never +// emit an option an unknown client might reject. +func dockerMajorAtLeast(version string, minMajor int) bool { + major, _, ok := strings.Cut(version, ".") + if !ok { + return false + } + n, err := strconv.Atoi(strings.TrimSpace(major)) + if err != nil { + return false + } + return n >= minMajor +} + // withBindCreateSrc adds bind-create-src=true to a bind --mount whose source -// exists, forcing Docker Desktop to resolve the path. +// exists, forcing Docker Desktop to re-resolve the path and bust a stale inode +// from a delete-then-recreated parent. +// +// The os.Stat check gates this on the source actually existing. With +// bind-create-src set, the daemon would create a missing source as an empty +// directory and start the container against it. Omitting the option for a +// missing source preserves docker's "bind source path does not exist" +// rejection, surfaced by startContainer as a failed run. func withBindCreateSrc(mountPath string) string { m := config.ParseMount(mountPath) if m.Type != "bind" || m.Source == "" { diff --git a/pkg/driver/docker/docker_test.go b/pkg/driver/docker/docker_test.go index 15889bc5f..06af1f6fa 100644 --- a/pkg/driver/docker/docker_test.go +++ b/pkg/driver/docker/docker_test.go @@ -286,6 +286,25 @@ func (s *DockerDriverTestSuite) TestWithBindCreateSrc() { s.Equal(vol, withBindCreateSrc(vol)) } +func (s *DockerDriverTestSuite) TestDockerMajorAtLeast() { + tests := []struct { + version string + want bool + }{ + {"29.5.3", true}, + {"29.0.0", true}, + {"30.1.0", true}, + {"28.0.4", false}, // CI's version; must not emit bind-create-src + {"20.10.21", false}, + {"", false}, // version unknown -> do not risk the option + {"garbage", false}, // unparseable major + } + for _, tt := range tests { + s.Equalf(tt.want, dockerMajorAtLeast(tt.version, minBindCreateSrcMajor), + "dockerMajorAtLeast(%q)", tt.version) + } +} + func (s *DockerDriverTestSuite) TestAddRunPlatform_SetAppendsFlag() { b := &runArgsBuilder{ args: []string{testRunArg}, From 131b47675780fe1bcb57434febfe6dfc1060776c Mon Sep 17 00:00:00 2001 From: Samuel K Date: Tue, 23 Jun 2026 11:11:42 -0500 Subject: [PATCH 3/5] fix(docker): relocate workspace content to a delete-stable parent dir The up -> delete -> up bind-mount failure on Docker Desktop is caused by the bind-mount source's PARENT directory being delete+recreated at the same host path. Docker Desktop's file share caches the parent inode; once it is recreated, the VM can no longer resolve bind sources beneath it. The content folder lived at workspaces//agent/content, entirely inside the workspaces/ tree that `delete` removes with RemoveAll, so every delete churned the parent inode. Move the host-side content folder to a per-context contents/ dir. The contents/ parent is created once and never removed on delete (only the leaf is), so its host inode stays stable across recreate and the file share keeps resolving the bind source. Verified: up/delete/up cycles pass 5/5 with bind-create-src disabled (i.e. this fixes Docker Desktop versions older than v29 that lack the flag), and inode tracking confirms contents/ is never recreated while the leaf is. bind-create-src is retained as a fast path / extra safety on v29+. Co-Authored-By: Claude Opus 4.8 --- pkg/agent/agent.go | 21 +++++++++++-- .../clientimplementation/workspace_client.go | 10 +++++++ pkg/config/pathmanager.go | 30 +++++++++++++++++++ pkg/config/pathmanager_linux_test.go | 10 +++++++ pkg/provider/dir.go | 17 +++++++++++ 5 files changed, 86 insertions(+), 2 deletions(-) diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 149e27c75..15b12b3b1 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -264,9 +264,26 @@ func resolveContentFolder( workspaceInfo.ContentFolder = workspaceInfo.Workspace.Source.LocalFolder } } - if workspaceInfo.ContentFolder == "" { - workspaceInfo.ContentFolder = GetAgentWorkspaceContentDir(workspaceDir) + if workspaceInfo.ContentFolder != "" { + return } + + // On the host the content folder is the docker bind-mount source. Place it + // under the per-context "contents" dir whose parent survives workspace + // delete, so Docker Desktop's file-share inode cache for the parent stays + // valid across up -> delete -> up. Container-side agents keep content under + // the workspace dir (no host file share involved). + if IsHostAgentInvocation(workspaceInfo.Agent.DataPath) { + if contentDir, err := provider2.GetWorkspaceContentDir( + workspaceInfo.Workspace.Context, + workspaceInfo.Workspace.ID, + ); err == nil { + workspaceInfo.ContentFolder = contentDir + return + } + } + + workspaceInfo.ContentFolder = GetAgentWorkspaceContentDir(workspaceDir) } func CreateWorkspaceBusyFile(folder string) { diff --git a/pkg/client/clientimplementation/workspace_client.go b/pkg/client/clientimplementation/workspace_client.go index c19064f77..7b05b47cb 100644 --- a/pkg/client/clientimplementation/workspace_client.go +++ b/pkg/client/clientimplementation/workspace_client.go @@ -870,6 +870,16 @@ func DeleteWorkspaceFolder(params DeleteWorkspaceFolderParams) error { return err } + // remove the stable content folder leaf, which lives outside WorkspaceDir + // (under the per-context "contents" dir) and is therefore not covered by + // the RemoveAll above. The parent "contents" dir is intentionally left in + // place so its host inode stays stable across recreate. + if contentFolder, err := provider.GetWorkspaceContentDir(params.Context, params.WorkspaceID); err == nil { + if err := os.RemoveAll(contentFolder); err != nil && !os.IsNotExist(err) { + return err + } + } + return nil } diff --git a/pkg/config/pathmanager.go b/pkg/config/pathmanager.go index dead55942..93b9d3e84 100644 --- a/pkg/config/pathmanager.go +++ b/pkg/config/pathmanager.go @@ -41,6 +41,8 @@ type PathManager interface { WorkspacesDir(context string) (string, error) WorkspaceDir(context, workspaceID string) (string, error) WorkspaceAgentDir(context, workspaceID string) (string, error) + WorkspaceContentsDir(context string) (string, error) + WorkspaceContentDir(context, workspaceID string) (string, error) WorkspaceLogDir(context, workspaceID string) (string, error) MachinesDir(context string) (string, error) MachineDir(context, machineID string) (string, error) @@ -135,6 +137,34 @@ func (b *basePathManager) WorkspaceAgentDir(context, workspaceID string) (string return filepath.Join(dir, "agent"), nil } +// WorkspaceContentsDir is the per-context parent of all workspace content +// folders. Unlike WorkspaceDir it is created once and never removed during a +// workspace delete, so the bind-mount source's parent directory keeps a stable +// host inode across up -> delete -> up. Docker Desktop's file share caches the +// parent inode; recreating it (as happens when content lives under the +// delete-and-recreate WorkspaceDir) leaves the share unable to resolve the new +// bind source. +func (b *basePathManager) WorkspaceContentsDir(context string) (string, error) { + dir, err := b.ContextDir(context) + if err != nil { + return "", err + } + + return filepath.Join(dir, "contents"), nil +} + +// WorkspaceContentDir is the stable bind-mount source for a workspace, living +// under WorkspaceContentsDir. Only this leaf is removed on delete; its parent +// persists. +func (b *basePathManager) WorkspaceContentDir(context, workspaceID string) (string, error) { + dir, err := b.WorkspaceContentsDir(context) + if err != nil { + return "", err + } + + return filepath.Join(dir, workspaceID), nil +} + // WorkspaceLogDir is the per-workspace log directory used by the desktop's // streaming-log store. Lives under WorkspaceDir for the same reason as // WorkspaceAgentDir. diff --git a/pkg/config/pathmanager_linux_test.go b/pkg/config/pathmanager_linux_test.go index 8ff235506..58d0b0455 100644 --- a/pkg/config/pathmanager_linux_test.go +++ b/pkg/config/pathmanager_linux_test.go @@ -131,6 +131,16 @@ func TestContextDataSubPaths(t *testing.T) { func() (string, error) { return pm.WorkspaceAgentDir(ctx, "ws1") }, filepath.Join(base, "workspaces", "ws1", "agent"), }, + { + "WorkspaceContentsDir", + func() (string, error) { return pm.WorkspaceContentsDir(ctx) }, + filepath.Join(base, "contents"), + }, + { + "WorkspaceContentDir", + func() (string, error) { return pm.WorkspaceContentDir(ctx, "ws1") }, + filepath.Join(base, "contents", "ws1"), + }, { "WorkspaceLogDir", func() (string, error) { return pm.WorkspaceLogDir(ctx, "ws1") }, diff --git a/pkg/provider/dir.go b/pkg/provider/dir.go index c42fa46cd..e1361ce87 100644 --- a/pkg/provider/dir.go +++ b/pkg/provider/dir.go @@ -85,6 +85,23 @@ func GetWorkspaceAgentDir(context, workspaceID string) (string, error) { return config.DefaultPathManager().WorkspaceAgentDir(context, workspaceID) } +// GetWorkspaceContentsDir returns the per-context parent of workspace content +// folders (.../contexts//contents). It is never removed on delete. +func GetWorkspaceContentsDir(context string) (string, error) { + return config.DefaultPathManager().WorkspaceContentsDir(context) +} + +// GetWorkspaceContentDir returns the host-side stable bind-mount source for a +// workspace (.../contexts//contents/). Its parent persists across +// delete so Docker Desktop's file-share inode cache stays valid. +func GetWorkspaceContentDir(context, workspaceID string) (string, error) { + if workspaceID == "" { + return "", fmt.Errorf("workspace id is empty") + } + + return config.DefaultPathManager().WorkspaceContentDir(context, workspaceID) +} + // GetWorkspaceLogDir returns the per-workspace log dir // (.../workspaces//logs) used by the desktop's streaming-log store. func GetWorkspaceLogDir(context, workspaceID string) (string, error) { From 8465d8bd49080b2c92ee047b9be995bff5e1d54b Mon Sep 17 00:00:00 2001 From: Samuel K Date: Tue, 23 Jun 2026 11:17:29 -0500 Subject: [PATCH 4/5] chore(docker): trim verbose comments Condense the explanatory comments added for the bind-mount fix to concise doc comments and one-liners. Co-Authored-By: Claude Opus 4.8 --- pkg/agent/agent.go | 7 ++--- .../clientimplementation/workspace_client.go | 6 ++-- pkg/config/pathmanager.go | 15 ++++------ pkg/docker/helper.go | 4 +-- pkg/driver/docker/docker.go | 30 +++++++------------ pkg/driver/docker/docker_test.go | 16 +++++----- pkg/provider/dir.go | 7 ++--- 7 files changed, 30 insertions(+), 55 deletions(-) diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 15b12b3b1..49f824a06 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -268,11 +268,8 @@ func resolveContentFolder( return } - // On the host the content folder is the docker bind-mount source. Place it - // under the per-context "contents" dir whose parent survives workspace - // delete, so Docker Desktop's file-share inode cache for the parent stays - // valid across up -> delete -> up. Container-side agents keep content under - // the workspace dir (no host file share involved). + // On the host, place the bind-mount source under the delete-stable + // "contents" dir so its parent inode survives up -> delete -> up. if IsHostAgentInvocation(workspaceInfo.Agent.DataPath) { if contentDir, err := provider2.GetWorkspaceContentDir( workspaceInfo.Workspace.Context, diff --git a/pkg/client/clientimplementation/workspace_client.go b/pkg/client/clientimplementation/workspace_client.go index 7b05b47cb..9350eabc5 100644 --- a/pkg/client/clientimplementation/workspace_client.go +++ b/pkg/client/clientimplementation/workspace_client.go @@ -870,10 +870,8 @@ func DeleteWorkspaceFolder(params DeleteWorkspaceFolderParams) error { return err } - // remove the stable content folder leaf, which lives outside WorkspaceDir - // (under the per-context "contents" dir) and is therefore not covered by - // the RemoveAll above. The parent "contents" dir is intentionally left in - // place so its host inode stays stable across recreate. + // remove the content folder leaf (outside WorkspaceDir); leave its parent + // "contents" dir in place to keep its host inode stable across recreate. if contentFolder, err := provider.GetWorkspaceContentDir(params.Context, params.WorkspaceID); err == nil { if err := os.RemoveAll(contentFolder); err != nil && !os.IsNotExist(err) { return err diff --git a/pkg/config/pathmanager.go b/pkg/config/pathmanager.go index 93b9d3e84..006f702be 100644 --- a/pkg/config/pathmanager.go +++ b/pkg/config/pathmanager.go @@ -137,13 +137,10 @@ func (b *basePathManager) WorkspaceAgentDir(context, workspaceID string) (string return filepath.Join(dir, "agent"), nil } -// WorkspaceContentsDir is the per-context parent of all workspace content -// folders. Unlike WorkspaceDir it is created once and never removed during a -// workspace delete, so the bind-mount source's parent directory keeps a stable -// host inode across up -> delete -> up. Docker Desktop's file share caches the -// parent inode; recreating it (as happens when content lives under the -// delete-and-recreate WorkspaceDir) leaves the share unable to resolve the new -// bind source. +// WorkspaceContentsDir is the per-context parent of workspace content folders. +// It is never removed on delete, keeping a stable host inode for the bind-mount +// source's parent across up -> delete -> up (avoids Docker Desktop's stale +// file-share inode cache). func (b *basePathManager) WorkspaceContentsDir(context string) (string, error) { dir, err := b.ContextDir(context) if err != nil { @@ -153,9 +150,7 @@ func (b *basePathManager) WorkspaceContentsDir(context string) (string, error) { return filepath.Join(dir, "contents"), nil } -// WorkspaceContentDir is the stable bind-mount source for a workspace, living -// under WorkspaceContentsDir. Only this leaf is removed on delete; its parent -// persists. +// WorkspaceContentDir is the bind-mount source leaf under WorkspaceContentsDir. func (b *basePathManager) WorkspaceContentDir(context, workspaceID string) (string, error) { dir, err := b.WorkspaceContentsDir(context) if err != nil { diff --git a/pkg/docker/helper.go b/pkg/docker/helper.go index fcc8b9973..1cc34a30d 100644 --- a/pkg/docker/helper.go +++ b/pkg/docker/helper.go @@ -75,9 +75,7 @@ func (r *DockerHelper) GetRuntime() ContainerRuntime { return DetectRuntime(r.DockerCommand) } -// ClientVersion returns the docker CLI version (e.g. "29.5.3"), or "" if it -// cannot be determined. It queries the client only, so it succeeds even when -// no daemon is reachable. +// ClientVersion returns the docker CLI version (e.g. "29.5.3"), or "". func (r *DockerHelper) ClientVersion(ctx context.Context) string { cctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() diff --git a/pkg/driver/docker/docker.go b/pkg/driver/docker/docker.go index 418221086..f83360472 100644 --- a/pkg/driver/docker/docker.go +++ b/pkg/driver/docker/docker.go @@ -668,17 +668,13 @@ func (d *dockerDriver) addWorkspaceMountArgs( return args } -// minBindCreateSrcMajor is the docker CLI major version that introduced the -// bind-create-src mount option. Older clients reject it at parse time. +// minBindCreateSrcMajor is the docker CLI major version that added the +// bind-create-src mount option; older clients reject it at parse time. const minBindCreateSrcMajor = 29 -// shouldBustBindCache reports whether the workspace bind mount should carry -// bind-create-src=true to work around Docker Desktop's stale file-share inode -// cache (the up -> delete -> up failure mode). It is gated three ways: -// - Docker runtime only (Podman/nerdctl reject the option). -// - Docker Desktop only (GOOS != linux); native Linux binds the host path -// directly and never hits the file-share cache. -// - docker CLI >= v29, which introduced the option; older clients error out. +// shouldBustBindCache reports whether to add bind-create-src to the workspace +// mount. Gated to Docker (Podman/nerdctl reject it), Docker Desktop +// (GOOS != linux), and CLI >= v29. func shouldBustBindCache(helper *docker.DockerHelper) bool { if helper.GetRuntime().Name() != docker.RuntimeDocker { return false @@ -689,9 +685,8 @@ func shouldBustBindCache(helper *docker.DockerHelper) bool { return dockerMajorAtLeast(helper.ClientVersion(context.Background()), minBindCreateSrcMajor) } -// dockerMajorAtLeast reports whether a docker version string like "29.5.3" has -// a major component >= min. An unparseable version returns false so we never -// emit an option an unknown client might reject. +// dockerMajorAtLeast reports whether version's major component is >= minMajor. +// An unparseable version returns false. func dockerMajorAtLeast(version string, minMajor int) bool { major, _, ok := strings.Cut(version, ".") if !ok { @@ -705,14 +700,9 @@ func dockerMajorAtLeast(version string, minMajor int) bool { } // withBindCreateSrc adds bind-create-src=true to a bind --mount whose source -// exists, forcing Docker Desktop to re-resolve the path and bust a stale inode -// from a delete-then-recreated parent. -// -// The os.Stat check gates this on the source actually existing. With -// bind-create-src set, the daemon would create a missing source as an empty -// directory and start the container against it. Omitting the option for a -// missing source preserves docker's "bind source path does not exist" -// rejection, surfaced by startContainer as a failed run. +// exists, forcing Docker Desktop to re-resolve a stale-cached path. A missing +// source is left untouched so docker still fails loudly rather than binding an +// empty dir. func withBindCreateSrc(mountPath string) string { m := config.ParseMount(mountPath) if m.Type != "bind" || m.Source == "" { diff --git a/pkg/driver/docker/docker_test.go b/pkg/driver/docker/docker_test.go index 06af1f6fa..73a449d1b 100644 --- a/pkg/driver/docker/docker_test.go +++ b/pkg/driver/docker/docker_test.go @@ -268,20 +268,18 @@ func (s *DockerDriverTestSuite) TestStripMountConsistency() { func (s *DockerDriverTestSuite) TestWithBindCreateSrc() { existing := s.T().TempDir() - // Source exists: bind-create-src is appended to bust a stale file-share - // inode while still binding the real (present) directory. + // source exists -> option appended withExisting := "type=bind,src=" + existing + ",dst=/b" s.Equal(withExisting+",bind-create-src=true", withBindCreateSrc(withExisting)) - // Source missing: spec is left untouched so docker fails loudly instead of - // silently materializing an empty placeholder directory. + // source missing -> untouched s.Equal(testBindMount, withBindCreateSrc(testBindMount)) - // Idempotent: an existing bind-create-src is not duplicated. + // idempotent already := withExisting + ",bind-create-src=true" s.Equal(already, withBindCreateSrc(already)) - // Non-bind mounts are ignored. + // non-bind ignored vol := "type=volume,src=myvol,dst=/b" s.Equal(vol, withBindCreateSrc(vol)) } @@ -294,10 +292,10 @@ func (s *DockerDriverTestSuite) TestDockerMajorAtLeast() { {"29.5.3", true}, {"29.0.0", true}, {"30.1.0", true}, - {"28.0.4", false}, // CI's version; must not emit bind-create-src + {"28.0.4", false}, {"20.10.21", false}, - {"", false}, // version unknown -> do not risk the option - {"garbage", false}, // unparseable major + {"", false}, + {"garbage", false}, } for _, tt := range tests { s.Equalf(tt.want, dockerMajorAtLeast(tt.version, minBindCreateSrcMajor), diff --git a/pkg/provider/dir.go b/pkg/provider/dir.go index e1361ce87..c5e5fd2b3 100644 --- a/pkg/provider/dir.go +++ b/pkg/provider/dir.go @@ -86,14 +86,13 @@ func GetWorkspaceAgentDir(context, workspaceID string) (string, error) { } // GetWorkspaceContentsDir returns the per-context parent of workspace content -// folders (.../contexts//contents). It is never removed on delete. +// folders (.../contexts//contents). func GetWorkspaceContentsDir(context string) (string, error) { return config.DefaultPathManager().WorkspaceContentsDir(context) } -// GetWorkspaceContentDir returns the host-side stable bind-mount source for a -// workspace (.../contexts//contents/). Its parent persists across -// delete so Docker Desktop's file-share inode cache stays valid. +// GetWorkspaceContentDir returns the host-side bind-mount source for a +// workspace (.../contexts//contents/). func GetWorkspaceContentDir(context, workspaceID string) (string, error) { if workspaceID == "" { return "", fmt.Errorf("workspace id is empty") From d60c402f9bef3eb6d18808587254da0e5572986c Mon Sep 17 00:00:00 2001 From: Samuel K Date: Tue, 23 Jun 2026 11:23:27 -0500 Subject: [PATCH 5/5] style: wrap long line to satisfy golines Split the content-folder cleanup if-init statement so the line stays under the 100-char golines limit. Co-Authored-By: Claude Opus 4.8 --- pkg/client/clientimplementation/workspace_client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/client/clientimplementation/workspace_client.go b/pkg/client/clientimplementation/workspace_client.go index 9350eabc5..8ae274df9 100644 --- a/pkg/client/clientimplementation/workspace_client.go +++ b/pkg/client/clientimplementation/workspace_client.go @@ -872,7 +872,8 @@ func DeleteWorkspaceFolder(params DeleteWorkspaceFolderParams) error { // remove the content folder leaf (outside WorkspaceDir); leave its parent // "contents" dir in place to keep its host inode stable across recreate. - if contentFolder, err := provider.GetWorkspaceContentDir(params.Context, params.WorkspaceID); err == nil { + contentFolder, err := provider.GetWorkspaceContentDir(params.Context, params.WorkspaceID) + if err == nil { if err := os.RemoveAll(contentFolder); err != nil && !os.IsNotExist(err) { return err }