Skip to content

Commit 351ea82

Browse files
Copilotpelikhan
andauthored
Align manifest private blocking with private:true semantics
Co-authored-by: pelikhan <[email protected]>
1 parent c1ce795 commit 351ea82

5 files changed

Lines changed: 23 additions & 30 deletions

docs/adr/36227-reject-private-field-in-manifest-workflows.md

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,37 @@
1-
# ADR-36227: Reject the `private` Field in Manifest-Listed Installable Workflows
1+
# ADR-36227: Enforce `private: true` for Add/Package Blocking
22

33
**Date**: 2026-06-01
44
**Status**: Draft
55

66
## Context
77

8-
`aw.yml` package manifests can list installable workflows that other repositories add via `gh aw add` / `gh aw add-wizard`. The `private` frontmatter field was designed to block a standalone workflow from being installed elsewhere (`private: true`), but the manifest resolution path only consulted `ExtractWorkflowPrivate`, which returned `false` for both an absent field and an explicit `private: false`. As a result, a manifest could list a workflow declaring `private: false`, and that declaration would silently pass — leaking an installation-control field into package manifests where it has no coherent meaning. A workflow that is listed in a manifest is, by definition, meant to be installed, so any `private` declaration on it (true *or* false) is contradictory and should be surfaced rather than ignored.
8+
`aw.yml` package manifests can list installable workflows that other repositories add via `gh aw add` / `gh aw add-wizard`. The `private` frontmatter field is intended to block installation only when it is explicitly set to `true`. Manifest-backed resolution and compile-time validation must preserve that semantics consistently, so package workflows only fail when they declare `private: true`.
99

1010
## Decision
1111

12-
We will treat the presence of a `private` field on a manifest-listed installable workflow as a manifest validation error, distinguishing presence from value. We split `ExtractWorkflowPrivate(content) bool` into a presence-aware `ExtractWorkflowPrivateSetting(content) (value, present bool)`, keeping `ExtractWorkflowPrivate` as a thin wrapper for the existing standalone-add behavior. Both manifest-backed resolution (`ResolveWorkflows` for `FromRepositoryManifest` specs) and compile-time local manifest validation (`validateLocalRepositoryPackageContents`) now reject any listed workflow that declares `private`, emitting manifest-scoped errors that name the offending workflow path: `private: true` reports that private workflows cannot be added, and `private: false` instructs the author to remove the field.
12+
We keep `ExtractWorkflowPrivateSetting(content) (value, present bool)` for frontmatter parsing, but manifest-backed resolution (`ResolveWorkflows` for `FromRepositoryManifest` specs) and compile-time local manifest validation reject only `private: true`. `private: false` remains installable.
1313

1414
## Alternatives Considered
1515

16-
### Alternative 1: Keep value-only extraction and reject only `private: true`
17-
Continue using the boolean-only `ExtractWorkflowPrivate` and reject manifest workflows only when it returns `true`. This was rejected because it cannot distinguish an absent field from `private: false`, so the originally-reported leak (`private: false` slipping into a manifest) would remain silently accepted, and authors would get no signal that the field is meaningless in a manifest context.
16+
### Alternative 1: Reject any manifest-listed `private` declaration
17+
Reject `private: true` and `private: false` equally for manifest-listed workflows. This was rejected because `private: false` is not a disable signal and should not block install/package behavior.
1818

19-
### Alternative 2: Strip or normalize the `private` field during manifest install instead of erroring
20-
Silently drop the `private` field when resolving manifest workflows so installation proceeds regardless. This was rejected because silent normalization hides author intent and conflicts with the project's fail-loud manifest-validation posture; an explicit, path-scoped error is more debuggable and prevents shipping manifests that encode contradictory intent.
19+
### Alternative 2: Strip or normalize the `private` field during manifest install instead of respecting value
20+
Silently normalize `private` during manifest resolution. This was rejected because it hides intent; explicit `private: true` should continue to fail loudly.
2121

2222
## Consequences
2323

2424
### Positive
25-
- Closes the leak where `private: false` in a manifest-listed workflow was silently accepted; the field is now rejected at both add time and compile time.
26-
- Error messages are presence- and value-aware and name the offending workflow path, making the fix obvious to manifest authors.
25+
- Add/package and compile-time validation consistently reject only workflows that declare `private: true`.
26+
- Error messages continue to name the offending workflow path when rejection occurs.
2727

2828
### Negative
29-
- Existing published manifests that list workflows carrying any `private` declaration will now fail to add/compile until the field is removed — a breaking change for those packages.
30-
- Introduces a second extraction function (`ExtractWorkflowPrivateSetting` alongside `ExtractWorkflowPrivate`) and parallel rejection logic in both the resolution and compile-time paths that must stay in sync.
29+
- Existing manifests using `private: false` remain installable; only `private: true` workflows are blocked.
30+
- Introduces a second extraction function (`ExtractWorkflowPrivateSetting` alongside `ExtractWorkflowPrivate`) and parallel checks in both the resolution and compile-time paths that must stay in sync.
3131

3232
### Neutral
3333
- Standalone (non-manifest) workflow adds are unchanged: `private: true` still blocks installation via the preserved `ExtractWorkflowPrivate` path.
34-
- Compile-time validation now scans manifest-listed installable paths (explicit `files:`/`includes:` entries, or discovered package directories) to apply the rule uniformly.
34+
- Compile-time validation scans manifest-listed installable paths (explicit `files:`/`includes:` entries, or discovered package directories) using the same `private: true` semantics.
3535

3636
---
3737

pkg/cli/add_package_manifest.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -717,13 +717,9 @@ func validateManifestInstallableWorkflowPrivacy(manifestPath string, installatio
717717
}
718718

719719
privateValue, hasPrivate := ExtractWorkflowPrivateSetting(string(content))
720-
if !hasPrivate {
721-
continue
722-
}
723-
if privateValue {
720+
if hasPrivate && privateValue {
724721
return fmt.Errorf("invalid Agentic Workflow manifest %q: workflow %q sets private: true and cannot be included because private workflows cannot be added", manifestPath, installationSource)
725722
}
726-
return fmt.Errorf("invalid Agentic Workflow manifest %q: workflow %q sets private: false; remove the private field because manifest-listed workflows must not declare it", manifestPath, installationSource)
727723
}
728724

729725
return nil

pkg/cli/add_package_manifest_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ files:
473473
assert.Equal(t, ".github/workflows/nightly-review.md", resolved.Workflows[1].Spec.WorkflowPath)
474474
}
475475

476-
func TestResolveWorkflows_RepositoryPackageRejectsPrivateFalse(t *testing.T) {
476+
func TestResolveWorkflows_RepositoryPackageRejectsPrivateTrue(t *testing.T) {
477477
originalFetchFn := fetchWorkflowFromSourceWithContextFn
478478
originalDownload := downloadPackageFileFromGitHubForHost
479479
originalList := listPackageWorkflowFilesForHost
@@ -504,7 +504,7 @@ func TestResolveWorkflows_RepositoryPackageRejectsPrivateFalse(t *testing.T) {
504504
case "README.md":
505505
return []byte("# Repo Assist\n"), nil
506506
case "workflows/review.md":
507-
return []byte("---\nprivate: false\n---\n\n# Review\n"), nil
507+
return []byte("---\nprivate: true\n---\n\n# Review\n"), nil
508508
default:
509509
return nil, createRepositoryPackageNotFoundError(path)
510510
}
@@ -515,7 +515,7 @@ func TestResolveWorkflows_RepositoryPackageRejectsPrivateFalse(t *testing.T) {
515515
}
516516
fetchWorkflowFromSourceWithContextFn = func(_ context.Context, spec *WorkflowSpec, _ bool) (*FetchedWorkflow, error) {
517517
return &FetchedWorkflow{
518-
Content: []byte("---\nprivate: false\n---\n\n# Review\n"),
518+
Content: []byte("---\nprivate: true\n---\n\n# Review\n"),
519519
CommitSHA: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
520520
IsLocal: false,
521521
SourcePath: spec.WorkflowPath,
@@ -524,7 +524,7 @@ func TestResolveWorkflows_RepositoryPackageRejectsPrivateFalse(t *testing.T) {
524524

525525
_, err := ResolveWorkflows(context.Background(), []string{"owner/repo"}, false)
526526
require.Error(t, err)
527-
assert.Contains(t, err.Error(), `workflow "workflows/review.md" sets private: false`)
527+
assert.Contains(t, err.Error(), `workflow "workflows/review.md" sets private: true`)
528528
}
529529

530530
func TestResolveWorkflows_NestedRepositoryPackage(t *testing.T) {

pkg/cli/add_workflow_resolution.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -216,12 +216,9 @@ func ResolveWorkflows(ctx context.Context, workflows []string, verbose bool) (*R
216216

217217
if spec.FromRepositoryManifest {
218218
privateValue, hasPrivate := ExtractWorkflowPrivateSetting(string(fetched.Content))
219-
if hasPrivate {
219+
if hasPrivate && privateValue {
220220
manifestPath := joinRepositoryPackagePath(spec.PackagePath, repositoryPackageManifestFileName)
221-
if privateValue {
222-
return nil, fmt.Errorf("invalid Agentic Workflow manifest %q: workflow %q sets private: true and cannot be included because private workflows cannot be added", manifestPath, resolvedSpec.WorkflowPath)
223-
}
224-
return nil, fmt.Errorf("invalid Agentic Workflow manifest %q: workflow %q sets private: false; remove the private field because manifest-listed workflows must not declare it", manifestPath, resolvedSpec.WorkflowPath)
221+
return nil, fmt.Errorf("invalid Agentic Workflow manifest %q: workflow %q sets private: true and cannot be included because private workflows cannot be added", manifestPath, resolvedSpec.WorkflowPath)
225222
}
226223
}
227224

pkg/cli/compile_repository_manifest_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,8 @@ name: Repo Assist
180180
assert.Contains(t, err.Error(), "missing required README.md")
181181
}
182182

183-
func TestCompileWorkflows_RejectsManifestWorkflowWithPrivateFalse(t *testing.T) {
184-
tmpDir := testutil.TempDir(t, "aw-manifest-private-false-*")
183+
func TestCompileWorkflows_RejectsManifestWorkflowWithPrivateTrue(t *testing.T) {
184+
tmpDir := testutil.TempDir(t, "aw-manifest-private-true-*")
185185
originalWd, err := os.Getwd()
186186
require.NoError(t, err)
187187
t.Cleanup(func() { _ = os.Chdir(originalWd) })
@@ -193,7 +193,7 @@ func TestCompileWorkflows_RejectsManifestWorkflowWithPrivateFalse(t *testing.T)
193193

194194
require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "workflows"), 0o755))
195195
require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "workflows", "review.md"), []byte(`---
196-
private: false
196+
private: true
197197
---
198198
199199
# Review
@@ -207,7 +207,7 @@ files:
207207

208208
_, err = CompileWorkflows(context.Background(), CompileConfig{})
209209
require.Error(t, err)
210-
assert.Contains(t, err.Error(), `workflow "workflows/review.md" sets private: false`)
210+
assert.Contains(t, err.Error(), `workflow "workflows/review.md" sets private: true`)
211211
}
212212

213213
func TestValidateRepositoryManifestForCompilation_PropagatesGitRootErrors(t *testing.T) {

0 commit comments

Comments
 (0)