Include grader evaluator scripts in workflow packages

docs/adr/56268-include-grader-evaluator-scripts-in-workflow-packages.md

@@ -0,0 +1,45 @@
+# ADR-56268: Include Grader Evaluator Scripts in Workflow Packages
+
+**Date**: 2026-08-27
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+When users install a packaged workflow via `gh aw` that declares grader evaluators under `graders.*.run`, the evaluator shell scripts referenced by those keys were not co-installed alongside the workflow files. This caused workflow compilation to fail immediately in clean repositories because the compiler expects those scripts to be present. The root cause was that the package resource resolver (`extractResources`) only scanned the `resources:` frontmatter field and had no awareness of the `graders:` stanza. Evaluator scripts live at repository-root-relative paths (under `.github/graders/`) rather than relative to the workflow file, and the same evaluator may be shared across multiple workflows in one package.
+
+### Decision
+
+We will extend `extractResources` to also parse grader frontmatter via `workflow.ParseGradersFromFrontmatter` and append each non-empty `run` path to the resource list, then deduplicate the combined set before returning it. Within `fetchAndSaveRemoteResources` we will introduce an `isGraderEvaluator` branch that (a) resolves evaluator paths repository-root-relative rather than workflow-directory-relative, (b) installs them relative to the git repository root rather than the workflows target directory, and (c) allows silent no-op re-installs when the existing file content matches the incoming download, blocking only on content divergence. This routes evaluator scripts through the existing resource installation and ownership infrastructure so that `gh aw update` can restore or update them without additional code paths.
+
+### Alternatives Considered
+
+#### Alternative 1: Require explicit listing in `resources:` field
+
+Workflow authors would be required to manually duplicate the evaluator path in both `graders.*.run` and `resources:`. This was the implicit status-quo before this fix. It is rejected because it creates a footgun: authors writing a `graders:` stanza naturally expect the referenced script to be packaged, and the silent omission produces a compilation failure that is hard to diagnose in clean repositories.
+
+#### Alternative 2: Make missing evaluators a non-fatal warning at compile time
+
+The compiler could treat a missing evaluator as a warning rather than an error, allowing workflows to install without the script. This is rejected because evaluators are required for grader execution — silencing the error would allow users to install broken workflows that fail at runtime rather than at installation/compilation time, which is a worse developer experience.
+
+### Consequences
+
+#### Positive
+- Workflows with grader evaluators install successfully in clean repositories without any manual `resources:` duplication.
+- `gh aw update` automatically restores or updates missing evaluator scripts using the existing resource lifecycle.
+- Evaluator scripts shared across multiple packaged workflows are deduplicated, avoiding redundant downloads.
+
+#### Negative
+- `fetchAndSaveRemoteResources` now contains two distinct path-resolution conventions (workflow-dir-relative for ordinary resources, repo-root-relative for grader evaluators), increasing the function's complexity. A `//nolint:largefunc` suppression is needed.
+- Content-based conflict detection for grader evaluators (byte-by-byte comparison before allowing overwrite) is deferred until after the download, adding a network round-trip even when the file would ultimately be skipped. Ordinary resources use a pre-download existence check.
+- The `isGraderEvaluator` heuristic relies on the `.github/graders/` path prefix convention; evaluators stored elsewhere would not be detected and would still require manual `resources:` listing.
+
+#### Neutral
+- The `downloadResourceFileFromGitHub` function is extracted as a package-level variable to allow test injection, which is a minor testability refactor with no production behavior change.
+- The `absTargetDir` pre-computation is moved from the function entry point into the per-resource loop body to support per-resource target base switching.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/cli/add_command_resources.go

@@ -42,13 +42,13 @@ func addResourceFileWithTracking(resolved *ResolvedWorkflow, tracker *FileTracke
 		return fmt.Errorf("resource destination %q is invalid", resolved.Spec.DestinationPath)
 	}
 	destFile := filepath.Join(gitRoot, destination)
+	if err := fileutil.ValidatePathWithinBase(gitRoot, destFile); err != nil {
+		return fmt.Errorf("failed to validate resource destination %q: %w", resolved.Spec.DestinationPath, err)
+	}
 	rel, err := filepath.Rel(gitRoot, destFile)
 	if err != nil {
 		return fmt.Errorf("failed to validate resource destination %q: %w", resolved.Spec.DestinationPath, err)
 	}
-	if rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
-		return fmt.Errorf("resource destination %q escapes repository root", resolved.Spec.DestinationPath)
-	}
 
 	fileExists := fileutil.FileExists(destFile)
 	if fileExists && !opts.Force {

pkg/cli/add_package_manifest_resolve.go

@@ -6,7 +6,10 @@ package cli
 import (
 	"context"
 	"fmt"
+	"path"
 	"strings"
+
+	"github.com/github/gh-aw/pkg/constants"
 )
 
 func resolveRepositoryPackage(ctx context.Context, repoSpec *RepoSpec, host string) (*resolvedRepositoryPackage, error) {
@@ -31,7 +34,10 @@ func resolveRepositoryPackage(ctx context.Context, repoSpec *RepoSpec, host stri
 	if err != nil {
 		return nil, err
 	}
-	resourceFiles := normalizePackageResourcePaths(manifest.Resources, packagePath)
+	resourceFiles, err := resolveRepositoryPackageResourceFiles(ctx, owner, repo, packagePath, ref, host, manifest, installationSources)
+	if err != nil {
+		return nil, err
+	}
 
 	docsPath, err := resolveRepositoryPackageDocsPath(ctx, owner, repo, packagePath, ref, host)
 	if err != nil {
@@ -60,6 +66,69 @@ func resolveRepositoryPackage(ctx context.Context, repoSpec *RepoSpec, host stri
 	return newResolvedRepositoryPackage(manifestPath, ref, docsPath, manifest, installationSources, resourceFiles, extensionFiles, warnings), nil
 }
 
+func resolveRepositoryPackageResourceFiles(ctx context.Context, owner, repo, packagePath, ref, host string, manifest *repositoryPackageManifest, installationSources []resolvedPackageInstallable) ([]resolvedPackageResource, error) {
+	resourceFiles := normalizePackageResourcePaths(manifest.Resources, packagePath)
+	return appendPackageGraderEvaluatorResources(ctx, owner, repo, ref, host, packagePath, resourceFiles, installationSources)
+}
+
+func appendPackageGraderEvaluatorResources(ctx context.Context, owner, repo, ref, host, packagePath string, resourceFiles []resolvedPackageResource, installationSources []resolvedPackageInstallable) ([]resolvedPackageResource, error) {
+	seen := make(map[string]string, len(resourceFiles))
+	for _, resource := range resourceFiles {
+		seen[packageResourceDestinationKey(resource.DestinationPath)] = resource.SourcePath
+	}
+	addPackageManifestLog.Printf("resolving grader evaluators from %d installable package source(s)", len(installationSources))
+	for _, installable := range installationSources {
+		if !strings.HasSuffix(strings.ToLower(installable.SourcePath), ".md") {
+			continue
+		}
+		content, err := downloadPackageFileFromGitHubForHost(ctx, owner, repo, installable.SourcePath, ref, host)
+		if err != nil {
+			if isRepositoryFileNotFound(err) {
+				addPackageManifestLog.Printf("skipping grader evaluator resource discovery for unavailable package workflow %q: %v", installable.SourcePath, err)
+				continue
+			}
+			return nil, fmt.Errorf("failed to read package workflow %q while resolving grader evaluator resources: %w", installable.SourcePath, err)
+		}
+		entries, err := extractResourceEntries(string(content))
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse package workflow %q grader resources: %w", installable.SourcePath, err)
+		}
+		for _, entry := range entries {
+			if !entry.isGraderEvaluator {
+				continue
+			}
+			resource := packageGraderEvaluatorResource(installable, entry.path, packagePath)
+			key := packageResourceDestinationKey(resource.DestinationPath)
+			if previousSource, exists := seen[key]; exists {
+				if previousSource != resource.SourcePath {
+					return nil, fmt.Errorf("package workflows reference multiple grader evaluator resources for %q: %q and %q", resource.DestinationPath, previousSource, resource.SourcePath)
+				}
+				continue
+			}
+			seen[key] = resource.SourcePath
+			resourceFiles = append(resourceFiles, resource)
+		}
+	}
+	return resourceFiles, nil
+}
+
+func packageGraderEvaluatorResource(installable resolvedPackageInstallable, runPath, packagePath string) resolvedPackageResource {
+	if localPath, ok := strings.CutPrefix(runPath, "./"); ok {
+		return resolvedPackageResource{
+			SourcePath:      path.Join(path.Dir(installable.SourcePath), localPath),
+			DestinationPath: path.Join(path.Dir(installable.DestinationPath), localPath),
+		}
+	}
+	sourcePath := joinRepositoryPackagePath(packagePath, runPath)
+	if localWorkflowsPath, ok := strings.CutPrefix(runPath, constants.WorkflowsDirSlash); ok {
+		sourcePath = path.Join(path.Dir(installable.SourcePath), localWorkflowsPath)
+	}
+	return resolvedPackageResource{
+		SourcePath:      sourcePath,
+		DestinationPath: runPath,
+	}
+}
+
 func splitRepositoryPackageSlug(repoSlug string) (string, string, error) {
 	parts := strings.SplitN(repoSlug, "/", 2)
 	if len(parts) != 2 {
@@ -173,7 +242,7 @@ func newResolvedRepositoryPackage(manifestPath, ref, docsPath string, manifest *
 
 func loadRepositoryPackageManifestFile(ctx context.Context, owner, repo, packagePath, ref, host string) (string, []byte, error) {
 	manifestPath := joinRepositoryPackagePath(packagePath, repositoryPackageManifestFileName)
-	repoSlug := owner + "/" + repo
+	repoSlug := fmt.Sprintf("%s/%s", owner, repo)
 	packageID := repositoryPackageIdentifier(repoSlug, packagePath)
 	content, err := downloadPackageFileFromGitHubForHost(ctx, owner, repo, manifestPath, ref, host)
 	if err != nil {

pkg/cli/add_package_manifest_resources.go

@@ -102,9 +102,14 @@ func normalizePackageResourcePaths(resources []repositoryPackageResource, packag
 			DestinationPath: resource.Destination,
 		})
 	}
+
 	return normalized
 }
 
+func packageResourceDestinationKey(destination string) string {
+	return strings.ToLower(filepath.ToSlash(filepath.Clean(destination)))
+}
+
 func normalizeLocalPackageResourcePaths(resources []repositoryPackageResource, packageDir string) ([]resolvedPackageResource, error) {
 	normalized := make([]resolvedPackageResource, 0, len(resources))
 	for _, resource := range resources {

pkg/cli/add_package_manifest_test.go

@@ -122,6 +122,54 @@ resources:
 			assert.Equal(t, "packages/repo-assist/policy/controls.json", pkg.ResourceFiles[1].SourcePath)
 			assert.Equal(t, ".github/aw/policy/controls.json", pkg.ResourceFiles[1].DestinationPath)
 		})
+
+		t.Run("adds grader evaluators to package resources", func(t *testing.T) {
+			downloadPackageFileFromGitHubForHost = func(_ context.Context, owner, repo, path, ref, host string) ([]byte, error) {
+				switch path {
+				case "packages/repo-assist/aw.yml":
+					return []byte(`name: Repo Assist
+files:
+  - workflows/review.md
+  - workflows/triage.md
+`), nil
+				case "packages/repo-assist/README.md":
+					return []byte("# Repo Assist\n"), nil
+				case "packages/repo-assist/workflows/review.md":
+					return []byte(`---
+on: pull_request
+graders:
+  operational-value:
+    run: .github/workflows/graders/shared-operational-value.sh
+---
+# Review
+`), nil
+				case "packages/repo-assist/workflows/triage.md":
+					return []byte(`---
+on: issues
+graders:
+  operational-value:
+    run: ./graders/triage-operational-value.sh
+---
+# Triage
+`), nil
+				default:
+					return nil, createRepositoryPackageNotFoundError(path)
+				}
+			}
+			listPackageWorkflowFilesForHost = func(_ context.Context, owner, repo, ref, workflowPath, host string) ([]string, error) {
+				t.Fatalf("unexpected scan of %s", workflowPath)
+				return nil, nil
+			}
+
+			pkg, err := resolveRepositoryPackage(t.Context(), &RepoSpec{RepoSlug: "owner/repo", PackagePath: "packages/repo-assist"}, "")
+			require.NoError(t, err)
+			require.Len(t, pkg.ResourceFiles, 2)
+			assert.Equal(t, "packages/repo-assist/workflows/graders/shared-operational-value.sh", pkg.ResourceFiles[0].SourcePath)
+			assert.Equal(t, ".github/workflows/graders/shared-operational-value.sh", pkg.ResourceFiles[0].DestinationPath)
+			assert.Equal(t, "packages/repo-assist/workflows/graders/triage-operational-value.sh", pkg.ResourceFiles[1].SourcePath)
+			assert.Equal(t, ".github/workflows/graders/triage-operational-value.sh", pkg.ResourceFiles[1].DestinationPath)
+			assert.True(t, isPackageResourceDestination(pkg.ResourceFiles[1].DestinationPath))
+		})
 		getRepositoryPackageLatestRelease = func(_ context.Context, repoSlug, host string) (string, error) {
 			assert.Equal(t, "owner/repo", repoSlug)
 			assert.Equal(t, "github.com", host)

pkg/cli/add_package_ownership.go

@@ -243,7 +243,7 @@ func readPackageOwnershipRecords(gitRoot string) ([]packageOwnershipRecord, erro
 	return records, nil
 }
 
-func syncManifestManagedResources(ctx context.Context, repoSpec *RepoSpec, pkg *resolvedRepositoryPackage, ref string, opts UpdateWorkflowsOptions) error {
+func syncManifestManagedResources(ctx context.Context, repoSpec *RepoSpec, pkg *resolvedRepositoryPackage, ref string, opts UpdateWorkflowsOptions) error { //nolint:largefunc
 	if pkg == nil || repoSpec == nil {
 		return nil
 	}
@@ -286,6 +286,9 @@ func syncManifestManagedResources(ctx context.Context, repoSpec *RepoSpec, pkg *
 	for _, resource := range pkg.ResourceFiles {
 		destination := filepath.ToSlash(filepath.Clean(resource.DestinationPath))
 		destPath := filepath.Join(gitRoot, filepath.FromSlash(destination))
+		if err := fileutil.ValidatePathWithinBase(gitRoot, destPath); err != nil {
+			return fmt.Errorf("resource %q escapes repository root: %w", destination, err)
+		}
 		if fileutil.FileExists(destPath) && !opts.Force {
 			if owned, drifted := packageOwnershipAllowsOverwrite(gitRoot, destination, packageBase); !owned || drifted {
 				if owned {
@@ -442,7 +445,8 @@ func upsertPackageOwnershipFile(entries []packageOwnershipFileEntry, next packag
 func isPackageResourceDestination(destination string) bool {
 	return strings.EqualFold(destination, constants.GithubDir+"CODEOWNERS") ||
 		strings.HasPrefix(destination, constants.GithubDir+"ISSUE_TEMPLATE/") ||
-		strings.HasPrefix(destination, constants.GithubDir+"aw/")
+		strings.HasPrefix(destination, constants.GithubDir+"aw/") ||
+		workflow.IsValidOperationalValueEvaluatorRunPath(destination)
 }
 
 func removePackageOwnedFilesIfUnused(packageBase string) error {

pkg/cli/compile_pipeline_shellcheck_resources_test.go

@@ -21,7 +21,7 @@ func TestCompileWorkflows_ShellScriptResourcesIncludeOperationalValueGrader(t *t
 	tmpDir := t.TempDir()
 	require.NoError(t, initTestGitRepo(tmpDir))
 
-	gradersDir := filepath.Join(tmpDir, ".github", "graders")
+	gradersDir := filepath.Join(tmpDir, ".github", "workflows", "graders")
 	require.NoError(t, os.MkdirAll(gradersDir, 0o755))
 
 	evaluatorScript := "#!/usr/bin/env bash\nset -euo pipefail\necho '{}'\n"
@@ -45,7 +45,7 @@ mcp-scripts:
       git status --short
 graders:
   operational-value:
-    run: .github/graders/example-operational-value.sh
+    run: ./graders/example-operational-value.sh
 ---
 
 # Test Workflow
@@ -78,7 +78,7 @@ This is a test workflow exercising frontmatter shell script resources.
 		case "graders.operational-value":
 			gradersFound = true
 			assert.Equal(t, evaluatorScript, resource.Script)
-			assert.Equal(t, ".github/graders/example-operational-value.sh", resource.Source)
+			assert.Equal(t, "./graders/example-operational-value.sh", resource.Source)
 			assert.Equal(t, "bash", resource.Shell)
 		case "mcp-scripts.inspect":
 			mcpScriptsFound = true