diff --git a/cmd/platform/run.go b/cmd/platform/run.go index a755a559..feeb06d1 100644 --- a/cmd/platform/run.go +++ b/cmd/platform/run.go @@ -178,11 +178,14 @@ func newRunLogger(clients *shared.ClientFactory, cmd *cobra.Command) *logger.Log case "on_cloud_run_watch_error": message := event.DataToString("cloud_run_watch_error") clients.IO.PrintError(ctx, "Error: %s", message) - case "on_cloud_run_watch_file_change": - path := event.DataToString("cloud_run_watch_file_change") - cmd.Println(style.Secondary(fmt.Sprintf("File change detected: %s, reinstalling app...", path))) - case "on_cloud_run_watch_file_change_reinstalled": + case "on_cloud_run_watch_manifest_change": + path := event.DataToString("cloud_run_watch_manifest_change") + cmd.Println(style.Secondary(fmt.Sprintf("Manifest change detected: %s, reinstalling app...", path))) + case "on_cloud_run_watch_manifest_change_reinstalled": cmd.Println(style.Secondary("App successfully reinstalled")) + case "on_cloud_run_watch_app_change": + path := event.DataToString("cloud_run_watch_app_change") + cmd.Println(style.Secondary(fmt.Sprintf("App change detected: %s, restarting server...", path))) case "on_cleanup_app_install_done": cmd.Println(style.Secondary(fmt.Sprintf( `Cleaned up local app install for "%s".`, diff --git a/docs/reference/hooks/index.md b/docs/reference/hooks/index.md index e525ae41..b41343a7 100644 --- a/docs/reference/hooks/index.md +++ b/docs/reference/hooks/index.md @@ -446,7 +446,17 @@ The format for the JSON representing the CLI-SDK interface is as follows: }, "config": { "protocol-version": ["message-boundaries"], - "sdk-managed-connection-enabled": false + "sdk-managed-connection-enabled": false, + "watch": { + "manifest": { + "paths": ["manifest.json"] + }, + "app": { + "paths": ["app.js", "listeners/"], + "filter-regex": "\\.(ts|js)$" + } + }, + "trigger-paths": ["triggers/"] } } ``` @@ -457,7 +467,7 @@ The format for the JSON representing the CLI-SDK interface is as follows: | hooks | Object whose keys must match the hook names outlined in the above [Hooks Specification](#specification). Arguments can be provided within this string by separating them with spaces. | Required | | config | Object of key-value settings. | Optional | | config.protocol-version | Array of strings representing the named CLI-SDK protocols supported by the SDK, in descending order of support, as in the first element in the array defines the preferred protocol for use by the SDK, the second element defines the next-preferred protocol, and so on. The only supported named protocol currently is `message-boundaries`. The CLI will use the v1 protocol if this field is not provided. | Optional | -| config.watch | Object with configuration settings for file-watching. | Optional | +| config.watch | Object with configuration settings for file-watching during `slack run`. Supports updating the `manifest` on change and reloading the `app` server. Read [Watch configurations](#watch-configurations) for details. | Optional | | config.sdk-managed-connection-enabled | Boolean specifying whether the WebSocket connection between the CLI and Slack should be managed by the CLI or by the SDK during `slack run` executions. If `true`, the SDK will manage this connection. If `false` or not provided, the CLI will manage this connection. | Optional | | config.trigger-paths | Array of strings that are paths to files of trigger definitions. | Optional | @@ -466,6 +476,37 @@ This format must be adhered to, in order of preference, either: 1. As the response to `get-hooks`, or 2. Comprising the contents of the `hooks.json` file +### Watch configurations {#watch-configurations} + +The `config.watch` setting looks for file changes during local development with the `slack run` command. The CLI supports separate file watchers for **manifest** changes and changes to **application code** as options for reinstalling the app or reloading the server. + +```json +{ + "config": { + "watch": { + "manifest": { + "paths": ["manifest.json"] + }, + "app": { + "paths": ["app.js", "listeners/"], + "filter-regex": "\\.(ts|js)$" + } + } + } +} +``` + +| Field | Description | Required | +| --------------------------- | ---------------------------------------------------------------------------------- | -------- | +| watch.manifest | Object configuring the manifest watcher for reinstalling the app. | Optional | +| watch.manifest.paths | Array of file paths or directories to watch for manifest changes. | Required | +| watch.manifest.filter-regex | Regex pattern to filter which files trigger manifest reinstall (e.g., `\\.json$`). | Optional | +| watch.app | Object configuring the app watcher for restarting the app server. | Optional | +| watch.app.paths | Array of file paths or directories to watch for app/code changes. | Required | +| watch.app.filter-regex | Regex pattern to filter which files trigger server reload (e.g., `\\.(ts\|js)$`). | Optional | + +**Note:** For backward compatibility, top-level `paths` and `filter-regex` fields are treated as manifest watching configuration only. No server reloading will occur with the legacy structure. + ## Hook resolution {#hook-resolution} The CLI will employ the following algorithm in order to resolve the command to be executed for a particular hook: @@ -516,8 +557,13 @@ The CLI will employ the following algorithm in order to resolve the command to b }, "config": { "watch": { - "filter-regex": "^manifest\\.(ts|js|json)$", - "paths": ["."] + "manifest": { + "paths": ["manifest.json"] + }, + "app": { + "paths": ["app.js", "listeners/"], + "filter-regex": "\\.(ts|js)$" + } }, "sdk-managed-connection-enabled": "true" } @@ -543,6 +589,8 @@ The CLI will employ the following algorithm in order to resolve the command to b } ``` +**Note:** The legacy format (top-level `paths` and `filter-regex`) is treated as manifest watching only. No server reloading will occur with this configuration. + ## Terms {#terms} ### Types of developers diff --git a/internal/hooks/sdk_config.go b/internal/hooks/sdk_config.go index 6bd2d830..94419d08 100644 --- a/internal/hooks/sdk_config.go +++ b/internal/hooks/sdk_config.go @@ -15,6 +15,7 @@ package hooks import ( + "fmt" "strings" "github.com/slackapi/slack-cli/internal/slackerror" @@ -67,7 +68,22 @@ func (pv ProtocolVersions) Preferred() Protocol { return HookProtocolDefault } +// WatchOpts contains details of file watcher configurations type WatchOpts struct { + Manifest *ManifestWatchOpts `json:"manifest,omitempty"` + App *AppWatchOpts `json:"app,omitempty"` + FilterRegex string `json:"filter-regex,omitempty"` // Legacy for manifest watching + Paths []string `json:"paths,omitempty"` // Legacy for manifest watching +} + +// ManifestWatchOpts configures watching for manifest changes for reinstall +type ManifestWatchOpts struct { + FilterRegex string `json:"filter-regex,omitempty"` + Paths []string `json:"paths,omitempty"` +} + +// AppWatchOpts configures watching for app/code changes for server restart +type AppWatchOpts struct { FilterRegex string `json:"filter-regex,omitempty"` Paths []string `json:"paths,omitempty"` } @@ -76,3 +92,54 @@ type WatchOpts struct { func (w *WatchOpts) IsAvailable() bool { return w != nil } + +// GetManifestWatchConfig returns manifest watch config +func (w *WatchOpts) GetManifestWatchConfig() (paths []string, filterRegex string, enabled bool) { + if w == nil { + return nil, "", false + } + if w.Manifest != nil { + return w.Manifest.Paths, w.Manifest.FilterRegex, len(w.Manifest.Paths) > 0 + } + // Backward compatibility: top-level paths/filter-regex for manifest watching + return w.Paths, w.FilterRegex, len(w.Paths) > 0 +} + +// GetAppWatchConfig returns app watch config +func (w *WatchOpts) GetAppWatchConfig() (paths []string, filterRegex string, enabled bool) { + if w == nil { + return nil, "", false + } + if w.App != nil { + return w.App.Paths, w.App.FilterRegex, len(w.App.Paths) > 0 + } + return nil, "", false +} + +// String formats WatchOpts for debug outputs +func (w WatchOpts) String() string { + var parts []string + if w.Manifest != nil { + parts = append(parts, fmt.Sprintf("Manifest:%s", w.Manifest.String())) + } else if len(w.Paths) > 0 || w.FilterRegex != "" { + parts = append(parts, fmt.Sprintf("Paths:%v", w.Paths)) + parts = append(parts, fmt.Sprintf("FilterRegex:%s", w.FilterRegex)) + } + if w.App != nil { + parts = append(parts, fmt.Sprintf("App:%s", w.App.String())) + } + if len(parts) == 0 { + return "{}" + } + return fmt.Sprintf("{%s}", strings.Join(parts, " ")) +} + +// String formats ManifestWatchOpts for debug outputs +func (m ManifestWatchOpts) String() string { + return fmt.Sprintf("{Paths:%v FilterRegex:%s}", m.Paths, m.FilterRegex) +} + +// String formats AppWatchOpts for debug outputs +func (a AppWatchOpts) String() string { + return fmt.Sprintf("{Paths:%v FilterRegex:%s}", a.Paths, a.FilterRegex) +} diff --git a/internal/hooks/sdk_config_test.go b/internal/hooks/sdk_config_test.go index 70aedf00..25b95402 100644 --- a/internal/hooks/sdk_config_test.go +++ b/internal/hooks/sdk_config_test.go @@ -121,3 +121,194 @@ func Test_WatchOpts_IsAvailable(t *testing.T) { }) } } + +func Test_WatchOpts_GetManifestWatchConfig(t *testing.T) { + tests := map[string]struct { + watchOpts *WatchOpts + expectedPaths []string + expectedRegex string + expectedEnabled bool + }{ + "Nil WatchOpts pointer": { + watchOpts: nil, + expectedPaths: nil, + expectedRegex: "", + expectedEnabled: false, + }, + "Nested manifest config": { + watchOpts: &WatchOpts{ + Manifest: &ManifestWatchOpts{ + Paths: []string{"manifest.json", "workflows/"}, + FilterRegex: "\\.json$", + }, + }, + expectedPaths: []string{"manifest.json", "workflows/"}, + expectedRegex: "\\.json$", + expectedEnabled: true, + }, + "Legacy flat config": { + watchOpts: &WatchOpts{ + Paths: []string{"manifest.json", "src/"}, + FilterRegex: "\\.(json|ts)$", + }, + expectedPaths: []string{"manifest.json", "src/"}, + expectedRegex: "\\.(json|ts)$", + expectedEnabled: true, + }, + "Nested config takes precedence over legacy": { + watchOpts: &WatchOpts{ + Paths: []string{"old-path/"}, + FilterRegex: "old-regex", + Manifest: &ManifestWatchOpts{ + Paths: []string{"new-path/"}, + FilterRegex: "new-regex", + }, + }, + expectedPaths: []string{"new-path/"}, + expectedRegex: "new-regex", + expectedEnabled: true, + }, + "Empty nested manifest config": { + watchOpts: &WatchOpts{ + Manifest: &ManifestWatchOpts{ + Paths: []string{}, + }, + }, + expectedPaths: []string{}, + expectedRegex: "", + expectedEnabled: false, + }, + "Empty legacy config": { + watchOpts: &WatchOpts{ + Paths: []string{}, + }, + expectedPaths: []string{}, + expectedRegex: "", + expectedEnabled: false, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + paths, regex, enabled := tt.watchOpts.GetManifestWatchConfig() + assert.Equal(t, tt.expectedPaths, paths) + assert.Equal(t, tt.expectedRegex, regex) + assert.Equal(t, tt.expectedEnabled, enabled) + }) + } +} + +func Test_WatchOpts_GetAppWatchConfig(t *testing.T) { + tests := map[string]struct { + watchOpts *WatchOpts + expectedPaths []string + expectedRegex string + expectedEnabled bool + }{ + "Nil WatchOpts pointer": { + watchOpts: nil, + expectedPaths: nil, + expectedRegex: "", + expectedEnabled: false, + }, + "Nested app config": { + watchOpts: &WatchOpts{ + App: &AppWatchOpts{ + Paths: []string{"src/", "functions/"}, + FilterRegex: "\\.(ts|js)$", + }, + }, + expectedPaths: []string{"src/", "functions/"}, + expectedRegex: "\\.(ts|js)$", + expectedEnabled: true, + }, + "Legacy config does not enable app watching": { + watchOpts: &WatchOpts{ + Paths: []string{"manifest.json", "src/"}, + FilterRegex: "\\.(json|ts)$", + }, + expectedPaths: nil, + expectedRegex: "", + expectedEnabled: false, + }, + "Empty nested app config": { + watchOpts: &WatchOpts{ + App: &AppWatchOpts{ + Paths: []string{}, + }, + }, + expectedPaths: []string{}, + expectedRegex: "", + expectedEnabled: false, + }, + "Nil app config": { + watchOpts: &WatchOpts{}, + expectedPaths: nil, + expectedRegex: "", + expectedEnabled: false, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + paths, regex, enabled := tt.watchOpts.GetAppWatchConfig() + assert.Equal(t, tt.expectedPaths, paths) + assert.Equal(t, tt.expectedRegex, regex) + assert.Equal(t, tt.expectedEnabled, enabled) + }) + } +} + +func Test_WatchOpts_String(t *testing.T) { + tests := map[string]struct { + watchOpts WatchOpts + expectedString string + }{ + "Nested config with both manifest and app": { + watchOpts: WatchOpts{ + Manifest: &ManifestWatchOpts{ + Paths: []string{"manifest.json"}, + FilterRegex: "\\.json$", + }, + App: &AppWatchOpts{ + Paths: []string{"src/", "functions/"}, + FilterRegex: "\\.(ts|js)$", + }, + }, + expectedString: "{Manifest:{Paths:[manifest.json] FilterRegex:\\.json$} App:{Paths:[src/ functions/] FilterRegex:\\.(ts|js)$}}", + }, + "Nested manifest only": { + watchOpts: WatchOpts{ + Manifest: &ManifestWatchOpts{ + Paths: []string{"manifest.json"}, + FilterRegex: "\\.json$", + }, + }, + expectedString: "{Manifest:{Paths:[manifest.json] FilterRegex:\\.json$}}", + }, + "Nested app only": { + watchOpts: WatchOpts{ + App: &AppWatchOpts{ + Paths: []string{"src/"}, + FilterRegex: "\\.(ts|js)$", + }, + }, + expectedString: "{App:{Paths:[src/] FilterRegex:\\.(ts|js)$}}", + }, + "Legacy config": { + watchOpts: WatchOpts{ + Paths: []string{"manifest.json", "src/"}, + FilterRegex: "\\.(json|ts)$", + }, + expectedString: "{Paths:[manifest.json src/] FilterRegex:\\.(json|ts)$}", + }, + "Empty config": { + watchOpts: WatchOpts{}, + expectedString: "{}", + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + result := tt.watchOpts.String() + assert.Equal(t, tt.expectedString, result) + }) + } +} diff --git a/internal/hooks/shell.go b/internal/hooks/shell.go index e88b1fc0..b0c23a2c 100644 --- a/internal/hooks/shell.go +++ b/internal/hooks/shell.go @@ -35,6 +35,7 @@ type ShellCommand interface { StderrPipe() (io.ReadCloser, error) Start() error Wait() error + GetProcess() *os.Process } type ShellExec struct{} @@ -73,6 +74,14 @@ type execCommander struct { *exec.Cmd } +// GetProcess returns the underlying process +func (e execCommander) GetProcess() *os.Process { + if e.Cmd != nil { + return e.Process + } + return nil +} + type HookExecOpts struct { Directory string Hook HookScript diff --git a/internal/hooks/shell_mock.go b/internal/hooks/shell_mock.go index a79201fe..ca002ecb 100644 --- a/internal/hooks/shell_mock.go +++ b/internal/hooks/shell_mock.go @@ -17,6 +17,7 @@ package hooks import ( "context" "io" + "os" "github.com/stretchr/testify/mock" ) @@ -80,6 +81,10 @@ func (c *MockCommand) CombinedOutput() ([]byte, error) { return c.MockStdout, c.Err } +func (c *MockCommand) GetProcess() *os.Process { + return nil +} + type MockHookExecutor struct { mock.Mock } diff --git a/internal/pkg/platform/localserver.go b/internal/pkg/platform/localserver.go index 21c83031..693d5843 100644 --- a/internal/pkg/platform/localserver.go +++ b/internal/pkg/platform/localserver.go @@ -24,6 +24,7 @@ import ( "os/exec" "regexp" "strings" + "sync" "time" "github.com/gorilla/websocket" @@ -82,6 +83,8 @@ type LocalServer struct { localHostedContext LocalHostedContext cliConfig hooks.SDKCLIConfig Connection WebSocketConnection + delegateCmd hooks.ShellCommand // track running delegated process + delegateCmdMutex sync.Mutex // protect concurrent access } // Start establishes a socket connection to Slack, which will receive app-relevant events. It does so in a loop to support for re-establishing the socket connection. @@ -243,6 +246,39 @@ func (r *LocalServer) Listen(ctx context.Context, errChan chan<- error, done cha } } +// stopDelegateProcess terminates the currently running delegated process if one exists +func (r *LocalServer) stopDelegateProcess(ctx context.Context) { + r.delegateCmdMutex.Lock() + defer r.delegateCmdMutex.Unlock() + + if r.delegateCmd != nil { + process := r.delegateCmd.GetProcess() + if process != nil { + r.clients.IO.PrintDebug(ctx, "Stopping previous delegated process (PID: %d)", process.Pid) + // Kill the process gracefully + err := process.Signal(os.Interrupt) + if err != nil { + // If interrupt fails, force kill + r.clients.IO.PrintDebug(ctx, "Failed to interrupt process, sending SIGKILL: %v", err) + _ = process.Kill() + } + // Wait for process to exit (with timeout) + done := make(chan error) + go func() { + done <- r.delegateCmd.Wait() + }() + select { + case <-done: + r.clients.IO.PrintDebug(ctx, "Previous process stopped successfully") + case <-time.After(5 * time.Second): + r.clients.IO.PrintDebug(ctx, "Process did not exit in time, force killing") + _ = process.Kill() + } + } + r.delegateCmd = nil + } +} + // StartDelegate passes along required opts to SDK, delegating // connection for running app locally to script hook start func (r *LocalServer) StartDelegate(ctx context.Context) error { @@ -276,8 +312,20 @@ func (r *LocalServer) StartDelegate(ctx context.Context) error { var cmdEnvVars = os.Environ() cmdEnvVars = append(cmdEnvVars, goutils.MapToStringSlice(sdkManagedConnectionStartHookOpts.Env, "")...) cmd := sdkManagedConnectionStartHookOpts.Exec.Command(cmdEnvVars, os.Stdout, os.Stderr, nil, cmdArgs[0], cmdArgVars...) + + // Store command reference for lifecycle management + r.delegateCmdMutex.Lock() + r.delegateCmd = cmd + r.delegateCmdMutex.Unlock() + + // Start the process (non-blocking) + if err := cmd.Start(); err != nil { + return slackerror.Wrap(err, slackerror.ErrSDKHookInvocationFailed). + WithMessage("Failed to start 'start' hook") + } + // The following command will block, as the expectation is that SDK-delegated local-run invokes a long-running (blocking) child process - err = cmd.Run() + err = cmd.Wait() if err != nil { if status, ok := err.(*exec.ExitError); ok { switch status.ExitCode() { @@ -301,52 +349,75 @@ func (r *LocalServer) StartDelegate(ctx context.Context) error { return nil } -// Watch for file changes. If configuration for watch is provided -// The CLI will watch for a file changes. To watch specific changes -// provide additional filter regex. -func (r *LocalServer) Watch(ctx context.Context, auth types.SlackAuth, app types.App) error { - // Check for watch SDKCLI configuration +// WatchManifest watches for manifest file changes and triggers app reinstallation +func (r *LocalServer) WatchManifest(ctx context.Context, auth types.SlackAuth, app types.App) error { + // Check for watch SDK CLI configuration if !r.cliConfig.Config.Watch.IsAvailable() { r.clients.IO.PrintDebug(ctx, "To watch file changes, provide watch configuration in %s", config.GetProjectHooksJSONFilePath()) + // Block until context is cancelled to keep the goroutine alive + <-ctx.Done() return nil } + + // Get manifest source to determine if we should watch for local manifest changes + manifestSource, err := r.clients.Config.ProjectConfig.GetManifestSource(ctx) + if err != nil { + return err + } + + // Skip manifest watching if manifest source is remote + if manifestSource.Equals(config.ManifestSourceRemote) { + r.clients.IO.PrintDebug(ctx, "Manifest watching disabled: manifest.source is set to remote") + // Block until context is cancelled to keep the goroutine alive + <-ctx.Done() + return nil + } + + // Get manifest watch configuration + paths, filterRegex, enabled := r.cliConfig.Config.Watch.GetManifestWatchConfig() + if !enabled { + r.clients.IO.PrintDebug(ctx, "Manifest watching is not enabled") + // Block until context is cancelled to keep the goroutine alive + <-ctx.Done() + return nil + } + // Init watcher w := watcher.New() w.SetMaxEvents(1) w.FilterOps(watcher.Write) // Use SDK-provided filter regex - if r.cliConfig.Config.Watch.FilterRegex != "" { - r.clients.IO.PrintDebug(ctx, fmt.Sprintf("Watching changes to file paths matching: %s", r.cliConfig.Config.Watch.FilterRegex)) - w.AddFilterHook(watcher.RegexFilterHook(regexp.MustCompile(r.cliConfig.Config.Watch.FilterRegex), false)) + if filterRegex != "" { + r.clients.IO.PrintDebug(ctx, "Watching manifest changes to file paths matching: %s", filterRegex) + w.AddFilterHook(watcher.RegexFilterHook(regexp.MustCompile(filterRegex), false)) } // Add provided paths to watcher - for _, path := range r.cliConfig.Config.Watch.Paths { + for _, path := range paths { if err := w.AddRecursive(path); err != nil { r.log.Data["cloud_run_watch_error"] = fmt.Sprintf("manifest_watcher.paths: %s", err) r.log.Warn("on_cloud_run_watch_error") - // TODO: should this prevent further execution? If so, return the err } } - // Begin watching for file changes + // Begin watching for manifest file changes go func() { for { select { case <-ctx.Done(): - r.clients.IO.PrintDebug(ctx, "File watcher context canceled, returning.") + r.clients.IO.PrintDebug(ctx, "Manifest file watcher context canceled, returning.") return case event := <-w.Event: - r.log.Data["cloud_run_watch_file_change"] = event.Path - r.log.Info("on_cloud_run_watch_file_change") + r.log.Data["cloud_run_watch_manifest_change"] = event.Path + r.log.Info("on_cloud_run_watch_manifest_change") + // Reinstall the app when manifest changes if _, _, _, err := apps.InstallLocalApp(ctx, r.clients, "", r.log, auth, app); err != nil { - // The install command will have handled printing the error r.log.Data["cloud_run_watch_error"] = err.Error() r.log.Warn("on_cloud_run_watch_error") } else { - r.log.Info("on_cloud_run_watch_file_change_reinstalled") + r.log.Info("on_cloud_run_watch_manifest_change_reinstalled") } case err := <-w.Error: r.log.Data["cloud_run_watch_error"] = err.Error() @@ -360,6 +431,94 @@ func (r *LocalServer) Watch(ctx context.Context, auth types.SlackAuth, app types return w.Start(time.Millisecond * 100) } +// WatchApp starts the delegated server and watches for app/code file changes to trigger restarts (SDK-managed connections only) +func (r *LocalServer) WatchApp(ctx context.Context) error { + // Only run for SDK-managed connections + if !r.cliConfig.Config.SDKManagedConnection { + r.clients.IO.PrintDebug(ctx, "App watching is only enabled for SDK-managed connections") + return nil + } + + // Check for watch SDKCLI configuration + watchAvailable := r.cliConfig.Config.Watch.IsAvailable() + paths, filterRegex, watchEnabled := []string{}, "", false + if watchAvailable { + paths, filterRegex, watchEnabled = r.cliConfig.Config.Watch.GetAppWatchConfig() + } + + // Start the initial delegated server process + r.clients.IO.PrintDebug(ctx, "Starting initial delegated server process") + serverErrChan := make(chan error, 1) + go func() { + err := r.StartDelegate(ctx) + serverErrChan <- err + }() + + // If watch is not configured or not enabled, just wait for the server to exit + if !watchAvailable || !watchEnabled { + r.clients.IO.PrintDebug(ctx, "App file watching is not enabled, running server without restart capability") + return <-serverErrChan + } + + // Init watcher for restarts + w := watcher.New() + w.SetMaxEvents(1) + w.FilterOps(watcher.Write) + + // Use SDK-provided filter regex + if filterRegex != "" { + r.clients.IO.PrintDebug(ctx, "Watching app changes to file paths matching: %s", filterRegex) + w.AddFilterHook(watcher.RegexFilterHook(regexp.MustCompile(filterRegex), false)) + } + + // Add provided paths to watcher + for _, path := range paths { + if err := w.AddRecursive(path); err != nil { + r.log.Data["cloud_run_watch_error"] = fmt.Sprintf("app_watcher.paths: %s", err) + r.log.Warn("on_cloud_run_watch_error") + } + } + + // Begin watching for app file changes + go func() { + for { + select { + case <-ctx.Done(): + r.clients.IO.PrintDebug(ctx, "App file watcher context canceled, returning.") + return + case event := <-w.Event: + r.log.Data["cloud_run_watch_app_change"] = event.Path + r.log.Info("on_cloud_run_watch_app_change") + + // Stop the previous process before starting a new one + r.stopDelegateProcess(ctx) + + // Start new delegated server process + go func() { + err := r.StartDelegate(ctx) + if err != nil { + r.clients.IO.PrintDebug(ctx, "Delegated start hook failed on restart: %v", err) + return + } + }() + case err := <-w.Error: + r.log.Data["cloud_run_watch_error"] = err.Error() + r.log.Warn("on_cloud_run_watch_error") + case <-w.Closed: + return + } + } + }() + + // Start the watcher and wait for either the watcher or server to error + if err := w.Start(time.Millisecond * 100); err != nil { + return err + } + + // Wait for the initial server to exit (if it does) + return <-serverErrChan +} + func (r *LocalServer) WatchActivityLogs(ctx context.Context, minLevel string) error { // Default minimum log level if strings.TrimSpace(minLevel) == "" { diff --git a/internal/pkg/platform/localserver_test.go b/internal/pkg/platform/localserver_test.go index a4848114..da13af79 100644 --- a/internal/pkg/platform/localserver_test.go +++ b/internal/pkg/platform/localserver_test.go @@ -21,6 +21,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "testing" "github.com/gorilla/websocket" @@ -40,7 +41,7 @@ var WebsocketDialerDial = &websocketDialerDial func Test_LocalServer_Start(t *testing.T) { for name, tt := range map[string]struct { Setup func(t *testing.T, cm *shared.ClientsMock, clients *shared.ClientFactory, conn *WebSocketConnMock) - Test func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) + Test func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) wsHandler func(w http.ResponseWriter, r *http.Request) fakeDialer func(conn *WebSocketConnMock) func(d *websocket.Dialer, urlStr string, requestHeader http.Header) (WebSocketConnection, *http.Response, error) @@ -49,7 +50,7 @@ func Test_LocalServer_Start(t *testing.T) { Setup: func(t *testing.T, cm *shared.ClientsMock, clients *shared.ClientFactory, conn *WebSocketConnMock) { cm.API.On("ConnectionsOpen", mock.Anything, mock.Anything).Return(api.AppsConnectionsOpenResult{}, slackerror.New("no can do, pipes are clogged")) }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { require.ErrorContains(t, server.Start(ctx), "pipes are clogged") }, }, @@ -57,7 +58,7 @@ func Test_LocalServer_Start(t *testing.T) { wsHandler: func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(500) }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { require.ErrorContains(t, server.Start(ctx), "bad handshake") }, }, @@ -70,7 +71,7 @@ func Test_LocalServer_Start(t *testing.T) { return conn, nil, nil } }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { require.ErrorContains(t, server.Start(ctx), "oh no") }, }, @@ -84,7 +85,7 @@ func Test_LocalServer_Start(t *testing.T) { return conn, nil, nil } }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { require.ErrorContains(t, server.Start(ctx), slackerror.ErrLocalAppRunCleanExit) // Once to re-establish post-disconnect message and once when close message received conn.AssertNumberOfCalls(t, "Close", 2) @@ -103,7 +104,7 @@ func Test_LocalServer_Start(t *testing.T) { return conn, nil, nil } }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { require.ErrorContains(t, server.Start(ctx), slackerror.ErrLocalAppRunCleanExit) // Expectation is each WS message we configured in Setup // would cause the TCP connection to be closed (3 times) @@ -149,6 +150,8 @@ func Test_LocalServer_Start(t *testing.T) { localContext, clients.SDKConfig, conn, + nil, + sync.Mutex{}, } if tt.fakeDialer != nil { orig := *WebsocketDialerDial @@ -158,7 +161,7 @@ func Test_LocalServer_Start(t *testing.T) { }() } - tt.Test(t, ctx, clientsMock, server, conn) + tt.Test(t, ctx, clientsMock, &server, conn) }) } } @@ -166,10 +169,10 @@ func Test_LocalServer_Start(t *testing.T) { func Test_LocalServer_Listen(t *testing.T) { for name, tt := range map[string]struct { Setup func(t *testing.T, cm *shared.ClientsMock, clients *shared.ClientFactory, conn *WebSocketConnMock) - Test func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) + Test func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) }{ "should return and send special clean exit error if context is canceled": { - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { ctx2, cancel := context.WithCancel(ctx) cancel() errChan := make(chan error) @@ -187,7 +190,7 @@ func Test_LocalServer_Listen(t *testing.T) { Setup: func(t *testing.T, cm *shared.ClientsMock, clients *shared.ClientFactory, conn *WebSocketConnMock) { conn.On("ReadMessage").Return(0, []byte{}, slackerror.New("oh no")) }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { errChan := make(chan error) done := make(chan bool) go server.Listen(ctx, errChan, done) @@ -203,7 +206,7 @@ func Test_LocalServer_Listen(t *testing.T) { Setup: func(t *testing.T, cm *shared.ClientsMock, clients *shared.ClientFactory, conn *WebSocketConnMock) { conn.On("ReadMessage").Return(websocket.CloseMessage, []byte{}, &websocket.CloseError{Code: websocket.CloseNormalClosure, Text: "byebye"}) }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { errChan := make(chan error) done := make(chan bool) go server.Listen(ctx, errChan, done) @@ -219,7 +222,7 @@ func Test_LocalServer_Listen(t *testing.T) { Setup: func(t *testing.T, cm *shared.ClientsMock, clients *shared.ClientFactory, conn *WebSocketConnMock) { conn.On("ReadMessage").Return(websocket.TextMessage, []byte("cache_error"), nil) }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { errChan := make(chan error) done := make(chan bool) go server.Listen(ctx, errChan, done) @@ -235,7 +238,7 @@ func Test_LocalServer_Listen(t *testing.T) { Setup: func(t *testing.T, cm *shared.ClientsMock, clients *shared.ClientFactory, conn *WebSocketConnMock) { conn.On("ReadMessage").Return(websocket.TextMessage, []byte("{\"type\":\"disconnect\"}"), nil) }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { errChan := make(chan error) done := make(chan bool) go server.Listen(ctx, errChan, done) @@ -254,7 +257,7 @@ func Test_LocalServer_Listen(t *testing.T) { // TODO: should probably create a hookscript mock instead of doing this. clients.SDKConfig.Hooks.Start = hooks.HookScript{Command: "", Name: "start"} }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { errChan := make(chan error) done := make(chan bool) go server.Listen(ctx, errChan, done) @@ -275,7 +278,7 @@ func Test_LocalServer_Listen(t *testing.T) { conn.On("ReadMessage").Return(websocket.TextMessage, []byte("{\"type\":\"disconnect\"}"), nil).Once() cm.HookExecutor.On("Execute", mock.Anything, mock.Anything).Return("{}", nil) }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { errChan := make(chan error) done := make(chan bool) go server.Listen(ctx, errChan, done) @@ -296,7 +299,7 @@ func Test_LocalServer_Listen(t *testing.T) { conn.On("ReadMessage").Return(websocket.TextMessage, []byte("{\"type\":\"disconnect\"}"), nil).Once() cm.HookExecutor.On("Execute", mock.Anything, mock.Anything).Return("{}", slackerror.New("typescript error, probably")) }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { errChan := make(chan error) done := make(chan bool) go server.Listen(ctx, errChan, done) @@ -317,7 +320,7 @@ func Test_LocalServer_Listen(t *testing.T) { cm.HookExecutor.On("Execute", mock.Anything, mock.Anything).Return("{}", nil) conn.On("WriteMessage", mock.Anything, mock.Anything).Return(slackerror.New("socket pipe severed")) }, - Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server LocalServer, conn *WebSocketConnMock) { + Test: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, server *LocalServer, conn *WebSocketConnMock) { errChan := make(chan error) done := make(chan bool) go server.Listen(ctx, errChan, done) @@ -360,8 +363,10 @@ func Test_LocalServer_Listen(t *testing.T) { localContext, clients.SDKConfig, conn, + nil, + sync.Mutex{}, } - tt.Test(t, ctx, clientsMock, server, conn) + tt.Test(t, ctx, clientsMock, &server, conn) }) } } diff --git a/internal/pkg/platform/run.go b/internal/pkg/platform/run.go index d7501772..4f0f19ed 100644 --- a/internal/pkg/platform/run.go +++ b/internal/pkg/platform/run.go @@ -149,6 +149,7 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, // In turn, should trigger the final cleanup routine below (see deferred function below), which closes the socket connection. // In case this is an SDK-managed run, the next line is a no-op. sendWebSocketCloseControlMessage(ctx, clients, server.Connection) + server.stopDelegateProcess(ctx) clients.IO.PrintTrace(ctx, slacktrace.PlatformRunStop) clients.CleanupWaitGroup.Done() }(runArgs.Cleanup) @@ -174,23 +175,24 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, // Start watching for manifest changes // TODO - reinstalled apps via FS watcher do nothing with new tokens returned - may lead to permission issues / missing events? go func() { - errChan <- server.Watch(ctx, runArgs.Auth, installedApp) + errChan <- server.WatchManifest(ctx, runArgs.Auth, installedApp) }() // Check to see whether the SDK managed connection flag is enabled - // If so Delegate the connection to the SDK otherwise Start connection + // If so start app watcher (which handles initial start + restarts), otherwise start connection if cliConfig.Config.SDKManagedConnection { clients.IO.PrintDebug(ctx, "Delegating connection to SDK managed script hook") - // Delegate connection to hook; this should be a blocking call, as the delegate should be a server, too. + // Start app watcher which handles initial server start and restarts on file changes go func() { - errChan <- server.StartDelegate(ctx) + errChan <- server.WatchApp(ctx) }() } else { - // Listen for messages in a goroutine, and provide an error channel for raising errors and a done channel for signifying clean exit + // Listen for messages in a goroutine, and provide an error channel for raising errors go func() { errChan <- server.Start(ctx) }() } + if err := <-errChan; err != nil { switch slackerror.ToSlackError(err).Code { case slackerror.ErrLocalAppRunCleanExit: