Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions docs/adr/26551-migrate-github-app-token-input-to-client-id.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# ADR-26551: Migrate GitHub App Token Input from `app-id` to `client-id` with Backward Compatibility

**Date**: 2026-04-16
**Status**: Draft
**Deciders**: pelikhan, Copilot

---

## Part 1 — Narrative (Human-Friendly)

### Context

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.

### Decision

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.

### Alternatives Considered

#### Alternative 1: Hard breaking change — remove `app-id` support immediately

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.

#### Alternative 2: Emit both `app-id` and `client-id` in generated output

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.

#### Alternative 3: Accept `app-id` in frontmatter only; always emit `client-id` in compiled output (chosen approach, with codemod)

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.

### Consequences

#### Positive
- Deprecation warnings from `actions/create-github-app-token` are eliminated immediately once workflows are compiled or recompiled.
- Existing workflows using `app-id` continue to compile and run without modification.
- Authors can migrate automatically using `gh aw fix`, reducing manual effort.
- The JSON Schema accurately reflects the valid combinations, enabling editor validation for both old and new syntax.

#### Negative
- 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.
- The `anyOf` schema constraint is harder to read and may produce less clear validation error messages than a single `required` array.
- Maintaining two accepted field names indefinitely (if `app-id` is never removed) adds ongoing surface area to the parser.

#### Neutral
- The `app-id` field in the JSON Schema is now documented as a deprecated alias, which may prompt questions from authors reading schema documentation.
- 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.
- The codemod is registered in the global codemod registry and will run as part of `gh aw fix` alongside all other codemods.

---

## Part 2 — Normative Specification (RFC 2119)

> 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).

### Compiled Output — Token Minting Steps

1. All token-minting steps generated for `actions/create-github-app-token` **MUST** emit `with.client-id`, not `with.app-id`.
2. Generated workflows **MUST NOT** emit both `with.client-id` and `with.app-id` in the same step.

### Frontmatter Parsing — Field Acceptance

1. The frontmatter parser **MUST** accept `client-id` as the primary GitHub App identifier field within any `github-app` object.
2. The frontmatter parser **MUST** accept `app-id` as a backward-compatible alias for `client-id` within any `github-app` object.
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`.
4. A `github-app` object **MUST** include `private-key` alongside either `client-id` or `app-id`; objects missing `private-key` **MUST** be rejected.

### Schema Validation

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"]`.
2. The `app-id` schema property **MUST** be documented as a deprecated alias for `client-id`.
3. Schema examples **SHOULD** use `client-id` rather than `app-id`.

### Codemod

1. The `github-app-app-id-to-client-id` codemod **MUST** rename `app-id` keys to `client-id` only within `github-app` YAML blocks.
2. The codemod **MUST NOT** rename `app-id` keys that appear outside of `github-app` blocks, regardless of nesting depth.
3. The codemod **MUST** be a no-op when no `github-app.app-id` fields are present in the frontmatter.
4. The codemod **MUST** be registered in the global codemod registry returned by `GetAllCodemods()`.

### Conformance

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.

---

*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.*
101 changes: 101 additions & 0 deletions pkg/cli/codemod_github_app_client_id.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package cli

import (
"slices"
"strings"

"github.com/github/gh-aw/pkg/logger"
)

var githubAppClientIDCodemodLog = logger.New("cli:codemod_github_app_client_id")

// getGitHubAppClientIDCodemod creates a codemod that migrates github-app.app-id to github-app.client-id.
func getGitHubAppClientIDCodemod() Codemod {
return Codemod{
ID: "github-app-app-id-to-client-id",
Name: "Rename 'github-app.app-id' to 'github-app.client-id'",
Description: "Renames deprecated 'app-id:' to 'client-id:' inside github-app blocks.",
IntroducedIn: "0.68.4",
Apply: func(content string, frontmatter map[string]any) (string, bool, error) {
if !hasDeprecatedGitHubAppIDField(frontmatter) {
return content, false, nil
}
newContent, applied, err := applyFrontmatterLineTransform(content, renameGitHubAppIDToClientID)
if applied {
githubAppClientIDCodemodLog.Print("Renamed github-app app-id to client-id")
}
return newContent, applied, err
},
}
}

// hasDeprecatedGitHubAppIDField returns true when any github-app object contains app-id.
func hasDeprecatedGitHubAppIDField(frontmatter map[string]any) bool {
return containsDeprecatedGitHubAppID(frontmatter)
}

func containsDeprecatedGitHubAppID(value any) bool {
switch v := value.(type) {
case map[string]any:
for key, child := range v {
if key == "github-app" {
if appMap, ok := child.(map[string]any); ok {
if _, hasAppID := appMap["app-id"]; hasAppID {
return true
}
}
}
if containsDeprecatedGitHubAppID(child) {
return true
}
}
case []any:
return slices.ContainsFunc(v, containsDeprecatedGitHubAppID)
}
return false
}

func renameGitHubAppIDToClientID(lines []string) ([]string, bool) {
result := make([]string, 0, len(lines))
modified := false

inGitHubApp := false
var githubAppIndent string

for i, line := range lines {
trimmed := strings.TrimSpace(line)

if len(trimmed) == 0 {
result = append(result, line)
continue
}

if !strings.HasPrefix(trimmed, "#") && inGitHubApp && hasExitedBlock(line, githubAppIndent) {
inGitHubApp = false
}

if strings.HasPrefix(trimmed, "github-app:") {
inGitHubApp = true
githubAppIndent = getIndentation(line)
result = append(result, line)
continue
}

if inGitHubApp {
lineIndent := getIndentation(line)
if isDescendant(lineIndent, githubAppIndent) && strings.HasPrefix(trimmed, "app-id:") {
newLine, replaced := findAndReplaceInLine(line, "app-id", "client-id")
if replaced {
result = append(result, newLine)
modified = true
githubAppClientIDCodemodLog.Printf("Renamed github-app app-id on line %d", i+1)
continue
}
}
}

result = append(result, line)
}

return result, modified
}
113 changes: 113 additions & 0 deletions pkg/cli/codemod_github_app_client_id_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
//go:build !integration

package cli

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestGitHubAppClientIDCodemod(t *testing.T) {
codemod := getGitHubAppClientIDCodemod()

t.Run("renames app-id under github-app blocks", func(t *testing.T) {
content := `---
github-app:
app-id: ${{ vars.TOP_LEVEL_APP_ID }}
private-key: ${{ secrets.TOP_LEVEL_APP_PRIVATE_KEY }}
on:
github-app:
app-id: ${{ vars.ACTIVATION_APP_ID }}
private-key: ${{ secrets.ACTIVATION_APP_PRIVATE_KEY }}
checkout:
- repository: org/repo
github-app:
app-id: ${{ vars.CHECKOUT_APP_ID }}
private-key: ${{ secrets.CHECKOUT_APP_PRIVATE_KEY }}
---
`
frontmatter := map[string]any{
"github-app": map[string]any{
"app-id": "${{ vars.TOP_LEVEL_APP_ID }}",
"private-key": "${{ secrets.TOP_LEVEL_APP_PRIVATE_KEY }}",
},
"on": map[string]any{
"github-app": map[string]any{
"app-id": "${{ vars.ACTIVATION_APP_ID }}",
"private-key": "${{ secrets.ACTIVATION_APP_PRIVATE_KEY }}",
},
},
"checkout": []any{
map[string]any{
"repository": "org/repo",
"github-app": map[string]any{
"app-id": "${{ vars.CHECKOUT_APP_ID }}",
"private-key": "${{ secrets.CHECKOUT_APP_PRIVATE_KEY }}",
},
},
},
}

result, modified, err := codemod.Apply(content, frontmatter)
require.NoError(t, err, "Should not error when applying codemod")
assert.True(t, modified, "Should modify content")
assert.NotContains(t, result, "app-id:", "Should remove deprecated app-id key from github-app blocks")
assert.Contains(t, result, "client-id: ${{ vars.TOP_LEVEL_APP_ID }}", "Should migrate top-level github-app")
assert.Contains(t, result, "client-id: ${{ vars.ACTIVATION_APP_ID }}", "Should migrate on.github-app")
assert.Contains(t, result, "client-id: ${{ vars.CHECKOUT_APP_ID }}", "Should migrate checkout github-app")
})

t.Run("does not modify content without github-app.app-id", func(t *testing.T) {
content := `---
github-app:
client-id: ${{ vars.APP_ID }}
private-key: ${{ secrets.APP_PRIVATE_KEY }}
---
`
frontmatter := map[string]any{
"github-app": map[string]any{
"client-id": "${{ vars.APP_ID }}",
"private-key": "${{ secrets.APP_PRIVATE_KEY }}",
},
}

result, modified, err := codemod.Apply(content, frontmatter)
require.NoError(t, err, "Should not error")
assert.False(t, modified, "Should not modify already migrated content")
assert.Equal(t, content, result, "Content should remain unchanged")
})

t.Run("does not rename app-id outside github-app blocks", func(t *testing.T) {
content := `---
engine:
provider:
auth:
client-id: APP_CLIENT_ID
tools:
custom:
app-id: value
---
`
frontmatter := map[string]any{
"engine": map[string]any{
"provider": map[string]any{
"auth": map[string]any{
"client-id": "APP_CLIENT_ID",
},
},
},
"tools": map[string]any{
"custom": map[string]any{
"app-id": "value",
},
},
}

result, modified, err := codemod.Apply(content, frontmatter)
require.NoError(t, err, "Should not error")
assert.False(t, modified, "Should not modify non github-app app-id keys")
assert.Equal(t, content, result, "Content should remain unchanged")
})
}
1 change: 1 addition & 0 deletions pkg/cli/fix_codemods.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ func GetAllCodemods() []Codemod {
getPlaywrightDomainsToNetworkAllowedCodemod(), // Migrate tools.playwright.allowed_domains to network.allowed
getExpiresIntegerToDayStringCodemod(), // Convert expires integer (days) to string with 'd' suffix
getGitHubAppCodemod(), // Rename deprecated 'app' to 'github-app'
getGitHubAppClientIDCodemod(), // Rename deprecated github-app.app-id to github-app.client-id
getSafeInputsToMCPScriptsCodemod(), // Rename safe-inputs to mcp-scripts
getPluginsToDependenciesCodemod(), // Migrate plugins to dependencies (plugins removed in favour of APM)
getGitHubReposToAllowedReposCodemod(), // Rename deprecated tools.github.repos to tools.github.allowed-repos
Expand Down
3 changes: 2 additions & 1 deletion pkg/cli/fix_codemods_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestGetAllCodemods_ReturnsAllCodemods(t *testing.T) {
codemods := GetAllCodemods()

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

// Verify all codemods have required fields
Expand Down Expand Up @@ -130,6 +130,7 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) {
"playwright-allowed-domains-migration",
"expires-integer-to-string",
"app-to-github-app",
"github-app-app-id-to-client-id",
"safe-inputs-to-mcp-scripts",
"plugins-to-dependencies",
"github-repos-to-allowed-repos",
Expand Down
6 changes: 4 additions & 2 deletions pkg/parser/import_field_extractor.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ func computeImportRelPath(fullPath, importPath string) string {
}

// validateGitHubAppJSON validates that a JSON-encoded GitHub App configuration has the required
// fields (app-id and private-key). Returns the input JSON if valid, or "" otherwise.
// fields ((client-id or app-id) and private-key). Returns the input JSON if valid, or "" otherwise.
func validateGitHubAppJSON(appJSON string) string {
if appJSON == "" || appJSON == "null" {
return ""
Expand All @@ -520,7 +520,9 @@ func validateGitHubAppJSON(appJSON string) string {
if err := json.Unmarshal([]byte(appJSON), &appMap); err != nil {
return ""
}
if _, hasID := appMap["app-id"]; !hasID {
_, hasClientID := appMap["client-id"]
_, hasAppID := appMap["app-id"]
if !hasClientID && !hasAppID {
return ""
}
if _, hasKey := appMap["private-key"]; !hasKey {
Expand Down
20 changes: 20 additions & 0 deletions pkg/parser/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,26 @@ func TestNormalizeForJSONSchema(t *testing.T) {
}
}

func TestValidateMainWorkflowFrontmatterWithSchemaAndLocation_GitHubAppClientID(t *testing.T) {
frontmatter := map[string]any{
"name": "Client ID validation",
"on": map[string]any{
"issues": map[string]any{
"types": []any{"opened"},
},
},
"github-app": map[string]any{
"client-id": "${{ vars.APP_ID }}",
"private-key": "${{ secrets.APP_PRIVATE_KEY }}",
},
}

err := ValidateMainWorkflowFrontmatterWithSchemaAndLocation(frontmatter, "/tmp/gh-aw/client-id-schema-test.md")
if err != nil {
t.Fatalf("expected client-id in github-app to pass schema validation, got: %v", err)
}
}

// TestNormalizeForJSONSchema_NestedMap verifies recursive normalization of maps.
func TestNormalizeForJSONSchema_NestedMap(t *testing.T) {
input := map[string]any{
Expand Down
Loading