diff --git a/internal/core/control_panel/installer_local.go b/internal/core/control_panel/installer_local.go index 1243f1457..9ca948fda 100644 --- a/internal/core/control_panel/installer_local.go +++ b/internal/core/control_panel/installer_local.go @@ -2,6 +2,10 @@ package controlpanel import ( "errors" + "fmt" + "os" + "path/filepath" + "strings" "github.com/langgenius/dify-plugin-daemon/pkg/entities/plugin_entities" ) @@ -58,7 +62,7 @@ func (c *ControlPanel) RemoveLocalPlugin( ) error { // remove the package from the `installedBucket` err := c.installedBucket.Delete(pluginUniqueIdentifier) - if err != nil { + if err != nil && !os.IsNotExist(err) { return errors.Join( errors.New("failed to delete package file from installed bucket when trying to remove plugin from local"), err, @@ -67,3 +71,53 @@ func (c *ControlPanel) RemoveLocalPlugin( return nil } + +func (c *ControlPanel) RemoveLocalPluginStorage( + pluginUniqueIdentifier plugin_entities.PluginUniqueIdentifier, +) error { + var errs []error + + if err := c.packageBucket.Delete(pluginUniqueIdentifier.String()); err != nil && !os.IsNotExist(err) { + errs = append(errs, errors.Join( + errors.New("failed to delete package file from package bucket when trying to remove plugin from local"), + err, + )) + } + + workingPath, err := c.localPluginWorkingPath(pluginUniqueIdentifier) + if err != nil { + errs = append(errs, err) + } else if err := os.RemoveAll(workingPath); err != nil { + errs = append(errs, errors.Join( + fmt.Errorf("failed to delete plugin working directory %s", workingPath), + err, + )) + } + + return errors.Join(errs...) +} + +func (c *ControlPanel) localPluginWorkingPath( + pluginUniqueIdentifier plugin_entities.PluginUniqueIdentifier, +) (string, error) { + identity, _, ok := strings.Cut(pluginUniqueIdentifier.String(), "@") + if !ok { + return "", fmt.Errorf("invalid plugin unique identifier: %s", pluginUniqueIdentifier.String()) + } + identity = strings.ReplaceAll(identity, ":", "-") + + base, err := filepath.Abs(c.config.PluginWorkingPath) + if err != nil { + return "", err + } + target, err := filepath.Abs(filepath.Join(base, fmt.Sprintf("%s@%s", identity, pluginUniqueIdentifier.Checksum()))) + if err != nil { + return "", err + } + + if target == base || !strings.HasPrefix(target, base+string(os.PathSeparator)) { + return "", fmt.Errorf("refusing to delete plugin working directory outside base path: %s", target) + } + + return target, nil +} diff --git a/internal/core/local_runtime/dependency_installation_test.go b/internal/core/local_runtime/dependency_installation_test.go index ec65cea93..5c1ea5c3d 100644 --- a/internal/core/local_runtime/dependency_installation_test.go +++ b/internal/core/local_runtime/dependency_installation_test.go @@ -174,7 +174,7 @@ func TestPrepareSyncArgs(t *testing.T) { } args := runtime.prepareSyncArgs(false) - require.Equal(t, []string{"sync", "--no-dev"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow"}, args) }) t.Run("sync args with mirror URL", func(t *testing.T) { @@ -185,7 +185,7 @@ func TestPrepareSyncArgs(t *testing.T) { } args := runtime.prepareSyncArgs(false) - require.Equal(t, []string{"sync", "--no-dev", "-i", "https://pypi.tuna.tsinghua.edu.cn/simple"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow", "-i", "https://pypi.tuna.tsinghua.edu.cn/simple"}, args) }) t.Run("sync args with verbose flag", func(t *testing.T) { @@ -196,7 +196,7 @@ func TestPrepareSyncArgs(t *testing.T) { } args := runtime.prepareSyncArgs(false) - require.Equal(t, []string{"sync", "--no-dev", "-v"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow", "-v"}, args) }) t.Run("sync args with extra args", func(t *testing.T) { @@ -207,7 +207,7 @@ func TestPrepareSyncArgs(t *testing.T) { } args := runtime.prepareSyncArgs(false) - require.Equal(t, []string{"sync", "--no-dev", "--no-cache", "--retries", "3"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow", "--no-cache", "--retries", "3"}, args) }) t.Run("sync args with all config options", func(t *testing.T) { @@ -223,6 +223,7 @@ func TestPrepareSyncArgs(t *testing.T) { require.Equal(t, []string{ "sync", "--no-dev", + "--prerelease=allow", "-i", "https://pypi.tuna.tsinghua.edu.cn/simple", "-v", "--no-cache", @@ -235,7 +236,7 @@ func TestPrepareSyncArgs(t *testing.T) { } args := runtime.prepareSyncArgs(true) - require.Equal(t, []string{"sync", "--no-dev", "--frozen"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow", "--frozen"}, args) }) t.Run("sync args with uv.lock deduplicates --frozen from extra args", func(t *testing.T) { @@ -246,7 +247,7 @@ func TestPrepareSyncArgs(t *testing.T) { } args := runtime.prepareSyncArgs(true) - require.Equal(t, []string{"sync", "--no-dev", "--frozen", "--no-cache"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow", "--frozen", "--no-cache"}, args) }) t.Run("sync args without uv.lock keeps --frozen from extra args", func(t *testing.T) { @@ -257,7 +258,18 @@ func TestPrepareSyncArgs(t *testing.T) { } args := runtime.prepareSyncArgs(false) - require.Equal(t, []string{"sync", "--no-dev", "--frozen", "--no-cache"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow", "--frozen", "--no-cache"}, args) + }) + + t.Run("sync args deduplicate prerelease flag from extra args", func(t *testing.T) { + runtime := &LocalPluginRuntime{ + appConfig: &app.Config{ + PipExtraArgs: "--prerelease=allow --no-cache", + }, + } + + args := runtime.prepareSyncArgs(false) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow", "--no-cache"}, args) }) } @@ -287,7 +299,7 @@ func TestInstallDependenciesIgnoreUvLock(t *testing.T) { // hasUvLock should remain false, so --frozen is NOT added require.False(t, hasUvLock) args := runtime.prepareSyncArgs(hasUvLock) - require.Equal(t, []string{"sync", "--no-dev"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow"}, args) }) t.Run("--frozen is added when PluginIgnoreUvLock is false", func(t *testing.T) { @@ -313,7 +325,7 @@ func TestInstallDependenciesIgnoreUvLock(t *testing.T) { // hasUvLock should be true, so --frozen IS added require.True(t, hasUvLock) args := runtime.prepareSyncArgs(hasUvLock) - require.Equal(t, []string{"sync", "--no-dev", "--frozen"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow", "--frozen"}, args) }) t.Run("ignore flag with mirror URL produces correct args", func(t *testing.T) { @@ -326,7 +338,7 @@ func TestInstallDependenciesIgnoreUvLock(t *testing.T) { // When uv.lock is ignored, hasUvLock=false, so no --frozen args := runtime.prepareSyncArgs(false) - require.Equal(t, []string{"sync", "--no-dev", "-i", "https://mirrors.example.com/simple"}, args) + require.Equal(t, []string{"sync", "--no-dev", "--prerelease=allow", "-i", "https://mirrors.example.com/simple"}, args) }) } @@ -337,7 +349,7 @@ func TestPreparePipArgs(t *testing.T) { } args := runtime.preparePipArgs() - require.Equal(t, []string{"pip", "install", "-r", "requirements.txt"}, args) + require.Equal(t, []string{"pip", "install", "--prerelease=allow", "-r", "requirements.txt"}, args) }) t.Run("pip args with mirror URL", func(t *testing.T) { @@ -351,6 +363,7 @@ func TestPreparePipArgs(t *testing.T) { require.Equal(t, []string{ "pip", "install", + "--prerelease=allow", "-i", "https://pypi.tuna.tsinghua.edu.cn/simple", "-r", "requirements.txt", }, args) @@ -364,7 +377,7 @@ func TestPreparePipArgs(t *testing.T) { } args := runtime.preparePipArgs() - require.Equal(t, []string{"pip", "install", "-r", "requirements.txt", "-vvv"}, args) + require.Equal(t, []string{"pip", "install", "--prerelease=allow", "-r", "requirements.txt", "-vvv"}, args) }) t.Run("pip args with all config options", func(t *testing.T) { @@ -380,12 +393,24 @@ func TestPreparePipArgs(t *testing.T) { require.Equal(t, []string{ "pip", "install", + "--prerelease=allow", "-i", "https://pypi.tuna.tsinghua.edu.cn/simple", "-r", "requirements.txt", "-vvv", "--no-cache", }, args) }) + + t.Run("pip args deduplicate prerelease flag from extra args", func(t *testing.T) { + runtime := &LocalPluginRuntime{ + appConfig: &app.Config{ + PipExtraArgs: "--prerelease=allow --no-cache", + }, + } + + args := runtime.preparePipArgs() + require.Equal(t, []string{"pip", "install", "--prerelease=allow", "-r", "requirements.txt", "--no-cache"}, args) + }) } func TestGetPluginSdkVersionWithPyprojectToml(t *testing.T) { diff --git a/internal/core/local_runtime/setup_python_environment.go b/internal/core/local_runtime/setup_python_environment.go index a27da50e1..f5efbc24c 100644 --- a/internal/core/local_runtime/setup_python_environment.go +++ b/internal/core/local_runtime/setup_python_environment.go @@ -24,6 +24,8 @@ import ( "go.opentelemetry.io/otel/trace" ) +const uvAllowPrereleaseArg = "--prerelease=allow" + // tracing helpers func (p *LocalPluginRuntime) otelTracer() trace.Tracer { return gootel.Tracer("dify-plugin-daemon/python") @@ -73,7 +75,7 @@ func (p *LocalPluginRuntime) prepareUV() (string, error) { } func (p *LocalPluginRuntime) preparePipArgs() []string { - args := []string{"install"} + args := []string{"install", uvAllowPrereleaseArg} if p.appConfig.PipMirrorUrl != "" { args = append(args, "-i", p.appConfig.PipMirrorUrl) @@ -85,7 +87,8 @@ func (p *LocalPluginRuntime) preparePipArgs() []string { args = append(args, "-vvv") } - args = append(args, p.parseExtraArgs()...) + extraArgs := p.deduplicateArgs(p.parseExtraArgs(), uvAllowPrereleaseArg) + args = append(args, extraArgs...) args = append([]string{"pip"}, args...) @@ -93,7 +96,7 @@ func (p *LocalPluginRuntime) preparePipArgs() []string { } func (p *LocalPluginRuntime) prepareSyncArgs(hasUvLock bool) []string { - args := []string{"sync", "--no-dev"} + args := []string{"sync", "--no-dev", uvAllowPrereleaseArg} if hasUvLock { args = append(args, "--frozen") @@ -111,6 +114,7 @@ func (p *LocalPluginRuntime) prepareSyncArgs(hasUvLock bool) []string { if hasUvLock { extraArgs = p.deduplicateArgs(extraArgs, "--frozen") } + extraArgs = p.deduplicateArgs(extraArgs, uvAllowPrereleaseArg) args = append(args, extraArgs...) return args } diff --git a/internal/core/plugin_manager/runtime.go b/internal/core/plugin_manager/runtime.go index adba03e52..0ab0a3ee8 100644 --- a/internal/core/plugin_manager/runtime.go +++ b/internal/core/plugin_manager/runtime.go @@ -33,6 +33,12 @@ func (p *PluginManager) RemoveLocalPlugin( return p.controlPanel.RemoveLocalPlugin(pluginUniqueIdentifier) } +func (p *PluginManager) RemoveLocalPluginStorage( + pluginUniqueIdentifier plugin_entities.PluginUniqueIdentifier, +) error { + return p.controlPanel.RemoveLocalPluginStorage(pluginUniqueIdentifier) +} + // get local plugin runtime func (p *PluginManager) GetLocalPluginRuntime( pluginUniqueIdentifier plugin_entities.PluginUniqueIdentifier, diff --git a/internal/service/install_plugin.go b/internal/service/install_plugin.go index e4df027a6..580bf3a91 100644 --- a/internal/service/install_plugin.go +++ b/internal/service/install_plugin.go @@ -373,7 +373,7 @@ func UninstallPlugin( } } - if deleteResponse != nil && deleteResponse.IsPluginDeleted && deleteResponse.Plugin != nil && deleteResponse.Plugin.InstallType == plugin_entities.PLUGIN_RUNTIME_TYPE_LOCAL { + if shouldRemoveLocalPlugin(deleteResponse) { manager := plugin_manager.Manager() if manager == nil { return exception.InternalServerError(errors.New("plugin manager is not initialized")).ToResponse() @@ -385,7 +385,7 @@ func UninstallPlugin( shutdownCh, err := manager.ShutdownLocalPluginGracefully(pluginUniqueIdentifier) if errors.Is(err, controlpanel.ErrLocalPluginRuntimeNotFound) { - return entities.NewSuccessResponse(true) + shutdownCh = nil } else if err != nil { return exception.InternalServerError(err).ToResponse() } @@ -393,11 +393,30 @@ func UninstallPlugin( if err := waitGracefulShutdown(shutdownCh); err != nil { return exception.InternalServerError(err).ToResponse() } + + if err := manager.RemoveLocalPluginStorage(pluginUniqueIdentifier); err != nil { + return exception.InternalServerError(err).ToResponse() + } } return entities.NewSuccessResponse(true) } +func shouldRemoveLocalPlugin(deleteResponse *curd.DeletePluginResponse) bool { + if deleteResponse == nil || !deleteResponse.IsPluginDeleted { + return false + } + + runtimeType := plugin_entities.PluginRuntimeType("") + if deleteResponse.Plugin != nil { + runtimeType = deleteResponse.Plugin.InstallType + } else if deleteResponse.Installation != nil { + runtimeType = plugin_entities.PluginRuntimeType(deleteResponse.Installation.RuntimeType) + } + + return runtimeType == plugin_entities.PLUGIN_RUNTIME_TYPE_LOCAL +} + func waitGracefulShutdown(ch <-chan error) error { if ch == nil { return nil diff --git a/internal/service/install_plugin_test.go b/internal/service/install_plugin_test.go index a329c7bf1..d6fa6333d 100644 --- a/internal/service/install_plugin_test.go +++ b/internal/service/install_plugin_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/langgenius/dify-plugin-daemon/internal/types/app" + "github.com/langgenius/dify-plugin-daemon/internal/types/models" + "github.com/langgenius/dify-plugin-daemon/internal/types/models/curd" "github.com/langgenius/dify-plugin-daemon/pkg/entities/plugin_entities" ) @@ -24,23 +26,23 @@ func TestUpgradePlugin(t *testing.T) { } tests := []struct { - name string - tenantId string - source string - meta map[string]any - originalPluginUniqueIdentifier plugin_entities.PluginUniqueIdentifier - newPluginUniqueIdentifier plugin_entities.PluginUniqueIdentifier - wantSuccess bool - wantAllInstalled bool - wantTaskIDEmpty bool + name string + tenantId string + source string + meta map[string]any + originalPluginUniqueIdentifier plugin_entities.PluginUniqueIdentifier + newPluginUniqueIdentifier plugin_entities.PluginUniqueIdentifier + wantSuccess bool + wantAllInstalled bool + wantTaskIDEmpty bool }{ { name: "same plugin identifiers", tenantId: "tenant-123", source: "test", meta: map[string]any{}, - originalPluginUniqueIdentifier: originalIdentifier, - newPluginUniqueIdentifier: originalIdentifier, + originalPluginUniqueIdentifier: originalIdentifier, + newPluginUniqueIdentifier: originalIdentifier, wantSuccess: false, }, { @@ -48,8 +50,8 @@ func TestUpgradePlugin(t *testing.T) { tenantId: "tenant-123", source: "test", meta: map[string]any{}, - originalPluginUniqueIdentifier: originalIdentifier, - newPluginUniqueIdentifier: newIdentifier, + originalPluginUniqueIdentifier: originalIdentifier, + newPluginUniqueIdentifier: newIdentifier, wantSuccess: false, }, } @@ -86,3 +88,69 @@ func TestUpgradePlugin(t *testing.T) { }) } } + +func TestShouldRemoveLocalPlugin(t *testing.T) { + tests := []struct { + name string + deleteResponse *curd.DeletePluginResponse + want bool + }{ + { + name: "local plugin deleted with plugin row", + deleteResponse: &curd.DeletePluginResponse{ + Plugin: &models.Plugin{ + InstallType: plugin_entities.PLUGIN_RUNTIME_TYPE_LOCAL, + }, + IsPluginDeleted: true, + }, + want: true, + }, + { + name: "local plugin deleted with missing plugin row", + deleteResponse: &curd.DeletePluginResponse{ + Installation: &models.PluginInstallation{ + RuntimeType: string(plugin_entities.PLUGIN_RUNTIME_TYPE_LOCAL), + }, + IsPluginDeleted: true, + }, + want: true, + }, + { + name: "serverless plugin deleted", + deleteResponse: &curd.DeletePluginResponse{ + Installation: &models.PluginInstallation{ + RuntimeType: string(plugin_entities.PLUGIN_RUNTIME_TYPE_SERVERLESS), + }, + IsPluginDeleted: true, + }, + want: false, + }, + { + name: "plugin still referenced", + deleteResponse: &curd.DeletePluginResponse{ + Plugin: &models.Plugin{ + InstallType: plugin_entities.PLUGIN_RUNTIME_TYPE_LOCAL, + }, + Installation: &models.PluginInstallation{ + RuntimeType: string(plugin_entities.PLUGIN_RUNTIME_TYPE_LOCAL), + }, + IsPluginDeleted: false, + }, + want: false, + }, + { + name: "nil response", + deleteResponse: nil, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := shouldRemoveLocalPlugin(tt.deleteResponse) + if got != tt.want { + t.Fatalf("shouldRemoveLocalPlugin() = %v, want %v", got, tt.want) + } + }) + } +}