Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d9cf780f-418b-4a04-a1b9-32191a162ea9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
client-id and add schema-level compatibility
|
@copilot add codemod to migrate |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/779022f8-5eaf-43cb-8a90-62ddc3637685 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in commit Added a new codemod that migrates Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
client-id and add schema-level compatibilityclient-id, add schema-level compatibility, and provide codemod migration
There was a problem hiding this comment.
Pull request overview
This PR migrates GitHub App token minting from app-id to client-id to eliminate upstream deprecation warnings, while keeping backward compatibility via parsing/schema support and providing an automatic codemod.
Changes:
- Updated workflow generation to emit
with.client-id(instead ofwith.app-id) foractions/create-github-app-token. - Added backward-compatible parsing/validation so configs can use
client-id(preferred) or legacyapp-id. - Updated JSON schema and added a new
gh aw fixcodemod to migrategithub-app.app-id→github-app.client-id, with tests.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/workflow_github_app.go | Emits client-id when injecting top-level github-app fallback into tools map. |
| pkg/workflow/top_level_github_app_integration_test.go | Updates integration assertions to expect client-id in compiled output. |
| pkg/workflow/skip_if_no_match_test.go | Updates minted-step assertions to expect client-id. |
| pkg/workflow/skip_if_match_test.go | Updates minted-step assertions to expect client-id. |
| pkg/workflow/safe_outputs_validation.go | Updates user-facing guidance text to show client-id. |
| pkg/workflow/safe_outputs_app_config.go | Accepts client-id or legacy app-id on input; emits client-id in generated steps. |
| pkg/workflow/github_mcp_app_token_test.go | Updates GitHub MCP token minting assertions to expect client-id. |
| pkg/workflow/github_app_permissions_validation.go | Updates error/help text examples to use client-id. |
| pkg/workflow/frontmatter_extraction_yaml.go | Recognizes client-id: as a nested github-app field in YAML scanning logic. |
| pkg/workflow/compiler_pre_activation_job.go | Emits client-id in pre-activation token minting step. |
| pkg/workflow/compiler_github_mcp_steps.go | Updates logging text to reference client-id. |
| pkg/workflow/checkout_manager_test.go | Adds parsing coverage for client-id and updates error-message expectations. |
| pkg/workflow/checkout_config_parser.go | Updates validation error message to mention client-id (or app-id). |
| pkg/workflow/activation_github_token_test.go | Updates activation token minting assertions to expect client-id. |
| pkg/parser/schemas/main_workflow_schema.json | Adds client-id, keeps app-id as legacy, and makes required-ness anyOf for compatibility. |
| pkg/parser/schema_test.go | Adds schema validation test coverage for github-app.client-id. |
| pkg/parser/import_field_extractor.go | Treats (client-id OR app-id) + private-key as valid for imported github-app JSON detection. |
| pkg/cli/fix_codemods_test.go | Updates codemod registry tests for the newly added codemod. |
| pkg/cli/fix_codemods.go | Registers the new github-app app-id→client-id codemod. |
| pkg/cli/codemod_github_app_client_id_test.go | Adds focused tests for the new migration codemod behavior. |
| pkg/cli/codemod_github_app_client_id.go | Implements the codemod that renames app-id: to client-id: within github-app blocks. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
pkg/workflow/safe_outputs_app_config.go:45
parseAppConfigprefersclient-idwhen present, but ifclient-idexists with a non-string (or empty) value and a valid legacyapp-idis also provided, the currentif/elseprevents falling back toapp-id, leavingAppIDempty and causing validation to fail. Consider only preferringclient-idwhen it parses to a non-empty string; otherwise, fall back toapp-id(and optionally log/return an error for invalid types).
// Parse client-id/app-id (required)
// Prefer client-id when both are provided; app-id is accepted for backward compatibility.
if clientID, exists := appMap["client-id"]; exists {
if clientIDStr, ok := clientID.(string); ok {
appConfig.AppID = clientIDStr
}
} else if appID, exists := appMap["app-id"]; exists {
if appIDStr, ok := appID.(string); ok {
appConfig.AppID = appIDStr
}
}
pkg/parser/import_field_extractor.go:531
validateGitHubAppJSONonly checks for the presence ofclient-id/app-idandprivate-keykeys, not that they are non-empty strings. Because import extraction is first-wins, an imported workflow withclient-id: null(or numeric) can be treated as “valid” here, block later valid configs, and then fail parsing inresolveTopLevelGitHubApp. Tighten validation to require a non-empty string for whichever ID field is present (preferclient-idif both) and forprivate-key.
// validateGitHubAppJSON validates that a JSON-encoded GitHub App configuration has the required
// 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 ""
}
var appMap map[string]any
if err := json.Unmarshal([]byte(appJSON), &appMap); err != nil {
return ""
}
_, hasClientID := appMap["client-id"]
_, hasAppID := appMap["app-id"]
if !hasClientID && !hasAppID {
return ""
}
if _, hasKey := appMap["private-key"]; !hasKey {
return ""
}
return appJSON
- Files reviewed: 21/21 changed files
- Comments generated: 1
| "app-id": { | ||
| "type": "string", | ||
| "description": "GitHub App ID (e.g., '${{ vars.APP_ID }}'). Required to mint a GitHub App token.", | ||
| "description": "Deprecated alias for client-id. GitHub App ID/client ID (e.g., '${{ vars.APP_ID }}').", |
🧪 Test Quality Sentinel ReportTest Quality Score: 72/100
Test Classification DetailsView All Test Classifications (16 tests)
Flagged Tests — Requires Review
|
Generated by the Design Decision Gate workflow to document the architectural decision to migrate `app-id` → `client-id` in GitHub App token configuration, with backward-compatible parsing and codemod support. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: What the draft coversThe generated ADR documents the three interrelated design decisions in this PR:
What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: §24493163553
|
…ment object form - Add byok-copilot feature flag subsection to frontmatter.md (from #26544: bundles cli-proxy, COPILOT_API_KEY injection, and latest Copilot CLI install behind a single flag) - Update all github-app examples to use client-id (preferred) instead of app-id (deprecated alias) across auth.mdx, checkout.md, faq.md, imports.md, safe-outputs.md, safe-outputs-pull-requests.md, tokens.md, triggers.md, and glossary.md (from #26551: run gh aw fix to auto-migrate) - Add object-form documentation for status-comment in triggers.md, with issues/pull-requests/discussions toggle fields (from #26535) - Update status-comment description in frontmatter.md to mention object form Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Workflows using
github-appcheckout emitted upstream deprecation warnings because compiled jobs passedapp-idtoactions/create-github-app-token, while frontmatter rejectedclient-id. This change updates generation to useclient-idand allows workflow authors to provide either field during migration.Token minting output migration (
app-id→client-id)with.client-idwith.private-keyBackward-compatible frontmatter/import parsing
github-appparsing now accepts both:client-id(preferred)app-id(legacy)client-idis preferred.github-appJSON now treats(client-id OR app-id) + private-keyas valid.Schema updates for author-facing compatibility
github_appschema to support either required pair:client-id+private-key, orapp-id+private-keyapp-idas a deprecated alias in schema/docs/examples where relevant to preserve compatibility while enabling migration.Codemod support for automatic migration
gh aw fixcodemod to migrate:github-app.app-id→github-app.client-idgithub-appblocksgithub-appapp-idkeys.Error/help text and expectation updates
client-id (or app-id)semantics.client-idin compiled output and added coverage for parsingclient-idingithub-app.