Skip to content

ci: chart impit's own npm/PyPI release history on PR comments - #549

Open
barjin wants to merge 2 commits into
masterfrom
claude/benchmark-ci-version-compare-xckigq
Open

ci: chart impit's own npm/PyPI release history on PR comments#549
barjin wants to merge 2 commits into
masterfrom
claude/benchmark-ci-version-compare-xckigq

Conversation

@barjin

@barjin barjin commented Sep 8, 2026

Copy link
Copy Markdown
Member

Adds a workflow that benchmarks the last 5 npm and PyPI releases of impit against each other and comments a throughput chart on the PR, without committing the chart to the repo.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UnwET4Qxx14A9FG28Xniqx


Generated by Claude Code

@github-actions github-actions Bot added this to the 149th sprint - Tooling team milestone Sep 8, 2026
@github-actions github-actions Bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Sep 8, 2026

barjin commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

The comment job is gated by on: pull_request (not pull_request_target) but uses secrets.APIFY_SERVICE_ACCOUNT_GITHUB_TOKEN; since pull_request runs the workflow definition from the PR's own head ref, and this workflow's paths filter includes itself (.github/workflows/version-benchmark.yaml), any PR editing that file controls the steps that see the token, independent of the ref: master checkout used inside those steps. Scale is otherwise proportional to the change (new CI workflow + supporting scripts, no library API changes).


Generated by Claude Code

@apify-service-account

Copy link
Copy Markdown
Contributor

impit version comparison

Median throughput of the last 5 npm and PyPI releases of impit.

impit version comparison

pull_request runs the workflow file from the PR's own branch, and this
workflow's paths filter included itself, so a PR could edit the comment
job to run anything with its push token. workflow_run always uses the
copy on master instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnwET4Qxx14A9FG28Xniqx

barjin commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Good catch. Fixed in 6640681: split the comment-posting job into its own version-benchmark-comment.yaml, triggered by workflow_run instead of a second job on pull_request. workflow_run always runs the copy of the file from master, so a PR editing the workflow can no longer control what the job holding the push token does.

Note this means the comment workflow won't actually fire until it exists on master (a workflow_run listener is only registered from the default branch's copy) — so this PR itself won't get a fresh comment from the fixed path until merged; the earlier comment here was posted by the pre-fix version.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants