Skip to content

Commit ce15603

Browse files
authored
Migrate GitHub App token input to client-id, add schema-level compatibility, and provide codemod migration (#26551)
1 parent 2ed6728 commit ce15603

22 files changed

Lines changed: 404 additions & 46 deletions
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
# ADR-26551: Migrate GitHub App Token Input from `app-id` to `client-id` with Backward Compatibility
2+
3+
**Date**: 2026-04-16
4+
**Status**: Draft
5+
**Deciders**: pelikhan, Copilot
6+
7+
---
8+
9+
## Part 1 — Narrative (Human-Friendly)
10+
11+
### Context
12+
13+
The `actions/create-github-app-token` action from GitHub has deprecated the `app-id` input field in favour of `client-id`. Compiled agentic workflows that mint GitHub App tokens were emitting upstream deprecation warnings because the code generator always emitted `with.app-id`. At the same time, existing workflow frontmatter authored by users already uses `app-id`, and a hard breaking change would silently break those workflows. The system needed to close the gap between what upstream now expects (`client-id`) and what authors had been writing (`app-id`), without requiring every user to manually update their files.
14+
15+
### Decision
16+
17+
We will migrate all generated token-minting steps to emit `client-id` instead of `app-id` when calling `actions/create-github-app-token`, while simultaneously accepting both field names in frontmatter parsing so that existing workflows continue to work. When both fields are present, `client-id` takes precedence. We will also ship an automated `gh aw fix` codemod (`github-app-app-id-to-client-id`) that rewrites `app-id` to `client-id` within `github-app` blocks, and update the JSON Schema to use `anyOf` with two valid required-field combinations (`client-id + private-key` or `app-id + private-key`). This approach eliminates deprecation noise immediately, preserves backward compatibility, and gives users a path to migrate automatically.
18+
19+
### Alternatives Considered
20+
21+
#### Alternative 1: Hard breaking change — remove `app-id` support immediately
22+
23+
Immediately stop accepting `app-id` in frontmatter and require `client-id` everywhere. This would be the simplest long-term state but would break all existing workflow configurations without warning and require every author to update their files before their workflows would compile. Given that users have no tooling to detect the breakage until compile time, this was rejected as too disruptive.
24+
25+
#### Alternative 2: Emit both `app-id` and `client-id` in generated output
26+
27+
Emit both fields in the generated YAML step to satisfy both old and new versions of `actions/create-github-app-token`. This would silence deprecation warnings on the new action but produce noisy, confusing generated output and could cause unexpected behaviour if the upstream action changes its precedence rules. It was rejected because it couples generated output to undocumented upstream precedence logic.
28+
29+
#### Alternative 3: Accept `app-id` in frontmatter only; always emit `client-id` in compiled output (chosen approach, with codemod)
30+
31+
Accept both field names in parsing, but always emit `client-id` in compiled output. Pair this with a codemod that rewrites frontmatter on demand. This cleanly separates authoring compatibility from compiled output correctness, gives a migration path, and avoids the noise of the hard-cut approach. This is the approach implemented in this PR.
32+
33+
### Consequences
34+
35+
#### Positive
36+
- Deprecation warnings from `actions/create-github-app-token` are eliminated immediately once workflows are compiled or recompiled.
37+
- Existing workflows using `app-id` continue to compile and run without modification.
38+
- Authors can migrate automatically using `gh aw fix`, reducing manual effort.
39+
- The JSON Schema accurately reflects the valid combinations, enabling editor validation for both old and new syntax.
40+
41+
#### Negative
42+
- The internal `GitHubAppConfig.AppID` field now carries ambiguous semantics — it stores whatever the user provided (`client-id` or `app-id`) under a field named `AppID`. This mismatch between the Go field name and the preferred YAML key is mild technical debt.
43+
- The `anyOf` schema constraint is harder to read and may produce less clear validation error messages than a single `required` array.
44+
- Maintaining two accepted field names indefinitely (if `app-id` is never removed) adds ongoing surface area to the parser.
45+
46+
#### Neutral
47+
- The `app-id` field in the JSON Schema is now documented as a deprecated alias, which may prompt questions from authors reading schema documentation.
48+
- All existing tests that asserted `app-id` in compiled output were updated to assert `client-id`; new tests were added for `client-id` parsing paths.
49+
- The codemod is registered in the global codemod registry and will run as part of `gh aw fix` alongside all other codemods.
50+
51+
---
52+
53+
## Part 2 — Normative Specification (RFC 2119)
54+
55+
> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).
56+
57+
### Compiled Output — Token Minting Steps
58+
59+
1. All token-minting steps generated for `actions/create-github-app-token` **MUST** emit `with.client-id`, not `with.app-id`.
60+
2. Generated workflows **MUST NOT** emit both `with.client-id` and `with.app-id` in the same step.
61+
62+
### Frontmatter Parsing — Field Acceptance
63+
64+
1. The frontmatter parser **MUST** accept `client-id` as the primary GitHub App identifier field within any `github-app` object.
65+
2. The frontmatter parser **MUST** accept `app-id` as a backward-compatible alias for `client-id` within any `github-app` object.
66+
3. When both `client-id` and `app-id` are present in the same `github-app` object, the parser **MUST** use `client-id` and **SHOULD** ignore `app-id`.
67+
4. A `github-app` object **MUST** include `private-key` alongside either `client-id` or `app-id`; objects missing `private-key` **MUST** be rejected.
68+
69+
### Schema Validation
70+
71+
1. The JSON Schema for `github-app` objects **MUST** express validity via `anyOf` with two accepted combinations: `["client-id", "private-key"]` and `["app-id", "private-key"]`.
72+
2. The `app-id` schema property **MUST** be documented as a deprecated alias for `client-id`.
73+
3. Schema examples **SHOULD** use `client-id` rather than `app-id`.
74+
75+
### Codemod
76+
77+
1. The `github-app-app-id-to-client-id` codemod **MUST** rename `app-id` keys to `client-id` only within `github-app` YAML blocks.
78+
2. The codemod **MUST NOT** rename `app-id` keys that appear outside of `github-app` blocks, regardless of nesting depth.
79+
3. The codemod **MUST** be a no-op when no `github-app.app-id` fields are present in the frontmatter.
80+
4. The codemod **MUST** be registered in the global codemod registry returned by `GetAllCodemods()`.
81+
82+
### Conformance
83+
84+
An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance.
85+
86+
---
87+
88+
*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24493163553) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package cli
2+
3+
import (
4+
"slices"
5+
"strings"
6+
7+
"github.com/github/gh-aw/pkg/logger"
8+
)
9+
10+
var githubAppClientIDCodemodLog = logger.New("cli:codemod_github_app_client_id")
11+
12+
// getGitHubAppClientIDCodemod creates a codemod that migrates github-app.app-id to github-app.client-id.
13+
func getGitHubAppClientIDCodemod() Codemod {
14+
return Codemod{
15+
ID: "github-app-app-id-to-client-id",
16+
Name: "Rename 'github-app.app-id' to 'github-app.client-id'",
17+
Description: "Renames deprecated 'app-id:' to 'client-id:' inside github-app blocks.",
18+
IntroducedIn: "0.68.4",
19+
Apply: func(content string, frontmatter map[string]any) (string, bool, error) {
20+
if !hasDeprecatedGitHubAppIDField(frontmatter) {
21+
return content, false, nil
22+
}
23+
newContent, applied, err := applyFrontmatterLineTransform(content, renameGitHubAppIDToClientID)
24+
if applied {
25+
githubAppClientIDCodemodLog.Print("Renamed github-app app-id to client-id")
26+
}
27+
return newContent, applied, err
28+
},
29+
}
30+
}
31+
32+
// hasDeprecatedGitHubAppIDField returns true when any github-app object contains app-id.
33+
func hasDeprecatedGitHubAppIDField(frontmatter map[string]any) bool {
34+
return containsDeprecatedGitHubAppID(frontmatter)
35+
}
36+
37+
func containsDeprecatedGitHubAppID(value any) bool {
38+
switch v := value.(type) {
39+
case map[string]any:
40+
for key, child := range v {
41+
if key == "github-app" {
42+
if appMap, ok := child.(map[string]any); ok {
43+
if _, hasAppID := appMap["app-id"]; hasAppID {
44+
return true
45+
}
46+
}
47+
}
48+
if containsDeprecatedGitHubAppID(child) {
49+
return true
50+
}
51+
}
52+
case []any:
53+
return slices.ContainsFunc(v, containsDeprecatedGitHubAppID)
54+
}
55+
return false
56+
}
57+
58+
func renameGitHubAppIDToClientID(lines []string) ([]string, bool) {
59+
result := make([]string, 0, len(lines))
60+
modified := false
61+
62+
inGitHubApp := false
63+
var githubAppIndent string
64+
65+
for i, line := range lines {
66+
trimmed := strings.TrimSpace(line)
67+
68+
if len(trimmed) == 0 {
69+
result = append(result, line)
70+
continue
71+
}
72+
73+
if !strings.HasPrefix(trimmed, "#") && inGitHubApp && hasExitedBlock(line, githubAppIndent) {
74+
inGitHubApp = false
75+
}
76+
77+
if strings.HasPrefix(trimmed, "github-app:") {
78+
inGitHubApp = true
79+
githubAppIndent = getIndentation(line)
80+
result = append(result, line)
81+
continue
82+
}
83+
84+
if inGitHubApp {
85+
lineIndent := getIndentation(line)
86+
if isDescendant(lineIndent, githubAppIndent) && strings.HasPrefix(trimmed, "app-id:") {
87+
newLine, replaced := findAndReplaceInLine(line, "app-id", "client-id")
88+
if replaced {
89+
result = append(result, newLine)
90+
modified = true
91+
githubAppClientIDCodemodLog.Printf("Renamed github-app app-id on line %d", i+1)
92+
continue
93+
}
94+
}
95+
}
96+
97+
result = append(result, line)
98+
}
99+
100+
return result, modified
101+
}
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
//go:build !integration
2+
3+
package cli
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestGitHubAppClientIDCodemod(t *testing.T) {
13+
codemod := getGitHubAppClientIDCodemod()
14+
15+
t.Run("renames app-id under github-app blocks", func(t *testing.T) {
16+
content := `---
17+
github-app:
18+
app-id: ${{ vars.TOP_LEVEL_APP_ID }}
19+
private-key: ${{ secrets.TOP_LEVEL_APP_PRIVATE_KEY }}
20+
on:
21+
github-app:
22+
app-id: ${{ vars.ACTIVATION_APP_ID }}
23+
private-key: ${{ secrets.ACTIVATION_APP_PRIVATE_KEY }}
24+
checkout:
25+
- repository: org/repo
26+
github-app:
27+
app-id: ${{ vars.CHECKOUT_APP_ID }}
28+
private-key: ${{ secrets.CHECKOUT_APP_PRIVATE_KEY }}
29+
---
30+
`
31+
frontmatter := map[string]any{
32+
"github-app": map[string]any{
33+
"app-id": "${{ vars.TOP_LEVEL_APP_ID }}",
34+
"private-key": "${{ secrets.TOP_LEVEL_APP_PRIVATE_KEY }}",
35+
},
36+
"on": map[string]any{
37+
"github-app": map[string]any{
38+
"app-id": "${{ vars.ACTIVATION_APP_ID }}",
39+
"private-key": "${{ secrets.ACTIVATION_APP_PRIVATE_KEY }}",
40+
},
41+
},
42+
"checkout": []any{
43+
map[string]any{
44+
"repository": "org/repo",
45+
"github-app": map[string]any{
46+
"app-id": "${{ vars.CHECKOUT_APP_ID }}",
47+
"private-key": "${{ secrets.CHECKOUT_APP_PRIVATE_KEY }}",
48+
},
49+
},
50+
},
51+
}
52+
53+
result, modified, err := codemod.Apply(content, frontmatter)
54+
require.NoError(t, err, "Should not error when applying codemod")
55+
assert.True(t, modified, "Should modify content")
56+
assert.NotContains(t, result, "app-id:", "Should remove deprecated app-id key from github-app blocks")
57+
assert.Contains(t, result, "client-id: ${{ vars.TOP_LEVEL_APP_ID }}", "Should migrate top-level github-app")
58+
assert.Contains(t, result, "client-id: ${{ vars.ACTIVATION_APP_ID }}", "Should migrate on.github-app")
59+
assert.Contains(t, result, "client-id: ${{ vars.CHECKOUT_APP_ID }}", "Should migrate checkout github-app")
60+
})
61+
62+
t.Run("does not modify content without github-app.app-id", func(t *testing.T) {
63+
content := `---
64+
github-app:
65+
client-id: ${{ vars.APP_ID }}
66+
private-key: ${{ secrets.APP_PRIVATE_KEY }}
67+
---
68+
`
69+
frontmatter := map[string]any{
70+
"github-app": map[string]any{
71+
"client-id": "${{ vars.APP_ID }}",
72+
"private-key": "${{ secrets.APP_PRIVATE_KEY }}",
73+
},
74+
}
75+
76+
result, modified, err := codemod.Apply(content, frontmatter)
77+
require.NoError(t, err, "Should not error")
78+
assert.False(t, modified, "Should not modify already migrated content")
79+
assert.Equal(t, content, result, "Content should remain unchanged")
80+
})
81+
82+
t.Run("does not rename app-id outside github-app blocks", func(t *testing.T) {
83+
content := `---
84+
engine:
85+
provider:
86+
auth:
87+
client-id: APP_CLIENT_ID
88+
tools:
89+
custom:
90+
app-id: value
91+
---
92+
`
93+
frontmatter := map[string]any{
94+
"engine": map[string]any{
95+
"provider": map[string]any{
96+
"auth": map[string]any{
97+
"client-id": "APP_CLIENT_ID",
98+
},
99+
},
100+
},
101+
"tools": map[string]any{
102+
"custom": map[string]any{
103+
"app-id": "value",
104+
},
105+
},
106+
}
107+
108+
result, modified, err := codemod.Apply(content, frontmatter)
109+
require.NoError(t, err, "Should not error")
110+
assert.False(t, modified, "Should not modify non github-app app-id keys")
111+
assert.Equal(t, content, result, "Content should remain unchanged")
112+
})
113+
}

pkg/cli/fix_codemods.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ func GetAllCodemods() []Codemod {
4747
getPlaywrightDomainsToNetworkAllowedCodemod(), // Migrate tools.playwright.allowed_domains to network.allowed
4848
getExpiresIntegerToDayStringCodemod(), // Convert expires integer (days) to string with 'd' suffix
4949
getGitHubAppCodemod(), // Rename deprecated 'app' to 'github-app'
50+
getGitHubAppClientIDCodemod(), // Rename deprecated github-app.app-id to github-app.client-id
5051
getSafeInputsToMCPScriptsCodemod(), // Rename safe-inputs to mcp-scripts
5152
getPluginsToDependenciesCodemod(), // Migrate plugins to dependencies (plugins removed in favour of APM)
5253
getGitHubReposToAllowedReposCodemod(), // Rename deprecated tools.github.repos to tools.github.allowed-repos

pkg/cli/fix_codemods_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func TestGetAllCodemods_ReturnsAllCodemods(t *testing.T) {
4343
codemods := GetAllCodemods()
4444

4545
// Verify we have the expected number of codemods
46-
expectedCount := 29
46+
expectedCount := 30
4747
assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount)
4848

4949
// Verify all codemods have required fields
@@ -130,6 +130,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) {
130130
"playwright-allowed-domains-migration",
131131
"expires-integer-to-string",
132132
"app-to-github-app",
133+
"github-app-app-id-to-client-id",
133134
"safe-inputs-to-mcp-scripts",
134135
"plugins-to-dependencies",
135136
"github-repos-to-allowed-repos",

pkg/parser/import_field_extractor.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ func computeImportRelPath(fullPath, importPath string) string {
511511
}
512512

513513
// validateGitHubAppJSON validates that a JSON-encoded GitHub App configuration has the required
514-
// fields (app-id and private-key). Returns the input JSON if valid, or "" otherwise.
514+
// fields ((client-id or app-id) and private-key). Returns the input JSON if valid, or "" otherwise.
515515
func validateGitHubAppJSON(appJSON string) string {
516516
if appJSON == "" || appJSON == "null" {
517517
return ""
@@ -520,7 +520,9 @@ func validateGitHubAppJSON(appJSON string) string {
520520
if err := json.Unmarshal([]byte(appJSON), &appMap); err != nil {
521521
return ""
522522
}
523-
if _, hasID := appMap["app-id"]; !hasID {
523+
_, hasClientID := appMap["client-id"]
524+
_, hasAppID := appMap["app-id"]
525+
if !hasClientID && !hasAppID {
524526
return ""
525527
}
526528
if _, hasKey := appMap["private-key"]; !hasKey {

pkg/parser/schema_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,26 @@ func TestNormalizeForJSONSchema(t *testing.T) {
498498
}
499499
}
500500

501+
func TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_GitHubAppClientID(t *testing.T) {
502+
frontmatter := map[string]any{
503+
"name": "Client ID validation",
504+
"on": map[string]any{
505+
"issues": map[string]any{
506+
"types": []any{"opened"},
507+
},
508+
},
509+
"github-app": map[string]any{
510+
"client-id": "${{ vars.APP_ID }}",
511+
"private-key": "${{ secrets.APP_PRIVATE_KEY }}",
512+
},
513+
}
514+
515+
err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter, "/tmp/gh-aw/client-id-schema-test.md")
516+
if err != nil {
517+
t.Fatalf("expected client-id in github-app to pass schema validation, got: %v", err)
518+
}
519+
}
520+
501521
// TestNormalizeForJSONSchema_NestedMap verifies recursive normalization of maps.
502522
func TestNormalizeForJSONSchema_NestedMap(t *testing.T) {
503523
input := map[string]any{

0 commit comments

Comments
 (0)