Skip to content

docs: update GitHub App docs to use client-id#181

Merged
joshjohanning merged 2 commits into
joshjohanning:mainfrom
Wuodan:upstream-PR/Update-GitHub-App-docs-to-use-client-id
Apr 22, 2026
Merged

docs: update GitHub App docs to use client-id#181
joshjohanning merged 2 commits into
joshjohanning:mainfrom
Wuodan:upstream-PR/Update-GitHub-App-docs-to-use-client-id

Conversation

@Wuodan

@Wuodan Wuodan commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Update the README examples for actions/create-github-app-token@v3 to use client-id.

Prevents this warning:
Warning: Input 'app-id' has been deprecated with message: Use 'client-id' instead.

See actions/create-github-app-token#353

Update the README examples for actions/create-github-app-token@v3 to use client-id.

Prevents this warning:
Warning: Input 'app-id' has been deprecated with message: Use 'client-id' instead.

See actions/create-github-app-token#353
@Wuodan

Wuodan commented Apr 18, 2026

Copy link
Copy Markdown
Contributor Author

I just noticed that bulk-github-repo-settings-sync-action-live-tests uses variables not secret for APP_ID and also prepends vars/secrets with LIVE_TEST_ (probably to avoid confusion when those tests where in this action repo).

PR coming in the live tests repo ...

@joshjohanning

joshjohanning commented Apr 18, 2026

Copy link
Copy Markdown
Owner

Ahh, good point @Wuodan! I see there's a bit of inconsistency here in our docs.

In that case, let's just go with what the create-github-app-token uses to be consistent:

  client-id: ${{ vars.GITHUB_APP_CLIENT_ID }} # INVALID
  private-key: ${{ secrets.GITHUB_APP_PRIVATE_KEY }} # INVALID

@joshjohanning

Copy link
Copy Markdown
Owner

The action's docs are bad, vars and secrets can't start with GITHUB_

Variable name cannot start with "GITHUB_" prefix.

I haven't decided if I like CLIENT_ID or APP_CLIENT_ID. I think CLIENT_ID like you have it since it's just PRIVATE_KEY otherwise.

@Wuodan

Wuodan commented Apr 18, 2026

Copy link
Copy Markdown
Contributor Author

https://github.com/actions/create-github-app-token/blob/1b10c78c7865c340bc4f6099eb2f838309f1e8c3/.github/workflows/test.yml#L48

In the tests of actions/create-github-app-token, other var names or secret names are used:

        with:
          app-id: ${{ vars.TEST_APP_ID }}
          private-key: ${{ secrets.TEST_APP_PRIVATE_KEY }}

So those names in the docs seem to be ment as samples. But I agree that those docs might be confusing or lead to trial and error for many users.

@Wuodan

Wuodan commented Apr 18, 2026

Copy link
Copy Markdown
Contributor Author

joshjohanning/bulk-github-repo-settings-sync-action-live-tests#19

Updates the live-tests to what the action documents with this PR here:

          client-id: ${{ secrets.CLIENT_ID }}
          private-key: ${{ secrets.APP_PRIVATE_KEY }}

secrets.PRIVATE_KEY would also work, I ran a test.

And I would just leave it at that (or if anything rename APP_PRIVATE_KEY to PRIVATE_KEY).

Most users of this action will just copy the sample and have a syn-repo running the action with some config files (like dependabot.yml, etc.) and some glue like you and I have. And this only needs one GH app and thus no extra prefix to var and secret names.
Users having a complex setup with several GH apps are competent enough to prefix themselves what's needed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the README’s GitHub App authentication examples to use the non-deprecated client-id input for actions/create-github-app-token@v3, avoiding the deprecation warning for app-id.

Changes:

  • Replace app-id with client-id in README workflow examples.
  • Update the setup instructions to store the GitHub App Client ID as a repository variable (APP_CLIENT_ID) instead of a secret.
Show a summary per file
File Description
README.md Updates GitHub App token generation examples/instructions to use client-id + ${{ vars.APP_CLIENT_ID }} to match the upstream deprecation.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@joshjohanning joshjohanning changed the title Update GitHub App docs to use client-id docs: update GitHub App docs to use client-id Apr 22, 2026
@joshjohanning

Copy link
Copy Markdown
Owner

Thanks @Wuodan! I changed this one and joshjohanning/bulk-github-repo-settings-sync-action-live-tests#19 to use vars.APP_CLIENT_ID and secrets.APP_PRIVATE_KEY to match the names of the vars/secrets in the upstream action now- https://github.com/actions/create-github-app-token (because I like consistency 😁 )

@joshjohanning

Copy link
Copy Markdown
Owner

And in my opinion, the client ID is not a secret (it's a var), so having it exposed as a var instead of a secret is EASIER to know which exact app you're referencing and easier to thus change it if you have to.

@joshjohanning joshjohanning merged commit c7dddd4 into joshjohanning:main Apr 22, 2026
5 checks passed
joshjohanning added a commit that referenced this pull request Apr 22, 2026
- Restore private-vulnerability-reporting in README YAML examples (#185)
- Restore client-id/APP_CLIENT_ID in README auth docs (#181)
- Add warning subResult when listing deployment protection rule apps fails
- Add warning subResult when listing deployment branch policies fails
joshjohanning added a commit that referenced this pull request Apr 23, 2026
* Initial plan

* feat: add environment syncing support

Add new `environments-file` and `delete-unmanaged-environments` inputs
to sync deployment environments (production, staging, etc.) across
multiple repositories via the GitHub API.

- Create, update, and optionally delete environments
- Support wait timers, reviewers, branch policies, self-review prevention
- Dry-run mode shows planned changes without applying them
- Per-repo overrides via repos.yml or rules-based config
- 24 new tests covering all sync scenarios

Agent-Logs-Url: https://github.com/joshjohanning/bulk-github-repo-settings-sync-action/sessions/f2a87b79-f7d1-422d-99fb-83ac60d4a28a

Co-authored-by: joshjohanning <19912012+joshjohanning@users.noreply.github.com>

* refactor: address code review - extract buildEnvironmentParams helper and use explicit comparisons

Agent-Logs-Url: https://github.com/joshjohanning/bulk-github-repo-settings-sync-action/sessions/f2a87b79-f7d1-422d-99fb-83ac60d4a28a

Co-authored-by: joshjohanning <19912012+joshjohanning@users.noreply.github.com>

* fix: address rubber duck and CCR feedback for environment sync

- Extract prevent_self_review from required_reviewers protection rule
- Use normalized desired env for PUT calls (not raw config)
- Always send all fields in buildEnvironmentParams
- Use coerceBooleanConfig for delete-unmanaged-environments override
- Add pagination for environment listing
- Align global getBooleanInput usage

* refactor: support inline environments and YAML config

- Add 'environments' input for simple comma-separated names
- Change environments-file from JSON-only to YAML/JSON
- Extract parseEnvironmentsConfig for upstream parsing
- syncEnvironments now accepts an array instead of file path
- Per-repo overrides support both inline and file
- Replace JSON sample with YAML sample
- Add parseEnvironmentsConfig tests

* feat: add reviewer name resolution, deployment protection rules, and YAML examples

- Resolve reviewer login/slug to numeric IDs via API
- Add syncDeploymentProtectionRules for custom deployment gates
- Support deployment_protection_rules in environments config
- Update sample config with friendly names and protection rules

* chore: bumping version and badge

* fix: address rubber duck feedback

- Empty deployment_protection_rules array now deletes existing gates
- Aggregate protection rule subResults into environment result
- Correct false-positive detection when only rules match
- Always validate environments-file even when inline names set
- Add reviewer identity to resolution error messages

* fix: address CCR feedback

- Add environments input to mock action.yml in tests
- Add environments input to README inputs table
- Remove stray JSDoc block
- Add environments-file to FILE_PATH_CONFIG_KEYS for base-path resolution
- Fix startup log to show for both inline and file inputs
- Fix parseEnvironmentsConfig JSDoc

* fix: address CCR feedback round 3

- Deduplicate inline environment names
- Add environments to mockActionYmlParsed
- Update action.yml description for environments input
- Fix all .json references to .yml in README and sample config
- Update README with login/slug reviewer docs and deployment_protection_rules schema
- Replace JSON example with YAML in README
- Remove stray JSDoc opener
- Add environments-file to FILE_PATH_CONFIG_KEYS for base-path

* fix: address CCR feedback on environment syncing

- Log warning when listing deployment protection rules fails instead
  of silently swallowing the error
- Include deployment protection rules in dry-run preview message

* fix: address rubber duck findings on environment syncing

- Add custom deployment branch policy syncing (list/create/delete
  branch name patterns via deployment-branch-policies API)
- Fix repo-specific env parse failure: skip env sync instead of
  falling back to global config
- Validate duplicate environment names within config files
- Propagate protection rule and branch policy subResults to repo-level
  results for accurate summaries and warning counts
- Update sample config with branch_name_patterns example

* fix: restore merged PR changes lost during conflict resolution

- Restore private-vulnerability-reporting in README YAML examples (#185)
- Restore client-id/APP_CLIENT_ID in README auth docs (#181)
- Add warning subResult when listing deployment protection rule apps fails
- Add warning subResult when listing deployment branch policies fails

* fix: run branch policy and protection rule sync during dry-run

- Remove early return in dry-run so syncDeploymentBranchPolicies and
  syncDeploymentProtectionRules run (they handle dryRun internally),
  surfacing warnings and detailed previews
- Treat omitted branch_name_patterns as empty array when
  custom_branch_policies is true (syncs deletion of existing patterns)
- Use dry-run-aware message phrasing in final result

* fix: defensive input handling for reviewers, protection rules, branch patterns

- Coerce reviewer.id to number with validation (prevents string/number mismatch)
- Deduplicate deployment protection rule app slugs
- Normalize branch name patterns (trim, filter empty, deduplicate)

* fix: error handling for branch policy API calls, fix README wording

- Wrap POST/DELETE branch policy calls in try/catch with WARNING
  subResults (consistent with deployment protection rules pattern)
- Fix README: delete-unmanaged-environments applies to combined
  desired list, not just config file

* fix: validate environment config inputs

- Trim and validate environment names from file (reject whitespace-only)
- Validate reviewers is an array before iterating
- Validate deployment_protection_rules is an array before iterating

* fix: address CCR feedback - tests, docs, and input ordering

- Add 6 tests for deployment protection rules and branch policy syncing
  (create, delete, dry-run preview, app not found warning, omitted patterns)
- Fix duplicate detection to run after name trimming
- Document branch policy fields in README environments config table
- Follow-up issue #191 created for reviewer resolution caching

* fix: address CCR feedback - input validation edge cases

- Fix reviewer.id falsy check (0 was treated as unset)
- Validate protection rule items are objects with non-empty app string
- Validate branch pattern items are strings (skip with warning)
- Warn instead of silently coerce non-array branch_name_patterns

* fix: address CCR feedback - repo env coercion, docs wording

- Fix String() coercion of repo-specific environments (null/array now handled)
- Support array format for repo-specific environments in YAML
- Fix input table wording for delete-unmanaged-environments
- Show explicit per-repo opt-out in README example

* fix: address CCR feedback - docs improvements

- Update environments-file input description to include deployment
  protection rules and branch name patterns
- Add inline environments usage example to README
- Document that both inputs can be combined

* fix: preserve both YAML and JSON parse errors in diagnostics

Show both YAML and JSON parse errors when environments config file
fails to parse, so users see the actual format problem.

* fix: address CCR feedback - branch policy warnings, README callout

- Include branchPolicySubResults in unchanged return path
- Track hasBranchPolicyWarnings for message and hasWarnings propagation
- Include branch policy warnings in repo-level hasWarnings check
- Add warning callout for inline environments resetting existing config

* fix: simple mode environments only creates, does not update existing

Inline environments (comma-separated names) now only create environments
that don't exist. Existing environments are left unchanged to avoid
resetting their configured reviewers, wait timers, and branch policies.
Use environments-file for full config management of existing environments.

* fix: add WARNING subResults for skipped branch policy and protection rule syncs

- Add WARNING subResult when branch_name_patterns is invalid type
- Add WARNING subResult and early return when listing existing
  deployment protection rules fails

* fix: update environments input description to match create-only behavior

* fix: address CCR feedback - empty env delete, dry-run 404s, docs

- Allow empty environments list with delete-unmanaged to delete all
- Skip branch policy/protection rule sync for uncreated envs in dry-run
- Add NOTE about environments-file defaults resetting existing settings
- Clarify inline vs file mode behavior in docs

* fix: address CCR feedback - file entry validation, empty rules optimization, docs

- Validate file entries are objects before accessing .name
- Skip apps API lookup when desiredRules is empty (delete-all case)
- Clarify environments: '' skip behavior in README

* fix: safeguard against accidental deletion from invalid config

- Skip protection rule deletion when all desired rules failed validation
- Skip branch policy deletion when all desired patterns were invalid
- Include deleteUnmanagedEnvironments in hasSettings gate

* feat: add reviewer resolution caching across repos

Cache resolved reviewer IDs (User login → ID, Team slug → ID) in a
Map that persists across the repo loop, avoiding duplicate API calls
when the same reviewers appear in multiple repos' environment configs.

Closes #191

* fix: address CCR feedback - hasSettings guard, pagination, field handling

- Remove deleteUnmanagedEnvironments from hasSettings gate (requires
  environments config to be meaningful)
- Increase environment listing page size from 30 to 100

* fix: scope team cache key by org, guard repo-specific delete-unmanaged

- Include owner in team reviewer cache key since team slugs are
  org-scoped (prevents cross-org cache collision)
- Require explicit environments config for delete-unmanaged to
  prevent accidental mass deletion from inherited global empty list

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: joshjohanning <19912012+joshjohanning@users.noreply.github.com>
Co-authored-by: Josh Johanning <joshjohanning@github.com>
@Wuodan Wuodan deleted the upstream-PR/Update-GitHub-App-docs-to-use-client-id branch April 27, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants