Skip to content

fix(workspace): stop creating and sweep orphaned workspace dirs #496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
skevetter merged 1 commit into from
Jun 5, 2026
Merged

fix(workspace): stop creating and sweep orphaned workspace dirs #496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions pkg/provider/dir.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,10 @@ func SaveWorkspaceResult(workspace *Workspace, result *config2.Result) error {
return err
}

// #nosec G301 -- TODO Consider using a more secure permission setting and ownership if needed.
err = os.MkdirAll(workspaceDir, 0o755)
if err != nil {
if _, err := os.Stat(filepath.Join(workspaceDir, WorkspaceConfigFile)); err != nil {
if os.IsNotExist(err) {
return fmt.Errorf("workspace %s no longer exists", workspace.ID)
}
return err
}

Expand Down
46 changes: 46 additions & 0 deletions pkg/provider/dir_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package provider

import (
"os"
"path/filepath"
"testing"

"github.com/devsy-org/devsy/pkg/config"
devcontainerconfig "github.com/devsy-org/devsy/pkg/devcontainer/config"
"github.com/stretchr/testify/require"
)

func setupTestHome(t *testing.T) {
t.Helper()
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
config.ResetPathManager()
t.Cleanup(config.ResetPathManager)
}

func TestSaveWorkspaceResult_RefusesWithoutConfig(t *testing.T) {
setupTestHome(t)

ws := &Workspace{ID: "ghost", Context: config.DefaultContext}
err := SaveWorkspaceResult(ws, &devcontainerconfig.Result{})
require.Error(t, err)

// Must not leave an orphan dir behind.
dir, dirErr := GetWorkspaceDir(ws.Context, ws.ID)
require.NoError(t, dirErr)
require.NoDirExists(t, dir)
}

func TestSaveWorkspaceResult_WritesWhenConfigExists(t *testing.T) {
setupTestHome(t)

ws := &Workspace{ID: "real", Context: config.DefaultContext}
require.NoError(t, SaveWorkspaceConfig(ws))
require.NoError(t, SaveWorkspaceResult(ws, &devcontainerconfig.Result{}))

dir, err := GetWorkspaceDir(ws.Context, ws.ID)
require.NoError(t, err)
_, err = os.Stat(filepath.Join(dir, WorkspaceResultFile))
require.NoError(t, err)
}
47 changes: 46 additions & 1 deletion pkg/workspace/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@ package workspace
import (
"context"
"fmt"
"os"
"path/filepath"
"strings"

client2 "github.com/devsy-org/devsy/pkg/client"
"github.com/devsy-org/devsy/pkg/client/clientimplementation"
"github.com/devsy-org/devsy/pkg/config"
"github.com/devsy-org/devsy/pkg/ide/opener"
"github.com/devsy-org/devsy/pkg/log"
"github.com/devsy-org/devsy/pkg/platform"
providerpkg "github.com/devsy-org/devsy/pkg/provider"
)

// DeleteOptions holds the parameters for deleting a workspace.
Expand Down Expand Up @@ -51,7 +55,11 @@ func Delete(ctx context.Context, opts DeleteOptions) (string, error) {

stopIfRunning(ctx, client, status)

return deleteWorkspace(ctx, client, opts)
id, err := deleteWorkspace(ctx, client, opts)
if err == nil {
SweepOrphanWorkspaceDirs(opts.DevsyConfig.DefaultContext)
}
Comment on lines +58 to +61

@coderabbitai coderabbitai Bot Jun 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Run the orphan sweep on every successful delete path.

SweepOrphanWorkspaceDirs is only reached after the deleteWorkspace branch on Line 58. Successful deletes that return earlier—imported workspaces on Lines 46-47, and --force folder deletes returned from Line 41 via handleDeleteLoadError/forceDeleteFolder—skip the sweep entirely, so existing config-less dirs still survive in those flows. Please move the sweep into a shared post-success return path or invoke it from the other successful branches too, and add a regression test for those branches.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/workspace/delete.go` around lines 58 - 61, The orphan-sweep call
SweepOrphanWorkspaceDirs(opts.DevsyConfig.DefaultContext) is only executed after
the deleteWorkspace path and is skipped by other successful delete flows (the
imported-workspace early return and the --force folder delete via
handleDeleteLoadError/forceDeleteFolder); move the sweep into a shared
post-success exit path so it runs for every successful delete (or explicitly
invoke SweepOrphanWorkspaceDirs from the other success branches in the same
function), updating the code paths around deleteWorkspace,
handleDeleteLoadError, and forceDeleteFolder accordingly, and add a regression
test that covers an imported workspace delete and the --force folder delete to
assert orphan dirs are swept.

return id, err
}

// stopIfRunning stops the workspace before deletion when it is currently
Expand Down Expand Up @@ -308,3 +316,40 @@ func hasOtherWorkspaces(

return false, nil
}

func SweepOrphanWorkspaceDirs(contextName string) {
workspaceDir, err := providerpkg.GetWorkspacesDir(contextName)
if err != nil {
log.Debugf("sweep orphan workspaces: get dir: %v", err)
return
}

entries, err := os.ReadDir(workspaceDir)
if err != nil {
if !os.IsNotExist(err) {
log.Debugf("sweep orphan workspaces: read dir: %v", err)
}
return
}

for _, entry := range entries {
removeIfOrphan(workspaceDir, entry)
}
}

func removeIfOrphan(workspaceDir string, entry os.DirEntry) {
if !entry.IsDir() || strings.HasPrefix(entry.Name(), ".") {
return
}

configPath := filepath.Join(workspaceDir, entry.Name(), providerpkg.WorkspaceConfigFile)
if _, err := os.Stat(configPath); err == nil || !os.IsNotExist(err) {
return
}

if err := os.RemoveAll(filepath.Join(workspaceDir, entry.Name())); err != nil {
log.Warnf("remove orphan workspace dir %s: %v", entry.Name(), err)
return
}
log.Debugf("removed orphan workspace dir without config: workspace=%s", entry.Name())
}
44 changes: 44 additions & 0 deletions pkg/workspace/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package workspace

import (
"os"
"path/filepath"
"testing"

"github.com/devsy-org/devsy/pkg/provider"
"github.com/stretchr/testify/require"
)

func TestSweepOrphanWorkspaceDirs(t *testing.T) {
setupTestPathManager(t)

require.NoError(t, provider.SaveWorkspaceConfig(
&provider.Workspace{ID: "healthy", Context: testDefaultContext},
))

workspacesDir, err := provider.GetWorkspacesDir(testDefaultContext)
require.NoError(t, err)

// Orphan dir with only auxiliary state, plus a dotfile that must survive.
orphanDir := filepath.Join(workspacesDir, "orphan")
require.NoError(t, os.MkdirAll(filepath.Join(orphanDir, "logs"), 0o750))
require.NoError(t, os.WriteFile(
filepath.Join(orphanDir, provider.WorkspaceResultFile), []byte("{}"), 0o600,
))
dotDir := filepath.Join(workspacesDir, ".keep")
require.NoError(t, os.MkdirAll(dotDir, 0o750))

SweepOrphanWorkspaceDirs(testDefaultContext)

healthyDir := filepath.Join(workspacesDir, "healthy")
require.DirExists(t, healthyDir)
require.NoDirExists(t, orphanDir)
require.DirExists(t, dotDir)
}

func TestSweepOrphanWorkspaceDirs_MissingDirIsNoop(t *testing.T) {
setupTestPathManager(t)

// No workspaces dir created yet — sweep must not panic or error.
SweepOrphanWorkspaceDirs(testDefaultContext)
}
5 changes: 4 additions & 1 deletion pkg/workspace/rename_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func writeWorkspaceResult(
t.Helper()

ws := &provider.Workspace{ID: workspaceID, Context: testDefaultContext}
require.NoError(t, provider.SaveWorkspaceConfig(ws))
require.NoError(t, provider.SaveWorkspaceResult(ws, result))
}

Expand Down Expand Up @@ -353,7 +354,9 @@ func TestUpdateWorkspaceResult_RawJSON(t *testing.T) {
wsDir, err := provider.GetWorkspaceDir(testDefaultContext, newName)
require.NoError(t, err)
require.NoError(t, os.MkdirAll(wsDir, 0o750))

require.NoError(t, provider.SaveWorkspaceConfig(
&provider.Workspace{ID: newName, Context: testDefaultContext},
))
rawJSON := `{
"SubstitutionContext": {
"ContainerWorkspaceFolder": "/workspaces/old-ws",
Expand Down
Loading