Re-resolve package manifests during workflow updates

docs/adr/54119-reconcile-manifest-managed-assets-on-update.md

@@ -0,0 +1,43 @@
+# ADR-54119: Reconcile Manifest-Managed Assets During `gh aw update`
+
+**Date**: 2026-08-20
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+`gh aw update` previously updated only already-installed Markdown workflow files that carry a `source:` frontmatter field. Package manifests (`aw.yml`) can also declare non-Markdown assets—action workflows (`.yml`), skills, and agents—under their `includes` list. Because these assets do not carry per-file source tracking, new entries added to a package manifest after initial installation were silently skipped, leaving installed packages incomplete even as their existing workflows advanced to the latest source commit.
+
+### Decision
+
+We will re-resolve both the current and latest package manifests during `gh aw update` and install any package-owned assets (action workflows, skills, agents) that appear in the latest manifest but are absent from the current one. Installation is conservative: if the destination path already exists on disk the file is skipped to avoid overwriting local modifications. Ownership is derived from the manifest rather than per-file frontmatter.
+
+### Alternatives Considered
+
+#### Alternative 1: Require `source:` Frontmatter on All Package-Managed Assets
+
+Package authors could be required to embed `source:` metadata into every asset file. This would let the existing per-file update logic handle all asset types uniformly without new manifest-diffing code. However, it couples the manifest format to each asset's content, imposes a documentation burden on package authors, and is impractical for binary or generated files that cannot carry YAML frontmatter.
+
+#### Alternative 2: Track Installed Manifest Assets in a Dedicated Lockfile
+
+A separate lockfile (e.g., `aw.lock`) could record which asset paths were installed from which package version, enabling precise reconciliation and future removal support. This would centralize ownership tracking and decouple it from both frontmatter and manifest diffing. The trade-off is added file-system overhead, a required migration step for existing installations that lack a lockfile, and increased complexity in the update flow.
+
+### Consequences
+
+#### Positive
+- New assets declared in `aw.yml` are automatically installed when users run `gh aw update`, delivering complete package updates without manual intervention.
+- The conservative install behavior (destination-exists check) prevents unintentional overwrites of locally modified files.
+
+#### Negative
+- Assets are keyed by destination path (action workflows) or source path (skills/agents). A package that renames an asset across versions will produce a duplicate installation rather than an in-place update, requiring manual cleanup.
+- Reconciliation errors are appended to the failure list for the entire workflow group, so a single failed asset download can suppress success reporting for other updates in the same run.
+
+#### Neutral
+- The reconciliation logic runs unconditionally at the end of every `updateManifestWorkflowGroup` call, adding a GitHub API download per newly declared asset; no additional network calls are made for unchanged assets.
+- Removal of assets dropped from the manifest is not addressed by this change; removal behavior remains conservative (unchanged from the prior implementation).
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/cli/update_manifest.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/gitutil"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/workflow"
 )
@@ -192,9 +193,133 @@ func updateManifestWorkflowGroup(ctx context.Context, source string, grouped []*
 		successes = append(successes, name)
 	}
 
+	assetEngine := resolveManifestAssetEngine(grouped, opts)
+	if err := reconcileManifestManagedAssets(ctx, repoSpec.RepoSlug, currentPkg, latestPkg, assetEngine); err != nil {
+		failures = append(failures, updateFailure{Name: source, Error: err.Error()})
+	}
+
 	return successes, failures
 }
 
+// reconcileManifestManagedAssets installs package-owned action workflows, skills, and
+// agents that were added to the latest manifest. These assets do not carry source
+// frontmatter, so their package ownership is derived from the package manifest itself.
+func reconcileManifestManagedAssets(ctx context.Context, repo string, _ *resolvedRepositoryPackage, latestPkg *resolvedRepositoryPackage, engineOverride string) error {
+	gitRoot, err := gitutil.FindGitRoot()
+	if err != nil {
+		return fmt.Errorf("failed to find repository root for package assets: %w", err)
+	}
+	owner, repository, err := splitRepositoryPackageSlug(repo)
+	if err != nil {
+		return err
+	}
+
+	for _, installable := range latestPkg.InstallationSource {
+		if !isActionWorkflowPath(installable.SourcePath) {
+			continue
+		}
+		destPath := filepath.Join(gitRoot, filepath.FromSlash(installable.DestinationPath))
+		if _, err := os.Stat(destPath); err == nil {
+			updateManifestLog.Printf("Skipping new package action workflow because destination already exists: %s", destPath)
+			continue
+		} else if !os.IsNotExist(err) {
+			return fmt.Errorf("failed to inspect new package action workflow destination %s: %w", destPath, err)
+		}
+		content, err := downloadPackageFileFromGitHubForHost(ctx, owner, repository, installable.SourcePath, latestPkg.ResolvedRef, "")
+		if err != nil {
+			return fmt.Errorf("failed to download new package action workflow %s: %w", installable.SourcePath, err)
+		}
+		if err := os.MkdirAll(filepath.Dir(destPath), constants.DirPermPublic); err != nil {
+			return fmt.Errorf("failed to create package action workflow directory: %w", err)
+		}
+		if err := os.WriteFile(destPath, content, constants.FilePermPublic); err != nil {
+			return fmt.Errorf("failed to install new package action workflow %s: %w", installable.DestinationPath, err)
+		}
+		fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Added package action workflow: "+filepath.Base(destPath)))
+	}
+
+	for _, skill := range latestPkg.SkillFiles {
+		destPath, err := packageSkillDestinationPath(gitRoot, skill, engineOverride)
+		if err != nil {
+			return err
+		}
+		if _, err := os.Stat(destPath); err == nil {
+			updateManifestLog.Printf("Skipping new package skill because destination already exists: %s", destPath)
+			continue
+		} else if !os.IsNotExist(err) {
+			return fmt.Errorf("failed to inspect new package skill destination %s: %w", destPath, err)
+		}
+		content, err := downloadPackageFileFromGitHubForHost(ctx, owner, repository, skill.SourcePath, latestPkg.ResolvedRef, "")
+		if err != nil {
+			return fmt.Errorf("failed to download new package skill %s: %w", skill.SourcePath, err)
+		}
+		resolved := &ResolvedWorkflow{
+			Content:            content,
+			Spec:               &WorkflowSpec{WorkflowPath: skill.SourcePath},
+			IsPackageSkillFile: true,
+			SkillName:          skill.SkillName,
+		}
+		if err := addSkillFileWithTracking(resolved, nil, AddOptions{
+			EngineOverride: engineOverride,
+			Quiet:          false,
+		}, gitRoot); err != nil {
+			return fmt.Errorf("failed to install new package skill %s: %w", skill.SourcePath, err)
+		}
+	}
+
+	for _, agent := range latestPkg.AgentFiles {
+		destPath := packageAgentDestinationPath(gitRoot, agent, engineOverride)
+		if _, err := os.Stat(destPath); err == nil {
+			updateManifestLog.Printf("Skipping new package agent because destination already exists: %s", destPath)
+			continue
+		} else if !os.IsNotExist(err) {
+			return fmt.Errorf("failed to inspect new package agent destination %s: %w", destPath, err)
+		}
+		content, err := downloadPackageFileFromGitHubForHost(ctx, owner, repository, agent, latestPkg.ResolvedRef, "")
+		if err != nil {
+			return fmt.Errorf("failed to download new package agent %s: %w", agent, err)
+		}
+		resolved := &ResolvedWorkflow{Content: content, Spec: &WorkflowSpec{WorkflowPath: agent}, IsPackageAgentFile: true}
+		if err := addAgentFileWithTracking(resolved, nil, AddOptions{EngineOverride: engineOverride}, gitRoot); err != nil {
+			return fmt.Errorf("failed to install new package agent %s: %w", agent, err)
+		}
+	}
+	return nil
+}
+
+func resolveManifestAssetEngine(grouped []*workflowWithSource, opts UpdateWorkflowsOptions) string {
+	if opts.EngineOverride != "" {
+		return opts.EngineOverride
+	}
+	for _, wf := range grouped {
+		content, err := os.ReadFile(wf.Path)
+		if err != nil {
+			continue
+		}
+		if engine := strings.TrimSpace(ExtractWorkflowEngine(string(content))); engine != "" {
+			updateManifestLog.Printf("Using engine %q from installed manifest-managed workflow %s for package asset reconciliation", engine, wf.Name)
+			return engine
+		}
+	}
+	return ""
+}
+
+func packageSkillDestinationPath(gitRoot string, skill resolvedPackageSkillFile, engineOverride string) (string, error) {
+	resolved := &ResolvedWorkflow{
+		Spec:      &WorkflowSpec{WorkflowPath: skill.SourcePath},
+		SkillName: skill.SkillName,
+	}
+	relPath, err := resolveSkillRelativePath(resolved)
+	if err != nil {
+		return "", fmt.Errorf("failed to resolve destination for package skill %s: %w", skill.SourcePath, err)
+	}
+	return filepath.Join(gitRoot, workflow.GetEngineSkillDir(engineOverride), skill.SkillName, relPath), nil
+}
+
+func packageAgentDestinationPath(gitRoot, sourcePath, engineOverride string) string {
+	return filepath.Join(gitRoot, workflow.GetEngineSubAgentDir(engineOverride), filepath.Base(sourcePath))
+}
+
 func removeManifestManagedWorkflow(workflowPath string) error {
 	updateManifestLog.Printf("Removing manifest-managed workflow no longer in manifest: %s", filepath.Base(workflowPath))
 	if err := os.Remove(workflowPath); err != nil && !os.IsNotExist(err) {

pkg/cli/update_manifest_test.go

@@ -13,8 +13,115 @@ import (
 	"time"
 
 	"github.com/github/gh-aw/pkg/testutil"
+	"github.com/github/gh-aw/pkg/workflow"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+func TestReconcileManifestManagedAssets_AddsPackageOwnedAssets(t *testing.T) {
+	tmpDir := testutil.TempDir(t, "manifest-assets-*")
+	require.NoError(t, os.Mkdir(filepath.Join(tmpDir, ".git"), 0o755))
+	t.Chdir(tmpDir)
+
+	originalDownload := downloadPackageFileFromGitHubForHost
+	t.Cleanup(func() { downloadPackageFileFromGitHubForHost = originalDownload })
+	downloadPackageFileFromGitHubForHost = func(_ context.Context, owner, repo, path, ref, host string) ([]byte, error) {
+		if owner != "owner" || repo != "repo" || ref != "v2.0.0" {
+			return nil, fmt.Errorf("unexpected package source %s/%s@%s", owner, repo, ref)
+		}
+		switch path {
+		case ".github/workflows/new.yml":
+			return []byte("name: new action\n"), nil
+		case "skills/review/scripts/check.sh":
+			return []byte("#!/bin/sh\n"), nil
+		case "agents/reviewer.md":
+			return []byte("# Reviewer\n"), nil
+		default:
+			return nil, fmt.Errorf("unexpected package path %s", path)
+		}
+	}
+
+	err := reconcileManifestManagedAssets(context.Background(), "owner/repo",
+		&resolvedRepositoryPackage{},
+		&resolvedRepositoryPackage{
+			ResolvedRef: "v2.0.0",
+			InstallationSource: []resolvedPackageInstallable{{
+				SourcePath:      ".github/workflows/new.yml",
+				DestinationPath: ".github/workflows/new.yml",
+			}},
+			SkillFiles: []resolvedPackageSkillFile{{
+				SourcePath: "skills/review/scripts/check.sh",
+				SkillName:  "review",
+			}},
+			AgentFiles: []string{"agents/reviewer.md"},
+		},
+		"copilot",
+	)
+	require.NoError(t, err)
+	workflowPath := filepath.Join(tmpDir, ".github", "workflows", "new.yml")
+	assert.FileExists(t, workflowPath)
+	assert.FileExists(t, filepath.Join(tmpDir, workflow.GetEngineSkillDir("copilot"), "review", "scripts", "check.sh"))
+	assert.FileExists(t, filepath.Join(tmpDir, workflow.GetEngineSubAgentDir("copilot"), "reviewer.md"))
+	workflowContent, readErr := os.ReadFile(workflowPath)
+	require.NoError(t, readErr)
+	assert.Equal(t, "name: new action\n", string(workflowContent))
+}
+
+func TestReconcileManifestManagedAssets_BranchTrackingInstallsMissingAssets(t *testing.T) {
+	tmpDir := testutil.TempDir(t, "manifest-assets-branch-main-*")
+	require.NoError(t, os.Mkdir(filepath.Join(tmpDir, ".git"), 0o755))
+	t.Chdir(tmpDir)
+
+	originalDownload := downloadPackageFileFromGitHubForHost
+	t.Cleanup(func() { downloadPackageFileFromGitHubForHost = originalDownload })
+	downloadPackageFileFromGitHubForHost = func(_ context.Context, owner, repo, path, ref, host string) ([]byte, error) {
+		if owner != "owner" || repo != "repo" || ref != "7d8e9f0" {
+			return nil, fmt.Errorf("unexpected package source %s/%s@%s", owner, repo, ref)
+		}
+		switch path {
+		case ".github/workflows/new.yml":
+			return []byte("name: branch action\n"), nil
+		case "skills/review/scripts/check.sh":
+			return []byte("#!/bin/sh\n"), nil
+		case "agents/reviewer.md":
+			return []byte("# Reviewer\n"), nil
+		default:
+			return nil, fmt.Errorf("unexpected package path %s", path)
+		}
+	}
+
+	currentAndLatest := &resolvedRepositoryPackage{
+		ResolvedRef: "7d8e9f0",
+		InstallationSource: []resolvedPackageInstallable{{
+			SourcePath:      ".github/workflows/new.yml",
+			DestinationPath: ".github/workflows/new.yml",
+		}},
+		SkillFiles: []resolvedPackageSkillFile{{
+			SourcePath: "skills/review/scripts/check.sh",
+			SkillName:  "review",
+		}},
+		AgentFiles: []string{"agents/reviewer.md"},
+	}
+	err := reconcileManifestManagedAssets(context.Background(), "owner/repo", currentAndLatest, currentAndLatest, "copilot")
+	require.NoError(t, err)
+
+	assert.FileExists(t, filepath.Join(tmpDir, ".github", "workflows", "new.yml"))
+	assert.FileExists(t, filepath.Join(tmpDir, workflow.GetEngineSkillDir("copilot"), "review", "scripts", "check.sh"))
+	assert.FileExists(t, filepath.Join(tmpDir, workflow.GetEngineSubAgentDir("copilot"), "reviewer.md"))
+}
+
+func TestResolveManifestAssetEngine(t *testing.T) {
+	tmpDir := testutil.TempDir(t, "manifest-assets-engine-*")
+	workflowPath := filepath.Join(tmpDir, "existing.md")
+	require.NoError(t, os.WriteFile(workflowPath, []byte("---\nengine: claude\nsource: owner/repo@main\n---\n\n# Existing\n"), 0o644))
+
+	engine := resolveManifestAssetEngine([]*workflowWithSource{{Name: "existing", Path: workflowPath}}, UpdateWorkflowsOptions{})
+	assert.Equal(t, "claude", engine)
+
+	overrideEngine := resolveManifestAssetEngine([]*workflowWithSource{{Name: "existing", Path: workflowPath}}, UpdateWorkflowsOptions{EngineOverride: "copilot"})
+	assert.Equal(t, "copilot", overrideEngine)
+}
+
 func TestUpdateManifestWorkflowGroup_AddsUpdatesRemoves(t *testing.T) {
 	originalResolveLatestRef := resolveLatestRefFn
 	originalDownloadPackage := downloadPackageFileFromGitHubForHost