Fix hook validation cwd handling

Validate hooks in `azd hooks run` and middleware using scope-specific working directories so relative file hooks are resolved correctly for services and layers, and remove the redundant layer hook conversion during provision.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: weikanglim

cli/azd/cmd/hooks.go

@@ -351,44 +351,43 @@ func (hra *hooksRunAction) execHook(
 
 // Validates hooks and displays warnings for default shell usage and other issues
 func (hra *hooksRunAction) validateAndWarnHooks(ctx context.Context) error {
-	// Collect all hooks from project and services
-	allHooks := make(map[string][]*ext.HookConfig)
+	warningKeys := map[string]struct{}{}
+	validateAndWarn := func(cwd string, hooks map[string][]*ext.HookConfig) {
+		if len(hooks) == 0 {
+			return
+		}
+
+		hooksManager := ext.NewHooksManager(cwd, hra.commandRunner)
+		validationResult := hooksManager.ValidateHooks(ctx, hooks)
+
+		for _, warning := range validationResult.Warnings {
+			key := warning.Message + "\x00" + warning.Suggestion
+			if _, has := warningKeys[key]; has {
+				continue
+			}
 
-	// Add project hooks
-	for hookName, hookConfigs := range hra.projectConfig.Hooks {
-		allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
+			warningKeys[key] = struct{}{}
+			hra.console.MessageUxItem(ctx, &ux.WarningMessage{
+				Description: warning.Message,
+			})
+			if warning.Suggestion != "" {
+				hra.console.Message(ctx, warning.Suggestion)
+			}
+			hra.console.Message(ctx, "")
+		}
 	}
 
-	// Add service hooks
+	validateAndWarn(hra.projectConfig.Path, hra.projectConfig.Hooks)
+
 	stableServices, err := hra.importManager.ServiceStable(ctx, hra.projectConfig)
 	if err == nil {
 		for _, service := range stableServices {
-			for hookName, hookConfigs := range service.Hooks {
-				allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
-			}
+			validateAndWarn(service.Path(), service.Hooks)
 		}
 	}
 
-	// Add layer hooks
 	for _, layer := range hra.projectConfig.Infra.Layers {
-		for hookName, hookConfigs := range layer.Hooks {
-			allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
-		}
-	}
-
-	// Create hooks manager and validate
-	hooksManager := ext.NewHooksManager(hra.projectConfig.Path, hra.commandRunner)
-	validationResult := hooksManager.ValidateHooks(ctx, allHooks)
-
-	// Display any warnings
-	for _, warning := range validationResult.Warnings {
-		hra.console.MessageUxItem(ctx, &ux.WarningMessage{
-			Description: warning.Message,
-		})
-		if warning.Suggestion != "" {
-			hra.console.Message(ctx, warning.Suggestion)
-		}
-		hra.console.Message(ctx, "")
+		validateAndWarn(layer.AbsolutePath(hra.projectConfig.Path), layer.Hooks)
 	}
 
 	return nil

cli/azd/cmd/hooks_test.go

@@ -5,6 +5,7 @@ package cmd
 
 import (
 	"context"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -227,3 +228,49 @@ func Test_HooksRunAction_RejectsServiceAndLayerTogether(t *testing.T) {
 	require.Error(t, err)
 	require.ErrorContains(t, err, "--service and --layer cannot be used together")
 }
+
+func Test_HooksRunAction_ValidatesLayerHooksRelativeToLayerPath(t *testing.T) {
+	mockContext := mocks.NewMockContext(context.Background())
+	env := environment.NewWithValues("test", nil)
+
+	projectPath := t.TempDir()
+	layerScriptPath := filepath.Join(projectPath, "infra", "core", "scripts", "preprovision.sh")
+	require.NoError(t, os.MkdirAll(filepath.Dir(layerScriptPath), 0o755))
+	require.NoError(t, os.WriteFile(layerScriptPath, []byte("echo pre"), 0o600))
+
+	layerHook := &ext.HookConfig{
+		Run: filepath.Join("scripts", "preprovision.sh"),
+	}
+
+	projectConfig := &project.ProjectConfig{
+		Name:     "test",
+		Path:     projectPath,
+		Services: map[string]*project.ServiceConfig{},
+		Infra: provisioning.Options{
+			Layers: []provisioning.Options{
+				{
+					Name: "core",
+					Path: filepath.Join("infra", "core"),
+					Hooks: provisioning.HooksConfig{
+						"preprovision": {layerHook},
+					},
+				},
+			},
+		},
+	}
+
+	action := &hooksRunAction{
+		projectConfig:  projectConfig,
+		env:            env,
+		importManager:  project.NewImportManager(nil),
+		commandRunner:  mockContext.CommandRunner,
+		console:        mockContext.Console,
+		flags:          &hooksRunFlags{},
+		serviceLocator: mockContext.Container,
+	}
+
+	err := action.validateAndWarnHooks(*mockContext.Context)
+	require.NoError(t, err)
+	require.False(t, layerHook.IsUsingDefaultShell())
+	require.Equal(t, ext.ScriptTypeUnknown, layerHook.Shell)
+}

cli/azd/cmd/middleware/hooks.go

@@ -181,45 +181,41 @@ func (m *HooksMiddleware) createServiceEventHandler(
 
 // validateHooks validates hook configurations and displays any warnings
 func (m *HooksMiddleware) validateHooks(ctx context.Context, projectConfig *project.ProjectConfig) error {
-	// Get service hooks for validation
-	var serviceHooks []map[string][]*ext.HookConfig
-	stableServices, err := m.importManager.ServiceStable(ctx, projectConfig)
-	if err != nil {
-		return fmt.Errorf("failed getting services for hook validation: %w", err)
-	}
+	warningKeys := map[string]struct{}{}
+	validateAndWarn := func(cwd string, hooks map[string][]*ext.HookConfig) {
+		if len(hooks) == 0 {
+			return
+		}
 
-	for _, service := range stableServices {
-		serviceHooks = append(serviceHooks, service.Hooks)
-	}
+		hooksManager := ext.NewHooksManager(cwd, m.commandRunner)
+		validationResult := hooksManager.ValidateHooks(ctx, hooks)
 
-	// Combine project and service hooks into a single map
-	allHooks := make(map[string][]*ext.HookConfig)
+		for _, warning := range validationResult.Warnings {
+			key := warning.Message + "\x00" + warning.Suggestion
+			if _, has := warningKeys[key]; has {
+				continue
+			}
 
-	// Add project hooks
-	for hookName, hookConfigs := range projectConfig.Hooks {
-		allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
+			warningKeys[key] = struct{}{}
+			m.console.MessageUxItem(ctx, &ux.WarningMessage{
+				Description: warning.Message,
+			})
+			if warning.Suggestion != "" {
+				m.console.Message(ctx, warning.Suggestion)
+			}
+			m.console.Message(ctx, "")
+		}
 	}
 
-	// Add service hooks
-	for _, serviceHookMap := range serviceHooks {
-		for hookName, hookConfigs := range serviceHookMap {
-			allHooks[hookName] = append(allHooks[hookName], hookConfigs...)
-		}
+	validateAndWarn(projectConfig.Path, projectConfig.Hooks)
+
+	stableServices, err := m.importManager.ServiceStable(ctx, projectConfig)
+	if err != nil {
+		return fmt.Errorf("failed getting services for hook validation: %w", err)
 	}
 
-	// Create hooks manager and validate
-	hooksManager := ext.NewHooksManager(projectConfig.Path, m.commandRunner)
-	validationResult := hooksManager.ValidateHooks(ctx, allHooks)
-
-	// Display any warnings
-	for _, warning := range validationResult.Warnings {
-		m.console.MessageUxItem(ctx, &ux.WarningMessage{
-			Description: warning.Message,
-		})
-		if warning.Suggestion != "" {
-			m.console.Message(ctx, warning.Suggestion)
-		}
-		m.console.Message(ctx, "")
+	for _, service := range stableServices {
+		validateAndWarn(service.Path(), service.Hooks)
 	}
 
 	return nil

cli/azd/cmd/middleware/hooks_test.go

@@ -6,7 +6,10 @@ package middleware
 import (
 	"context"
 	"errors"
+	"os"
 	osexec "os/exec"
+	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -288,6 +291,81 @@ func Test_ServiceHooks_Registered(t *testing.T) {
 	require.Equal(t, 1, preDeployCount)
 }
 
+func Test_ServiceHooks_ValidationUsesServicePath(t *testing.T) {
+	mockContext := mocks.NewMockContext(context.Background())
+	azdContext := createAzdContext(t)
+
+	envName := "test"
+	runOptions := Options{CommandPath: "deploy"}
+
+	projectConfig := project.ProjectConfig{
+		Name:     envName,
+		Services: map[string]*project.ServiceConfig{},
+	}
+
+	hookPath := filepath.Join("scripts", "predeploy.ps1")
+	expectedShell := "pwsh"
+	scriptContents := "Write-Host 'Hello'\n"
+	if runtime.GOOS == "windows" {
+		hookPath = filepath.Join("scripts", "predeploy.sh")
+		expectedShell = "bash"
+		scriptContents = "echo hello\n"
+	}
+
+	serviceConfig := &project.ServiceConfig{
+		EventDispatcher: ext.NewEventDispatcher[project.ServiceLifecycleEventArgs](project.ServiceEvents...),
+		Language:        "ts",
+		RelativePath:    "./src/api",
+		Host:            "appservice",
+		Hooks: map[string][]*ext.HookConfig{
+			"predeploy": {
+				{
+					Run: hookPath,
+				},
+			},
+		},
+	}
+
+	projectConfig.Services["api"] = serviceConfig
+
+	err := ensureAzdValid(mockContext, azdContext, envName, &projectConfig)
+	require.NoError(t, err)
+
+	projectConfig.Services["api"].Project = &projectConfig
+
+	serviceHookPath := filepath.Join(serviceConfig.Path(), hookPath)
+	require.NoError(t, os.MkdirAll(filepath.Dir(serviceHookPath), 0o755))
+	require.NoError(t, os.WriteFile(serviceHookPath, []byte(scriptContents), 0o600))
+
+	mockContext.CommandRunner.MockToolInPath("pwsh", nil)
+
+	var executedShell string
+	mockContext.CommandRunner.When(func(args exec.RunArgs, command string) bool {
+		return true
+	}).RespondFn(func(args exec.RunArgs) (exec.RunResult, error) {
+		executedShell = args.Cmd
+		return exec.NewRunResult(0, "", ""), nil
+	})
+
+	nextFn := func(ctx context.Context) (*actions.ActionResult, error) {
+		err := serviceConfig.Invoke(ctx, project.ServiceEventDeploy, project.ServiceLifecycleEventArgs{
+			Project:        &projectConfig,
+			Service:        serviceConfig,
+			ServiceContext: project.NewServiceContext(),
+		}, func() error {
+			return nil
+		})
+
+		return &actions.ActionResult{}, err
+	}
+
+	result, err := runMiddleware(mockContext, envName, &projectConfig, &runOptions, nextFn)
+
+	require.NotNil(t, result)
+	require.NoError(t, err)
+	require.Equal(t, expectedShell, executedShell)
+}
+
 func createAzdContext(t *testing.T) *azdcontext.AzdContext {
 	tempDir := t.TempDir()
 	ostest.Chdir(t, tempDir)

cli/azd/internal/cmd/provision.go

@@ -531,8 +531,7 @@ func (p *ProvisionAction) runLayerProvisionWithHooks(
 	layerPath string,
 	actionFn ext.InvokeFn,
 ) error {
-	hooks := map[string][]*ext.HookConfig(layer.Hooks)
-	if len(hooks) == 0 {
+	if len(layer.Hooks) == 0 {
 		return actionFn()
 	}
 
@@ -543,12 +542,12 @@ func (p *ProvisionAction) runLayerProvisionWithHooks(
 		p.envManager,
 		p.console,
 		layerPath,
-		hooks,
+		layer.Hooks,
 		p.env,
 		p.serviceLocator,
 	)
 
-	p.validateAndWarnLayerHooks(ctx, hooksManager, hooks)
+	p.validateAndWarnLayerHooks(ctx, hooksManager, layer.Hooks)
 
 	if err := hooksRunner.Invoke(ctx, []string{string(project.ProjectEventProvision)}, actionFn); err != nil {
 		if layer.Name == "" {