Skip to content

workflows: fix allowlist update race (#866)#876

Open
potiuk wants to merge 1 commit into
mainfrom
fix-866-allowlist-race
Open

workflows: fix allowlist update race (#866)#876
potiuk wants to merge 1 commit into
mainfrom
fix-866-allowlist-race

Conversation

@potiuk
Copy link
Copy Markdown
Member

@potiuk potiuk commented May 22, 2026

Summary

Consolidates update_actions.yml (composite → actions.yml) and
update_composite_action.yml (actions.yml → composite) into a single
update_allowlist.yml. The previous pair ran in separate concurrency
groups and could overlap, silently clobbering one direction's edit —
see #866 for raboof's reconstruction of how a dependabot bump got
downgraded.

The combined workflow:

  • Triggers on push to main when either file changes
  • Single concurrency group so only one run touches the allowlist at a time
  • Always runs both directions: merge composite → actions.yml (additive),
    then regenerate composite + approved_patterns.yml from actions.yml
  • Checks out latest main HEAD (not the trigger SHA) so a queued run
    sees the prior run's commit
  • Rebases and retries on push failure (the sync script is idempotent)

Also updates the README's three mermaid diagrams + prose to point at the
new workflow name. The two utils/check_cache_settings/*.py changes are
prek auto-inserting missing Apache license headers on pre-existing
files — unrelated to #866 but caught by prek run --all-files; CI would
have auto-fixed them anyway.

Fixes #866

Test plan

  • uv run pytest gateway/ — 8 passed
  • prek run --all-files clean
  • After merge: trigger via workflow_dispatch on an unchanged tree
    to confirm no-op behavior
  • Watch the next dependabot bump merge to confirm actions.yml +
    composite + approved_patterns.yml all update from a single workflow

The two workflows ran in separate concurrency groups and could overlap
on a push that changed both actions.yml and the dependabot composite
action.yml, silently clobbering one direction's edit. The new
update_allowlist.yml runs both directions in order with a single
concurrency group, so neither edit is lost.

Fixes #866

Generated-by: Claude Opus 4.7 (1M context)
Comment on lines +58 to +61
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
with:
persist-credentials: true
token: ${{ secrets.ALLOWLIST_WORKFLOW_TOKEN || github.token }} # zizmor: ignore[secrets-outside-env]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the purpose of the PAT here? Recursive behavior?

This workflow modifies the same file (actions.yml) that was used to trigger it. Are we sure the modifications are eventually idempotent?

We should probably add a comment here.

- name: Commit and push changes
if: ${{ github.event_name != 'pull_request' }}
env:
GH_TOKEN: ${{ secrets.ALLOWLIST_WORKFLOW_TOKEN || github.token }} # zizmor: ignore[secrets-outside-env]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is the purpose of this PAT?

git has already been configured with the recursive token above.

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.

race condition in allowlist / action updates

2 participants