Mark all agentic workflows as private#40048
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. PR #40048 ('Mark all agentic workflows as private') contains only workflow .md and .lock.yml file changes — no *_test.go, *.test.cjs, or *.test.js files. Test Quality Sentinel skipped. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
❌ Design Decision Gate 🏗️ failed during design decision gate check. |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /zoom-out — approving this clean bulk policy change.
📋 Key Themes & Highlights
Positive Highlights
- ✅ Exactly 245 locally-owned workflow
.mdfiles updated — none missed - ✅ 5 upstream-managed files (
agentic-token-audit,agentic-token-optimizer,ci-doctor,daily-team-status,dependabot-repair) correctly skipped — they carrysource:frontmatter and are read-only - ✅ 97 shared workflow files in
shared/correctly excluded — they have no frontmatter - ✅ Lock files updated with only
frontmatter_hashchanges;body_hashis identical in all 245, confirming no workflow logic was altered - ✅
private: trueis consistently placed as the first key in every frontmatter block (line 2 of each.mdfile)
Minor Observation
ci-coach.md shows an inflated diff (346 adds / 344 dels) due to opportunistic CRLF → LF line-ending normalization; the semantic change is still only private: true. See inline comment for details.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer
| pull-requests: read | ||
| issues: read | ||
| --- | ||
| private: true |
There was a problem hiding this comment.
[/zoom-out] ci-coach.md shows 346 additions / 344 deletions instead of the expected +1/-0 pattern shared by the other 244 files — this is a line-ending normalization, not a content change.
💡 What happened
The file originally had mixed line endings (CRLF, bare CR, and LF). Converting them all to LF plus inserting private: true causes git to diff the entire file as changed. The lock file's body_hash is identical before and after, confirming no semantic change. No action needed.
There was a problem hiding this comment.
Non-blocking observation
The mechanical execution is correct: exactly 245 non-upstream .md files stamped with private: true, lock files regenerated with updated frontmatter_hash and unchanged body_hash (confirmed across all 245 files), 5 upstream-managed files correctly excluded, and no extraneous files touched.
One gap: shared imports not covered
69 files in .github/workflows/shared/ have no private: true frontmatter. The enforcement code in add_workflow_resolution.go calls ExtractWorkflowPrivate() on the fetched content — these files return false, so installation is not blocked. isSupportedPackageInstallablePath() accepts any .md path starting with .github/workflows/ (the subdirectory restriction only applies to .yml files), so these shared imports satisfy the path check and can be installed to external repos via gh aw add github/gh-aw/shared/<name>.
A follow-up PR to add private: true to all 69 shared imports would close this gap. No recompile step is needed for them since they have no .lock.yml counterparts.
🔎 Code quality review by PR Code Quality Reviewer
| @@ -1,4 +1,5 @@ | |||
| --- | |||
| private: true | |||
There was a problem hiding this comment.
Coverage gap: shared imports not marked private — 69 files in .github/workflows/shared/ have no private: true frontmatter. Since ExtractWorkflowPrivate() returns false for them and isSupportedPackageInstallablePath() accepts any path starting with .github/workflows/ (including the shared/ subdirectory), an external user can run gh aw add github/gh-aw/shared/<name> and successfully copy any shared import into their repo.
💡 Suggested follow-up fix
ExtractWorkflowPrivateSetting returns (false, false) for these files because none have a private: YAML key — the YAML comments inside their frontmatter blocks parse as an empty map.
Fix: add a follow-up PR to stamp all 69 shared/*.md files with private: true using the same approach as this PR. For files with a --- block:
---
private: true
# CI Data AnalysisFor pure-markdown imports without frontmatter (e.g., noop-reminder.md), add a frontmatter block:
---
private: true
---
**Important**: If no action is needed...No recompile needed for shared imports — they have no .lock.yml counterpart.
| @@ -1,4 +1,5 @@ | |||
| --- | |||
| private: true | |||
There was a problem hiding this comment.
gh aw trial bypasses the private: true guard — installWorkflowInTrialMode calls FetchWorkflowFromSourceWithContext directly and then writes the workflow content to a cloned trial host repository without ever calling ExtractWorkflowPrivate. The enforcement only exists in add_workflow_resolution.go (the gh aw add code path). A user can run gh aw trial github/gh-aw/<any-workflow> and successfully install and execute any of the 245 newly-private workflows in a real temporary repository.
💡 Suggested fix
Add a private guard in pkg/cli/trial_repository.go:installWorkflowInTrialMode immediately after the fetch succeeds, before writeWorkflowToTrialDir:
// Enforce private: true — private workflows cannot be trialled from external repos
if !fetched.IsLocal && ExtractWorkflowPrivate(string(content)) {
return fmt.Errorf("workflow '%s' is private and cannot be run in trial mode from external repositories", parsedSpec.WorkflowName)
}This mirrors the same check already performed in add_workflow_resolution.go:226-228 for gh aw add. The !fetched.IsLocal guard ensures that local workflows (which the repo owner is running in their own context) are still allowed.
A test case should accompany the fix, since no existing test covers this path (the existing add_private_test.go tests only exercise ExtractWorkflowPrivate in isolation, not the trial installation path).
Workflows in this repo should not be installable into external repos via
gh aw add. Addingprivate: trueto frontmatter enforces this.Changes
private: trueto the frontmatter of all 245 locally-owned workflow.mdfilesagentic-token-audit.md,agentic-token-optimizer.md,ci-doctor.md,daily-team-status.md,dependabot-repair.md) — these carry asource:frontmatter entry and are read-only in this repoExample