From 04e414a4d21d2bcb6be7ec40d5f1181bada08d5c Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Thu, 22 Jan 2026 20:41:56 -0800 Subject: [PATCH 1/8] feat: restart app connections on file changes --- internal/pkg/platform/localserver.go | 20 ++++++++++++++++++++ internal/pkg/platform/run.go | 15 --------------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/internal/pkg/platform/localserver.go b/internal/pkg/platform/localserver.go index 21c83031..250d5d4c 100644 --- a/internal/pkg/platform/localserver.go +++ b/internal/pkg/platform/localserver.go @@ -341,6 +341,26 @@ func (r *LocalServer) Watch(ctx context.Context, auth types.SlackAuth, app types r.log.Data["cloud_run_watch_file_change"] = event.Path r.log.Info("on_cloud_run_watch_file_change") + // Check to see whether the SDK managed connection flag is enabled + // If so Delegate the connection to the SDK otherwise Start connection + if r.cliConfig.Config.SDKManagedConnection { + r.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. + go func() { + //errChan <- + err := r.StartDelegate(ctx) + if err != nil { + return // or error event? + } + }() + } else { + r.log.Info("on_cloud_run_watch_file_change") // TODO: DELETE THIS LINE BEFORE COMMIT - its for branching + // Listen for messages in a goroutine, and provide an error channel for raising errors and a done channel for signifying clean exit + // go func() { + // errChan <- r.Start(ctx) + // }() + } + 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() diff --git a/internal/pkg/platform/run.go b/internal/pkg/platform/run.go index d7501772..d904b246 100644 --- a/internal/pkg/platform/run.go +++ b/internal/pkg/platform/run.go @@ -176,21 +176,6 @@ func Run(ctx context.Context, clients *shared.ClientFactory, log *logger.Logger, go func() { errChan <- server.Watch(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 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. - go func() { - errChan <- server.StartDelegate(ctx) - }() - } else { - // Listen for messages in a goroutine, and provide an error channel for raising errors and a done channel for signifying clean exit - go func() { - errChan <- server.Start(ctx) - }() - } if err := <-errChan; err != nil { switch slackerror.ToSlackError(err).Code { case slackerror.ErrLocalAppRunCleanExit: From 867b7d398fe3bbb748d70985fc21f80809179abc Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Fri, 23 Jan 2026 23:40:19 -0800 Subject: [PATCH 2/8] fix: stop the earlier process --- internal/hooks/shell.go | 9 ++++ internal/hooks/shell_mock.go | 5 ++ internal/pkg/platform/localserver.go | 56 +++++++++++++++++++++-- internal/pkg/platform/localserver_test.go | 41 +++++++++-------- internal/pkg/platform/run.go | 1 + 5 files changed, 91 insertions(+), 21 deletions(-) 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 250d5d4c..f1fcd9bd 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,38 @@ 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 + if err := process.Signal(os.Interrupt); 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 +311,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() { @@ -345,12 +392,15 @@ func (r *LocalServer) Watch(ctx context.Context, auth types.SlackAuth, app types // If so Delegate the connection to the SDK otherwise Start connection if r.cliConfig.Config.SDKManagedConnection { r.clients.IO.PrintDebug(ctx, "Delegating connection to SDK managed script hook") + // Stop the previous process before starting a new one + r.stopDelegateProcess(ctx) + // Delegate connection to hook; this should be a blocking call, as the delegate should be a server, too. go func() { - //errChan <- err := r.StartDelegate(ctx) if err != nil { - return // or error event? + r.clients.IO.PrintDebug(ctx, "Delegated start hook failed on restart: %v", err) + return } }() } else { 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 d904b246..9543c318 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) From e2c6aaaeca8643054b2ad3ce371ca275c57bbb61 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 26 Jan 2026 21:03:44 -0800 Subject: [PATCH 3/8] docs: add reference for multiple file watchers --- docs/reference/hooks/index.md | 56 ++++++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 4 deletions(-) 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 From d7beff4449fe4a3758ed1bbd36f63af8c0e32d18 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 26 Jan 2026 21:19:10 -0800 Subject: [PATCH 4/8] feat: parse manifest and app file watchers from sdk config --- internal/hooks/sdk_config.go | 61 ++++++++++ internal/hooks/sdk_config_test.go | 179 ++++++++++++++++++++++++++++++ 2 files changed, 240 insertions(+) diff --git a/internal/hooks/sdk_config.go b/internal/hooks/sdk_config.go index 6bd2d830..00fafe8a 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,48 @@ 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.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.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..c907e061 100644 --- a/internal/hooks/sdk_config_test.go +++ b/internal/hooks/sdk_config_test.go @@ -121,3 +121,182 @@ 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 + }{ + "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 + }{ + "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) + }) + } +} From abddd41e8548f2f9258aa025e77586a597242747 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 26 Jan 2026 21:34:25 -0800 Subject: [PATCH 5/8] refactor: check error separate from setting --- internal/pkg/platform/localserver.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/pkg/platform/localserver.go b/internal/pkg/platform/localserver.go index f1fcd9bd..4f05cdd8 100644 --- a/internal/pkg/platform/localserver.go +++ b/internal/pkg/platform/localserver.go @@ -256,7 +256,8 @@ func (r *LocalServer) stopDelegateProcess(ctx context.Context) { if process != nil { r.clients.IO.PrintDebug(ctx, "Stopping previous delegated process (PID: %d)", process.Pid) // Kill the process gracefully - if err := process.Signal(os.Interrupt); err != nil { + 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() From fdf121aa59720ad4bba281399d04645e3459b024 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 26 Jan 2026 22:34:24 -0800 Subject: [PATCH 6/8] fix: separate manifest watch and server restarts --- cmd/platform/run.go | 11 +- internal/pkg/platform/localserver.go | 146 ++++++++++++++++++++------- internal/pkg/platform/run.go | 18 +++- 3 files changed, 132 insertions(+), 43 deletions(-) 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/internal/pkg/platform/localserver.go b/internal/pkg/platform/localserver.go index 4f05cdd8..18b37edb 100644 --- a/internal/pkg/platform/localserver.go +++ b/internal/pkg/platform/localserver.go @@ -349,75 +349,57 @@ 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 { +// 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 SDKCLI configuration if !r.cliConfig.Config.Watch.IsAvailable() { r.clients.IO.PrintDebug(ctx, "To watch file changes, provide watch configuration in %s", config.GetProjectHooksJSONFilePath()) 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") + 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") - - // Check to see whether the SDK managed connection flag is enabled - // If so Delegate the connection to the SDK otherwise Start connection - if r.cliConfig.Config.SDKManagedConnection { - r.clients.IO.PrintDebug(ctx, "Delegating connection to SDK managed script hook") - // Stop the previous process before starting a new one - r.stopDelegateProcess(ctx) - - // Delegate connection to hook; this should be a blocking call, as the delegate should be a server, too. - go func() { - err := r.StartDelegate(ctx) - if err != nil { - r.clients.IO.PrintDebug(ctx, "Delegated start hook failed on restart: %v", err) - return - } - }() - } else { - r.log.Info("on_cloud_run_watch_file_change") // TODO: DELETE THIS LINE BEFORE COMMIT - its for branching - // Listen for messages in a goroutine, and provide an error channel for raising errors and a done channel for signifying clean exit - // go func() { - // errChan <- r.Start(ctx) - // }() - } + 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() @@ -431,6 +413,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/run.go b/internal/pkg/platform/run.go index 9543c318..4f0f19ed 100644 --- a/internal/pkg/platform/run.go +++ b/internal/pkg/platform/run.go @@ -175,8 +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 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") + // Start app watcher which handles initial server start and restarts on file changes + go func() { + errChan <- server.WatchApp(ctx) + }() + } else { + // 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: From 233f43acf9eac6fe2f0f1878a65de5df946716e8 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 26 Jan 2026 23:19:29 -0800 Subject: [PATCH 7/8] fix: avoid reinstalling remote manifests --- internal/pkg/platform/localserver.go | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/internal/pkg/platform/localserver.go b/internal/pkg/platform/localserver.go index 18b37edb..693d5843 100644 --- a/internal/pkg/platform/localserver.go +++ b/internal/pkg/platform/localserver.go @@ -351,9 +351,25 @@ func (r *LocalServer) StartDelegate(ctx context.Context) error { // 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 SDKCLI configuration + // 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 } @@ -361,6 +377,8 @@ func (r *LocalServer) WatchManifest(ctx context.Context, auth types.SlackAuth, a 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 } From 5c176af63bbfd0fc9802e3f2f9b5f3ee3d9eff1b Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Thu, 29 Jan 2026 15:42:07 -0800 Subject: [PATCH 8/8] fix: guard against nil watch configurations --- internal/hooks/sdk_config.go | 6 ++++++ internal/hooks/sdk_config_test.go | 34 +++++++++++++++++++++---------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/internal/hooks/sdk_config.go b/internal/hooks/sdk_config.go index 00fafe8a..94419d08 100644 --- a/internal/hooks/sdk_config.go +++ b/internal/hooks/sdk_config.go @@ -95,6 +95,9 @@ func (w *WatchOpts) IsAvailable() bool { // 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 } @@ -104,6 +107,9 @@ func (w *WatchOpts) GetManifestWatchConfig() (paths []string, filterRegex string // 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 } diff --git a/internal/hooks/sdk_config_test.go b/internal/hooks/sdk_config_test.go index c907e061..25b95402 100644 --- a/internal/hooks/sdk_config_test.go +++ b/internal/hooks/sdk_config_test.go @@ -124,13 +124,19 @@ func Test_WatchOpts_IsAvailable(t *testing.T) { func Test_WatchOpts_GetManifestWatchConfig(t *testing.T) { tests := map[string]struct { - watchOpts WatchOpts + watchOpts *WatchOpts expectedPaths []string expectedRegex string expectedEnabled bool }{ + "Nil WatchOpts pointer": { + watchOpts: nil, + expectedPaths: nil, + expectedRegex: "", + expectedEnabled: false, + }, "Nested manifest config": { - watchOpts: WatchOpts{ + watchOpts: &WatchOpts{ Manifest: &ManifestWatchOpts{ Paths: []string{"manifest.json", "workflows/"}, FilterRegex: "\\.json$", @@ -141,7 +147,7 @@ func Test_WatchOpts_GetManifestWatchConfig(t *testing.T) { expectedEnabled: true, }, "Legacy flat config": { - watchOpts: WatchOpts{ + watchOpts: &WatchOpts{ Paths: []string{"manifest.json", "src/"}, FilterRegex: "\\.(json|ts)$", }, @@ -150,7 +156,7 @@ func Test_WatchOpts_GetManifestWatchConfig(t *testing.T) { expectedEnabled: true, }, "Nested config takes precedence over legacy": { - watchOpts: WatchOpts{ + watchOpts: &WatchOpts{ Paths: []string{"old-path/"}, FilterRegex: "old-regex", Manifest: &ManifestWatchOpts{ @@ -163,7 +169,7 @@ func Test_WatchOpts_GetManifestWatchConfig(t *testing.T) { expectedEnabled: true, }, "Empty nested manifest config": { - watchOpts: WatchOpts{ + watchOpts: &WatchOpts{ Manifest: &ManifestWatchOpts{ Paths: []string{}, }, @@ -173,7 +179,7 @@ func Test_WatchOpts_GetManifestWatchConfig(t *testing.T) { expectedEnabled: false, }, "Empty legacy config": { - watchOpts: WatchOpts{ + watchOpts: &WatchOpts{ Paths: []string{}, }, expectedPaths: []string{}, @@ -193,13 +199,19 @@ func Test_WatchOpts_GetManifestWatchConfig(t *testing.T) { func Test_WatchOpts_GetAppWatchConfig(t *testing.T) { tests := map[string]struct { - watchOpts WatchOpts + watchOpts *WatchOpts expectedPaths []string expectedRegex string expectedEnabled bool }{ + "Nil WatchOpts pointer": { + watchOpts: nil, + expectedPaths: nil, + expectedRegex: "", + expectedEnabled: false, + }, "Nested app config": { - watchOpts: WatchOpts{ + watchOpts: &WatchOpts{ App: &AppWatchOpts{ Paths: []string{"src/", "functions/"}, FilterRegex: "\\.(ts|js)$", @@ -210,7 +222,7 @@ func Test_WatchOpts_GetAppWatchConfig(t *testing.T) { expectedEnabled: true, }, "Legacy config does not enable app watching": { - watchOpts: WatchOpts{ + watchOpts: &WatchOpts{ Paths: []string{"manifest.json", "src/"}, FilterRegex: "\\.(json|ts)$", }, @@ -219,7 +231,7 @@ func Test_WatchOpts_GetAppWatchConfig(t *testing.T) { expectedEnabled: false, }, "Empty nested app config": { - watchOpts: WatchOpts{ + watchOpts: &WatchOpts{ App: &AppWatchOpts{ Paths: []string{}, }, @@ -229,7 +241,7 @@ func Test_WatchOpts_GetAppWatchConfig(t *testing.T) { expectedEnabled: false, }, "Nil app config": { - watchOpts: WatchOpts{}, + watchOpts: &WatchOpts{}, expectedPaths: nil, expectedRegex: "", expectedEnabled: false,