-
Notifications
You must be signed in to change notification settings - Fork 435
Enforce canonical dispatch-repository safe-output key (deprecate underscore alias)
#42150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
eedbbc0
b90ac33
e35808a
f706b8f
453e85c
b18f3a7
4406350
3a842f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # ADR-42150: Make `dispatch-repository` the Canonical Safe-Output Key | ||
|
|
||
| **Date**: 2026-06-29 | ||
| **Status**: Draft | ||
| **Deciders**: pelikhan, copilot-swe-agent | ||
|
|
||
| --- | ||
|
|
||
| ### Context | ||
|
|
||
| The `dispatch_repository` safe-output type allowed agents to trigger `repository_dispatch` events in external repositories. When first introduced, both the underscore form (`dispatch_repository`) and the dashed form (`dispatch-repository`) were accepted at runtime as aliases of each other. This created a mismatch between the runtime behavior and the rest of the safe-output naming convention, where all other types use hyphen-case (e.g., `dispatch-workflow`, `call-workflow`, `create-check-run`). The drift between runtime behavior, the JSON schema, and the public documentation made the contract ambiguous for both users and tooling. A codemod-based migration path allows backward compatibility to be preserved while the canonical key is established. | ||
|
|
||
| ### Decision | ||
|
|
||
| We will make `dispatch-repository` (hyphen) the canonical key for this safe-output type, demote `dispatch_repository` (underscore) to a deprecated backward-compatible alias in the JSON schema and runtime parser, and ship a `safe-output-dispatch-repository-key` codemod that automatically renames existing frontmatter. All documentation, warning messages, and schema definitions are updated to reference only the dashed form. The underscore alias is preserved in the schema as a `$ref` to avoid hard-breaking existing workflows while the codemod propagates. | ||
|
|
||
| ### Alternatives Considered | ||
|
|
||
| #### Alternative 1: Keep `dispatch_repository` (underscore) as canonical | ||
|
|
||
| The underscore key could remain primary and the dashed key could become the alias, matching Go struct field conventions. This was rejected because it contradicts the established hyphen-case naming pattern shared by every other safe-output type, and would make the public API feel inconsistent with its siblings. | ||
|
|
||
| #### Alternative 2: Hard removal — drop the underscore key with no alias | ||
|
|
||
| The underscore key could be removed entirely in a single release with no backward-compatible alias, requiring an immediate breaking migration. This was rejected because it would silently break all existing workflow files using `dispatch_repository` without any automated migration path, violating the project's convention of providing codemods for deprecations. | ||
|
|
||
| ### Consequences | ||
|
|
||
| #### Positive | ||
| - Schema, runtime, documentation, and compiler warnings are now consistent: all references use `dispatch-repository`. | ||
| - Aligns with every other safe-output type (`dispatch-workflow`, `call-workflow`, `create-check-run`, etc.), reducing cognitive overhead for new users. | ||
| - The automated codemod (`safe-output-dispatch-repository-key`) lets existing users upgrade without manual edits. | ||
|
|
||
| #### Negative | ||
| - The JSON schema retains a `dispatch_repository` `$ref` alias entry, adding a small amount of schema complexity that must be maintained until the alias can be removed. | ||
| - Any downstream tooling or documentation outside this repository that hard-codes the underscore key will require an update. | ||
|
|
||
| #### Neutral | ||
| - The runtime parser lookup order is inverted: `dispatch-repository` is now checked first, then `dispatch_repository` as fallback — behavior is identical for users until the alias is eventually removed. | ||
| - Compiler warning text changes from `dispatch_repository` to `dispatch-repository`; any scripts that grep for the old warning string will need updating. | ||
|
|
||
| --- | ||
|
|
||
| *ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| package cli | ||
|
|
||
| import ( | ||
| "strings" | ||
|
|
||
| "github.com/github/gh-aw/pkg/logger" | ||
| ) | ||
|
|
||
| var safeOutputDispatchRepositoryKeyCodemodLog = logger.New("cli:codemod_safe_output_dispatch_repository_key") | ||
|
|
||
| func getSafeOutputDispatchRepositoryKeyCodemod() Codemod { | ||
| return Codemod{ | ||
| ID: "safe-output-dispatch-repository-key", | ||
| Name: "Rename safe-outputs.dispatch_repository to dispatch-repository", | ||
| Description: "Renames deprecated safe-outputs.dispatch_repository to safe-outputs.dispatch-repository.", | ||
| IntroducedIn: "1.0.65", | ||
| Apply: func(content string, frontmatter map[string]any) (string, bool, error) { | ||
| if safeOutputDispatchRepositoryKeyHasBothKeys(frontmatter) { | ||
| safeOutputDispatchRepositoryKeyCodemodLog.Print("WARN: safe-outputs has both dispatch_repository and dispatch-repository; manual review needed, skipping migration") | ||
| return content, false, nil | ||
| } | ||
| if !safeOutputDispatchRepositoryKeyNeedsMigration(frontmatter) { | ||
| return content, false, nil | ||
| } | ||
|
|
||
| newContent, applied, err := applyFrontmatterLineTransform(content, renameSafeOutputDispatchRepositoryKey) | ||
| if applied { | ||
| safeOutputDispatchRepositoryKeyCodemodLog.Print("Renamed safe-outputs.dispatch_repository to safe-outputs.dispatch-repository") | ||
| } | ||
| return newContent, applied, err | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func safeOutputDispatchRepositoryKeyHasBothKeys(frontmatter map[string]any) bool { | ||
| safeOutputsAny, ok := frontmatter["safe-outputs"] | ||
| if !ok { | ||
| return false | ||
| } | ||
| safeOutputsMap, ok := safeOutputsAny.(map[string]any) | ||
| if !ok { | ||
| return false | ||
| } | ||
| _, hasOld := safeOutputsMap["dispatch_repository"] | ||
| _, hasNew := safeOutputsMap["dispatch-repository"] | ||
| return hasOld && hasNew | ||
| } | ||
|
|
||
| func safeOutputDispatchRepositoryKeyNeedsMigration(frontmatter map[string]any) bool { | ||
| safeOutputsAny, ok := frontmatter["safe-outputs"] | ||
| if !ok { | ||
| return false | ||
| } | ||
| safeOutputsMap, ok := safeOutputsAny.(map[string]any) | ||
| if !ok { | ||
| return false | ||
| } | ||
| _, hasOld := safeOutputsMap["dispatch_repository"] | ||
| _, hasNew := safeOutputsMap["dispatch-repository"] | ||
| return hasOld && !hasNew | ||
| } | ||
|
|
||
| func renameSafeOutputDispatchRepositoryKey(lines []string) ([]string, bool) { | ||
| result := make([]string, 0, len(lines)) | ||
| modified := false | ||
|
|
||
| inSafeOutputs := false | ||
| safeOutputsIndent := "" | ||
| safeOutputsChildIndent := "" | ||
|
|
||
| for i, line := range lines { | ||
| trimmed := strings.TrimSpace(line) | ||
| indent := getIndentation(line) | ||
|
|
||
| if !strings.HasPrefix(trimmed, "#") { | ||
| if inSafeOutputs && hasExitedBlock(line, safeOutputsIndent) { | ||
| inSafeOutputs = false | ||
| safeOutputsChildIndent = "" | ||
| } | ||
| } | ||
|
|
||
| if strings.HasPrefix(trimmed, "safe-outputs:") { | ||
| inSafeOutputs = true | ||
| safeOutputsIndent = indent | ||
| safeOutputsChildIndent = "" | ||
| result = append(result, line) | ||
| continue | ||
| } | ||
|
|
||
| if inSafeOutputs && isDescendant(indent, safeOutputsIndent) && !strings.HasPrefix(trimmed, "#") { | ||
| if safeOutputsChildIndent == "" { | ||
| safeOutputsChildIndent = indent | ||
| } | ||
| if indent == safeOutputsChildIndent && strings.HasPrefix(trimmed, "dispatch_repository:") { | ||
| newLine, replaced := findAndReplaceInLine(line, "dispatch_repository", "dispatch-repository") | ||
| if replaced { | ||
| result = append(result, newLine) | ||
| modified = true | ||
| safeOutputDispatchRepositoryKeyCodemodLog.Printf("Renamed dispatch_repository to dispatch-repository in safe-outputs on line %d", i+1) | ||
| continue | ||
| } | ||
| } | ||
| } | ||
|
|
||
| result = append(result, line) | ||
| } | ||
|
|
||
| return result, modified | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,166 @@ | ||
| //go:build !integration | ||
|
|
||
| package cli | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestSafeOutputDispatchRepositoryKeyCodemod(t *testing.T) { | ||
| codemod := getSafeOutputDispatchRepositoryKeyCodemod() | ||
|
|
||
| t.Run("metadata", func(t *testing.T) { | ||
| assert.Equal(t, "safe-output-dispatch-repository-key", codemod.ID) | ||
| assert.Equal(t, "Rename safe-outputs.dispatch_repository to dispatch-repository", codemod.Name) | ||
| assert.Equal(t, "Renames deprecated safe-outputs.dispatch_repository to safe-outputs.dispatch-repository.", codemod.Description) | ||
| assert.Equal(t, "1.0.65", codemod.IntroducedIn) | ||
| require.NotNil(t, codemod.Apply) | ||
| }) | ||
|
|
||
| t.Run("renames safe-outputs dispatch_repository key", func(t *testing.T) { | ||
| content := `--- | ||
| on: workflow_dispatch | ||
| safe-outputs: | ||
| dispatch_repository: | ||
| relay: | ||
| workflow: router.yml | ||
| event_type: dispatch | ||
| repository: github/gh-aw | ||
| --- | ||
|
|
||
| Body text. | ||
| ` | ||
| frontmatter := map[string]any{ | ||
| "on": "workflow_dispatch", | ||
| "safe-outputs": map[string]any{ | ||
| "dispatch_repository": map[string]any{ | ||
| "relay": map[string]any{ | ||
| "workflow": "router.yml", | ||
| "event_type": "dispatch", | ||
| "repository": "github/gh-aw", | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| result, applied, err := codemod.Apply(content, frontmatter) | ||
| require.NoError(t, err) | ||
| assert.True(t, applied) | ||
| assert.Contains(t, result, " dispatch-repository:") | ||
| assert.NotContains(t, result, " dispatch_repository:") | ||
| assert.Contains(t, result, "\n\nBody text.") | ||
| }) | ||
|
|
||
| t.Run("preserves comments and indentation", func(t *testing.T) { | ||
| content := `--- | ||
| safe-outputs: | ||
| # relay config | ||
| dispatch_repository: # inline comment | ||
| relay: | ||
| workflow: router.yml | ||
| event_type: dispatch | ||
| repository: github/gh-aw | ||
| --- | ||
| ` | ||
| frontmatter := map[string]any{ | ||
| "safe-outputs": map[string]any{ | ||
| "dispatch_repository": map[string]any{ | ||
| "relay": map[string]any{ | ||
| "workflow": "router.yml", | ||
| "event_type": "dispatch", | ||
| "repository": "github/gh-aw", | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| result, applied, err := codemod.Apply(content, frontmatter) | ||
| require.NoError(t, err) | ||
| assert.True(t, applied) | ||
| assert.Contains(t, result, " dispatch-repository: # inline comment") | ||
| assert.Contains(t, result, " # relay config") | ||
| }) | ||
|
|
||
| t.Run("no-op when deprecated key absent", func(t *testing.T) { | ||
| content := `--- | ||
| safe-outputs: | ||
| dispatch-repository: | ||
| relay: | ||
| workflow: router.yml | ||
| event_type: dispatch | ||
| repository: github/gh-aw | ||
| --- | ||
| ` | ||
| frontmatter := map[string]any{ | ||
| "safe-outputs": map[string]any{ | ||
| "dispatch-repository": map[string]any{ | ||
| "relay": map[string]any{ | ||
| "workflow": "router.yml", | ||
| "event_type": "dispatch", | ||
| "repository": "github/gh-aw", | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| result, applied, err := codemod.Apply(content, frontmatter) | ||
| require.NoError(t, err) | ||
| assert.False(t, applied) | ||
| assert.Equal(t, content, result) | ||
| }) | ||
|
|
||
| t.Run("no-op when both keys already exist", func(t *testing.T) { | ||
| content := `--- | ||
| safe-outputs: | ||
| dispatch-repository: | ||
| canonical: | ||
| workflow: router.yml | ||
| event_type: dispatch | ||
| repository: github/gh-aw | ||
| dispatch_repository: | ||
| alias: | ||
| workflow: router.yml | ||
| event_type: dispatch | ||
| repository: github/gh-aw | ||
| --- | ||
| ` | ||
| frontmatter := map[string]any{ | ||
| "safe-outputs": map[string]any{ | ||
| "dispatch-repository": map[string]any{}, | ||
| "dispatch_repository": map[string]any{}, | ||
| }, | ||
| } | ||
|
|
||
| result, applied, err := codemod.Apply(content, frontmatter) | ||
| require.NoError(t, err) | ||
| assert.False(t, applied) | ||
| assert.Equal(t, content, result) | ||
| }) | ||
|
|
||
| t.Run("no-op when dispatch_repository appears only in a description value", func(t *testing.T) { | ||
| content := `--- | ||
| safe-outputs: | ||
| some-tool: | ||
| description: "Triggers via dispatch_repository: mechanism" | ||
| workflow: router.yml | ||
| event_type: dispatch | ||
| repository: github/gh-aw | ||
| --- | ||
| ` | ||
| frontmatter := map[string]any{ | ||
| "safe-outputs": map[string]any{ | ||
| "some-tool": map[string]any{ | ||
| "description": "Triggers via dispatch_repository: mechanism", | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| result, applied, err := codemod.Apply(content, frontmatter) | ||
| require.NoError(t, err) | ||
| assert.False(t, applied) | ||
| assert.Equal(t, content, result) | ||
| }) | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] The four test cases are a solid set, but there is a gap: the codemod skips migration when 💡 Suggested additional testt.Run("no-op when dispatch_repository appears only in a description value", func(t *testing.T) {
content := `---
safe-outputs:
some-tool:
description: "Triggers via dispatch_repository: mechanism"
workflow: router.yml
event_type: dispatch
repository: github/gh-aw
---
`
frontmatter := map[string]any{
"safe-outputs": map[string]any{
"some-tool": map[string]any{
"description": "Triggers via dispatch_repository: mechanism",
},
},
}
result, applied, err := codemod.Apply(content, frontmatter)
require.NoError(t, err)
assert.False(t, applied)
assert.Equal(t, content, result)
})@copilot please address this. |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silent no-op when both keys coexist — no user-facing signal: When a workflow has both
dispatch_repositoryanddispatch-repositoryundersafe-outputs,gh aw fixwill silently do nothing, leaving the user with a deprecated key that the parser ignores (sincedispatch-repositorytakes priority at runtime).💡 Suggested fix
At minimum emit a log that the user can see via
--log-level debug(or upgrade to a warning), so they know manual cleanup is needed:Without this,
gh aw fixis not idempotent in user-perceptible terms: running it on a dual-key file produces no output and no change, but the deprecated key persists.