Refactor agentic-workflows dispatcher from legacy agent file to skill#35580
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot find occurrences of /agent and replace it with skill calling |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #35580 does not have the 'implementation' label and has only 56 new lines of code in business logic directories (≤100 threshold). |
|
🧪 Test Quality Sentinel completed test quality analysis. |
There was a problem hiding this comment.
Pull request overview
This PR migrates the agentic-workflows dispatcher from the legacy custom agent file (.github/agents/agentic-workflows.agent.md) to the skill format (.github/skills/agentic-workflows/SKILL.md) and updates CLI flows, upgrade/cleanup behavior, and documentation to match.
Changes:
- Update init/fix/upgrade CLI flows to manage the dispatcher as a skill (and download the template from the upstream skill path).
- Extend migration cleanup to remove the legacy dispatcher agent file during old-agent cleanup, while allowing upgrade PR staging to capture deletions.
- Update repository docs/workflows/action guidance to reference skills instead of agents.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/upgrade_command.go | Updates upgrade messaging around updating the dispatcher (now a skill). |
| pkg/cli/init.go | Updates init flow logs/messages and PR body wording to refer to “skill”. |
| pkg/cli/init_command.go | Updates gh aw init help text to reference the new skill path and skill invocation wording. |
| pkg/cli/init_command_test.go | Updates init tests to assert the skill file is not created for non-Copilot engines. |
| pkg/cli/fix_command.go | Updates fix flow logs/messages to refer to “skill”. |
| pkg/cli/copilot_agents.go | Writes dispatcher to .github/skills/agentic-workflows/SKILL.md and expands legacy .github/agents cleanup to delete the old dispatcher agent file. |
| pkg/cli/copilot_agents_test.go | Updates deletion tests to include the legacy dispatcher agent file. |
| pkg/cli/agent_download.go | Switches download source URL to the skill template path and updates related logging/messages. |
| install.md | Updates installation/setup instructions to reference the dispatcher skill file and skill invocation. |
| docs/src/content/docs/troubleshooting/debugging.md | Updates troubleshooting instructions to invoke the skill instead of /agent. |
| docs/src/content/docs/troubleshooting/debug-ghe.md | Updates GHE debugging steps to reference the skill. |
| docs/src/content/docs/troubleshooting/common-issues.md | Updates sample debugging invocations to use the skill. |
| docs/src/content/docs/setup/creating-workflows.mdx | Updates setup guidance to refer to the dispatcher skill and skill invocation. |
| docs/src/content/docs/setup/cli.md | Updates CLI docs for init to reference the dispatcher skill file. |
| docs/src/content/docs/reference/glossary.md | Renames “Agent Files” glossary entry to “Skill Files” and updates description. |
| docs/src/content/docs/reference/faq.md | Updates FAQ examples to use skill invocation wording. |
| docs/src/content/docs/reference/custom-agent-for-aw.mdx | Renames and updates the guide to “Skills” terminology and examples. |
| docs/src/content/docs/guides/upgrading.md | Updates upgrade guide to reference .github/skills/... and staging paths. |
| docs/src/content/docs/guides/agentic-authoring.mdx | Updates Copilot invocation examples to use the skill. |
| docs/src/content/docs/examples/maintaining-repos.md | Updates examples to use skill invocation. |
| debug.md | Updates debugging prompt docs to refer to the skill. |
| create.md | Updates creation docs to reference the skill file path. |
| CONTRIBUTING.md | Updates contributor prompt guidance to link to the skill template. |
| AGENTS.md | Updates repo overview guidance to refer to the agentic-workflows skill. |
| actions/setup/md/missing_tool_issue.md | Updates setup issue template to invoke the skill. |
| actions/setup/md/missing_data_issue.md | Updates setup issue template to invoke the skill. |
| actions/setup/md/agent_failure_issue.md | Updates failure template to load the skill from the new path/URL. |
| actions/setup/js/run_operation_update_upgrade.test.cjs | Updates upgrade PR staging expectations to include the skill file and legacy agent deletion staging. |
| actions/setup/js/run_operation_update_upgrade.cjs | Adds the skill file to the upgrade allowlist while keeping legacy agent path for deletion staging. |
| .github/workflows/workflow-generator.md | Updates generator workflow instructions to reference the skill file. |
| .github/workflows/workflow-generator.lock.yml | Regenerates lock metadata after prompt changes. |
| .github/workflows/cgo.yml | Updates CI check wording/path to validate skill file URL refs. |
| .github/skills/agentic-workflows/SKILL.md | Updates dispatcher content to the skill model and removes legacy self-references. |
| .github/aw/cli-commands.md | Updates CLI reference to point to the dispatcher skill path. |
| .github/agents/agentic-workflows.agent.md | Removes the legacy dispatcher agent file from the repo. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 35/35 changed files
- Comments generated: 5
| // downloadAgentFileFromGitHub downloads the agentic-workflows SKILL.md file from GitHub. | ||
| func downloadAgentFileFromGitHub(verbose bool) (string, error) { | ||
| agentDownloadLog.Print("Downloading agentic-workflows.agent.md from GitHub") | ||
| agentDownloadLog.Print("Downloading agentic-workflows SKILL.md from GitHub") |
| @@ -98,7 +98,7 @@ After successful initialization, the user can: | |||
|
|
|||
| - **Add workflows from repos**: `gh aw add githubnext/agentics` | |||
| - **Create new workflows**: `gh aw new <workflow-name>` os using the agent | |||
| https://github.com/owner/repo/actions/runs/RUN_ID | ||
|
|
||
| Load [https://github.com/github/gh-aw/.github/agents/agentic-workflows.agent.md and](https://github.com/github/gh-aw/blob/main/.github/agents/agentic-workflows.agent.md) investigate: | ||
| Load [https://github.com/github/gh-aw/.github/skills/agentic-workflows/SKILL.md and](https://github.com/github/gh-aw/blob/main/.github/skills/agentic-workflows/SKILL.md) investigate: |
| ## Installing the Copilot Agent Files for Agentic Workflows | ||
| ## Installing the Copilot Skill Files for Agentic Workflows | ||
|
|
||
| Follow these steps to set up your repository for agentic workflows using the custom `agentic-workflows` agent. |
| ## Debugging Workflows | ||
|
|
||
| Use the agentic workflows agent to diagnose and fix failing workflow runs. | ||
|
|
||
| ### Through Copilot | ||
|
|
||
| If your repository is [configured for agentic authoring](#configuring-your-repository), use the `agentic-workflows` agent in Copilot Chat: | ||
| If your repository is [configured for agentic authoring](#configuring-your-repository), use the `agentic-workflows` skill in Copilot Chat: | ||
|
|
🧪 Test Quality Sentinel Report
📊 Metrics & Test Classification (3 tests analyzed)
Test Classification Details
Language SupportTests analyzed:
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /improve-codebase-architecture and /tdd — commenting with non-blocking suggestions.
📋 Key Themes & Highlights
Key Themes
- Stale function names:
downloadAgentFileFromGitHub,downloadAgentFileViaGHCLI,updateAgentFiles, anddeleteOldAgentFilesstill use "agent" vocabulary after the migration to "skill". The path strings are correct; the function names are the remaining inconsistency. - Wrong file permission:
SKILL.mdis written withFilePermSensitive(0600) inherited from the old agent-file logic. The comment was updated to say "Skill files may contain sensitive configuration" but that's not accurate — this is a public instruction file.FilePermPublic(0644) is the right constant and already exists inpkg/constants. - Missing positive test: The updated test in
init_command_test.goonly asserts the negative case (SKILL.md must not be created for non-Copilot engines). A positive assertion that init does create the file at the new path is absent.
Positive Highlights
- ✅ Clean, mechanical migration — path strings and URL constructions are all correctly updated
- ✅ Migration cleanup logic (
deleteOldAgentFilesnow includesagentic-workflows.agent.md) is well-placed and tested - ✅ Upgrade staging allowlist in
run_operation_update_upgrade.cjscorrectly covers both the new skill path and the legacy agent path for atomic deletion - ✅ Broad docs/help-text sweep is thorough and consistent
- ✅ Net -231 lines — good deletion-to-addition ratio for a refactor of this scope
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 1.9M
| // Write the file with restrictive permissions (0600) to follow security best practices | ||
| // Agent files may contain sensitive configuration | ||
| // Skill files may contain sensitive configuration | ||
| if err := os.WriteFile(targetPath, []byte(agentContent), constants.FilePermSensitive); err != nil { |
There was a problem hiding this comment.
[/improve-codebase-architecture] SKILL.md is written with FilePermSensitive (0600), but the accompanying comment now reads "Skill files may contain sensitive configuration" — which is not true for a public instruction file committed to the repo.
💡 Suggestion
Switch to FilePermPublic (0o644) which already exists in pkg/constants:
// Skill files are public instruction files; use standard readable permissions.
if err := os.WriteFile(targetPath, []byte(agentContent), constants.FilePermPublic); err != nil {Using FilePermSensitive on a file that users and CI jobs need to read (and that lives in .github/) can cause subtle permission issues and is inconsistent with the file's nature.
|
|
||
| // downloadAgentFileFromGitHub downloads the agentic-workflows.agent.md file from GitHub | ||
| // downloadAgentFileFromGitHub downloads the agentic-workflows SKILL.md file from GitHub. | ||
| func downloadAgentFileFromGitHub(verbose bool) (string, error) { |
There was a problem hiding this comment.
[/improve-codebase-architecture] Function name downloadAgentFileFromGitHub (and its fallback downloadAgentFileViaGHCLI) still use the old "agent" vocabulary even though the function now fetches a skill file.
💡 Suggestion
Rename to downloadSkillFileFromGitHub / downloadSkillFileViaGHCLI to stay consistent with the migration. The same applies to updateAgentFiles in upgrade_command.go and deleteOldAgentFiles in copilot_agents.go — these function names are now misleading when the dominant concept is "skill".
Leaving the rename as follow-up work is fine, but a //nolint:revive // TODO: rename comment keeps the intent visible.
|
|
||
| if _, err := os.Stat(filepath.Join(".github", "agents", "agentic-workflows.agent.md")); err == nil { | ||
| t.Error("Expected dispatcher agent file to NOT be created for non-Copilot engine") | ||
| if _, err := os.Stat(filepath.Join(".github", "skills", "agentic-workflows", "SKILL.md")); err == nil { |
There was a problem hiding this comment.
[/tdd] This test only covers the negative case (SKILL.md must NOT be created for non-Copilot engines). There is no test asserting that gh aw init with a Copilot engine creates .github/skills/agentic-workflows/SKILL.md at the correct path — meaning a path regression in copilot_agents.go would go undetected.
💡 Suggested test addition
Add an assertion in TestInitRepositoryBasic (or a dedicated test) that confirms the skill file is created:
skillPath := filepath.Join(".github", "skills", "agentic-workflows", "SKILL.md")
if _, err := os.Stat(skillPath); err != nil {
t.Errorf("Expected dispatcher skill file to be created at %s: %v", skillPath, err)
}The test can mock downloadAgentFileFromGitHub or use a local fixture — the key is that a path change in the production code immediately turns this test red.
There was a problem hiding this comment.
REQUEST_CHANGES — migration cleanup is wired correctly in fix but missing from both upgrade and init, so the old dispatcher agent file survives those paths and two dispatcher definitions coexist.
### Blocking issues (2 high/medium)
upgrade_command.go/updateAgentFiles— never callsdeleteOldAgentFiles. Aftergh aw upgradethe newSKILL.mdis written but.github/agents/agentic-workflows.agent.mdis left on disk. Copilot will load both files with undefined resolution order.init.go— same omission. A repo that already has the legacy agent file installed retains it aftergh aw init.
fix_command.go already demonstrates the correct two-step pattern; replicate it in both callers.
The JS staging allowlist comment (// Old agent files that may be deleted by deleteOldAgentFiles) is also incorrect as a follow-on consequence — the file is never deleted by upgrade, so staging it changes nothing.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 2.7M
Comments that could not be inline-anchored
pkg/cli/upgrade_command.go:366
updateAgentFiles leaves the old dispatcher agent file on disk — after gh aw upgrade, .github/agents/agentic-workflows.agent.md is never deleted, so both the legacy agent and the new SKILL.md coexist.
<details>
<summary>💡 Fix</summary>
fix_command.go correctly calls both ensureAgenticWorkflowsDispatcher and deleteOldAgentFiles. The same pattern must be applied here:
func updateAgentFiles(ctx context.Context, verbose bool) error {
if err := ensureAgenticWorkflowsDisp…
</details>
<details><summary>pkg/cli/init.go:97</summary>
**`init` also skips `deleteOldAgentFiles`** — a repo that already has `.github/agents/agentic-workflows.agent.md` (e.g. cloned from a pre-migration template) will silently retain the old file after `gh aw init`.
<details>
<summary>💡 Fix</summary>
Add the same call that `fix_command.go` makes, right after `ensureAgenticWorkflowsDispatcher` succeeds:
```go
if err := deleteOldAgentFiles(opts.Verbose); err != nil {
initLog.Printf("Failed to delete old agent files: %v", err)
// non-fatal…
</details>
<details><summary>actions/setup/js/run_operation_update_upgrade.cjs:21</summary>
**Misleading comment**: `gh aw upgrade` does not call `deleteOldAgentFiles`.
<details>
<summary>💡 Details</summary>
The comment on the next line (`// Old agent files that may be deleted by deleteOldAgentFiles:`) implies the upgrade pipeline deletes the legacy agent file. It does not — `deleteOldAgentFiles` is only called from `fix_command.go`. As a result, `git add -- .github/agents/agentic-workflows.agent.md` stages an **unchanged existing file**, not a deletion. The old file stays on disk …
</details>|
|
|
@copilot no. Review all comments and reviews from copilot and github-actions actor. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in I reviewed the remaining Copilot and
The only review note still outstanding is an unrelated lockfile comment on |
|
|
This PR migrates the
agentic-workflowsdispatcher from the legacy custom agent file to the skill format and aligns init/upgrade flows with that model. It also adds migration cleanup so repositories are moved forward without keeping stale agent artifacts.Dispatcher source moved to skill path
init,fix, andupgradenow manage.github/skills/agentic-workflows/SKILL.mdinstead of.github/agents/agentic-workflows.agent.md.Codemod + upgrade migration behavior
.github/agents/agentic-workflows.agent.mdin old-file deletion codemods.Repository/docs alignment
.github/agents/agentic-workflows.agent.mdfrom this repository..github/skills/agentic-workflows/SKILL.md.Targeted test updates