Skip to content

ci: auto-format pull requests - #901

Draft
AntonioVentilii wants to merge 5 commits into
mainfrom
ci/auto-format-pull-requests
Draft

ci: auto-format pull requests#901
AntonioVentilii wants to merge 5 commits into
mainfrom
ci/auto-format-pull-requests

Conversation

@AntonioVentilii

@AntonioVentilii AntonioVentilii commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Stacked on #898. Base is style/prettier-3-9-5-formatting so the diff stays clean; GitHub retargets this to main automatically once #898 merges.

Motivation

Today the Format job runs prettier --check and fails, leaving the contributor to run the formatter by hand and push a follow-up commit. dfinity/oisy-wallet instead runs the formatter in CI and commits the result back to the PR branch — see its frontend-checks.yml (frontend/prettier) and formatting-checks.yml (Rust/shell). This ports that pattern.

Changes

  • .github/actions/add-and-commit — composite action ported from dfinity/oisy-wallet. Re-supplies the token to git locally (checkout runs with persist-credentials: false), commits, then unsets the config. The EndBug/add-and-commit pin is v10.0.0 — the one this repo already uses in e2e-snapshots.yml — rather than oisy-wallet's older v9.1.4.
  • lib-checks.yml format job — mint a GitHub App token (continue-on-error), check out the PR head when one is available, run npm run format, and commit 🤖 Apply formatting changes if anything changed. Job permission goes contents: read -> contents: write.

Behaviour when the token is unavailable

can_add_commit is false for pushes, fork PRs, and any run where the app token did not mint. In that case the job checks out normally, formats, prints the diff and exits 1 — i.e. exactly today's behaviour, just with a diff attached. Nothing gets silently skipped.

This composes with #900: on a push to main, github.event_name != 'pull_request', so it takes the fail-with-diff path rather than trying to commit to main.

Blocker found on the first run — the private key is not available to this repo

The Format run on this PR shows the app variable resolves but the secret does not:

with:
  app-id: 1063231
Error: The 'private-key' input must be set to a non-empty string.
      If using a secret or variable, ensure it is available in this workflow context.

So vars.PR_AUTOMATION_BOT_PUBLIC_APP_ID reaches oisy-wallet-signer, but secrets.PR_AUTOMATION_BOT_PUBLIC_PRIVATE_KEY does not — most likely the org secret's repository-access list does not include this repo.

The fallback behaved exactly as designed: continue-on-error swallowed the failure, can_add_commit became false, the job checked out normally, formatted, found no changes and passed. So this PR is safe to merge as-is — it is a no-op relative to today's behaviour until the secret is granted, at which point auto-commit starts working with no further code change.

To actually enable it: grant PR_AUTOMATION_BOT_PUBLIC_PRIVATE_KEY repository access to dfinity/oisy-wallet-signer in the org secret settings, and confirm the app is installed here with contents: write.

Minor: actions/create-github-app-token now warns that app-id is deprecated in favour of client-id. Kept as-is to match the org variable (a numeric app id) and dfinity/oisy-wallet; switching would need a separate client-id variable.

Known limitation — this would not have auto-fixed #890

The prettier bump that started this was authored by app/dependabot. Dependabot-triggered pull_request runs get a read-only token and read from the separate Dependabot secrets store, not Actions secrets, so the app token will not mint on them and they take the fail-with-diff path.

Auto-formatting dependabot PRs too would additionally require the app credentials to be added under Dependabot secrets. Worth doing as a follow-up if the team wants it, since dependency bumps are exactly the PRs that trigger reformats.

Verification

  • zizmor --persona pedantic passes locally on both changed files (the three audits needing a GitHub API token were skipped; the Lint GitHub Actions job here will cover those).
  • prettier --check passes on both files.

Prettier was bumped 3.8.3 -> 3.9.5 in #890, which changed how union
types that fit on a single line are printed. The Format job only runs
on `pull_request`, so the reformat landed on main unchecked and
`npm run format:check` now fails on main and every open PR.
The Format job ran `prettier --check` and failed, leaving contributors to
run the formatter by hand and push a follow-up commit.

Run `npm run format` instead and, when the job can authenticate as the PR
automation app, commit the result back to the PR branch. When it cannot
(pushes, forks, dependabot), fall back to printing the diff and failing,
which is the previous behaviour.

Ports the pattern already used by dfinity/oisy-wallet in
frontend-checks.yml, along with its add-and-commit composite action. The
EndBug/add-and-commit pin matches the one already used by this repo in
e2e-snapshots.yml.
@AntonioVentilii
AntonioVentilii requested a review from a team as a code owner August 19, 2026 08:45
@zeropath-ai

zeropath-ai Bot commented Aug 19, 2026

Copy link
Copy Markdown

No security or compliance issues detected. Reviewed everything up to a58b925.

Security Overview
Detected Code Changes
Change Type Relevant files
Enhancement ► .github/actions/add-and-commit/action.yml
    Add new GitHub Action: Add and Commit functionality to set remote, commit, and push changes
► .github/workflows/lib-checks.yml
    Modify lib checks workflow to support formatting changes: grant write permission, add steps for app token, commit, and diff output

Base automatically changed from style/prettier-3-9-5-formatting to main August 19, 2026 09:24
@AntonioVentilii
AntonioVentilii dismissed DenysKarmazynDFINITY’s stale review August 19, 2026 09:24

The base branch was changed.

@AntonioVentilii
AntonioVentilii enabled auto-merge (squash) August 19, 2026 09:27
@AntonioVentilii
AntonioVentilii marked this pull request as draft August 19, 2026 12:22
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.

2 participants