fix(ci): clear stale query-count label/comment when snapshots match base#1583
Conversation
…ch base The Query Counts label workflow was path-filtered on the snapshot files, so it only ran when a PR's diff touched them. That created a catch-22: once a previously-changed snapshot matched base again (reverted, or base caught up via a merge), the PR diff no longer touched the files, so the workflow never ran and its self-heal (remove label, delete comment) could never fire. The result was a stale "query counts dropped" comment lingering on a PR whose net diff no longer changes any counts. Drop the paths filter and let the script decide from the actual base/head diff on every run: add the label and upsert the comment when snapshots differ, remove the label and delete the comment when they match. The job only reads the snapshot files via the API and manages labels/comments, so it never executes PR code despite pull_request_target.
|
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
emdash-demo-cache | 38cab83 | Jun 22 2026, 03:16 PM |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
docs | 38cab83 | Jun 22 2026, 03:16 PM |
github-actions
Bot
added
review/needs-review
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
emdash-playground | 38cab83 | Jun 22 2026, 03:18 PM |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approach — correct, solves the right problem. The paths: filter on a pull_request_target workflow whose entire purpose is to self-heal (drop a stale label/comment when snapshots match base again) is self-defeating: once a snapshot delta is reverted or neutralized, the PR diff stops touching the snapshot files, so the workflow can never run to clear the now-wrong label/comment. That is exactly the catch-22 described. Dropping paths: and letting the script decide from the actual base/head diff on every run is the right, minimal fix — the script already had the self-heal branch; the filter was the only thing preventing it from firing. This fits the existing design (the sibling Measure/Apply workflows are untouched) and EmDash's "fewer queries is right; more needs a conversation" stance is preserved.
Security posture — verified, claim holds. I checked the new file end-to-end: no actions/checkout, no run: step, no secrets consumed. It only fetches snapshot JSON via repos.getContent at base/head SHAs and manages labels/comments with GITHUB_TOKEN (pull-requests: write). Removing paths: does not change what the workflow can touch. pull_request_target runs the base branch's workflow file, so (as the PR notes) this only takes effect after merge to main.
Implementation — clean.
hasChange = sections.length > 0drives both the label and the comment consistently. The old two-step split (unconditionaladdLabelsin step 1 + conditional comment delete in step 2) is replaced by a single step that adds label+comment when changed and removes both when not — strictly more correct; the old code could leave a stale label when snapshots were touched-but-identical.createLabel422 andremoveLabel404 are caught correctly;addLabelsis idempotent.- Traced the lifecycle with the Measure (
pull_request) and Apply (workflow_run) workflows: after Apply pushes regenerated snapshots, the resultingsynchronizere-runs Label against the new head and converges (upsert in place). No spurious flapping. - Event types (
opened, synchronize, reopened, edited) are appropriate; running oneditedis harmless (same SHAs → samehasChange→ no-op or re-confirm). The "base caught up via a merge" case works because mergingmaininto the PR head firessynchronize. - CI YAML only → no published package changed, so no changeset is needed; i18n/typecheck/lint are n/a as the PR states.
Conclusion: No findings. Genuinely clean — I'd sign off.
Non-blocking, pre-existing (not introduced by this diff, flagged only for the record):
- No
concurrency:group, so rapid pushes can overlap on the label/comment mutations; the ops are idempotent and converge, so this is cosmetic. - Snapshot keys are interpolated into the comment markdown unescaped (
rows.push(\| `${key}` …``)); on a fork PR the head snapshot is author-controlled. GitHub sanitizes script handlers, so this is low-severity (markup, not script execution), and the realistic keys are generated route names — but a future hardening pass could escape keys/cells. Neither is touched by this PR.
github-actions
Bot
added
review/approved
1 participant
What does this PR do?
Fixes a catch-22 in the Query Counts — Label workflow that leaves a stale "query counts dropped" comment on PRs whose net diff no longer changes any counts.
The workflow was path-filtered on the snapshot files:
The script already self-heals (removes the label / deletes the comment when base and head match), but the
paths:filter means it only runs when the PR diff touches the snapshot files. Once a previously-changed snapshot matches base again — reverted, or base caught up via a merge — the PR diff no longer touches those files, so the workflow never runs and the self-heal can never fire. The stale comment lingers.Real-world trigger: #1579 (Astro 7 upgrade). Its early CI run committed a snapshot delta (16->9) and posted a
-28 queriescomment. After #1580 fixed the harness and the branch merged main, the head snapshots matched base again — net diff: zero snapshot changes — but the path-filtered workflow couldn't run to clear the now-wrong comment.Fix: drop the
paths:filter and let the script decide from the actual base/head diff on every run — add the label + upsert the comment when snapshots differ, remove the label + delete the comment when they match. The job only reads snapshot files via the API and manages labels/comments; it never checks out or executes PR code, so removing the filter doesn't change thepull_request_targetsecurity posture.Type of change
Checklist
pnpm typecheckpasses (n/a — CI YAML only, no source changed)pnpm lintpasses (n/a — CI YAML only)pnpm testpasses (n/a — CI YAML only)pnpm formathas been run (prettier-clean)AI-generated code disclosure
Notes
The stale comment already on #1579 won't clear until this lands on
main(pull_request_target uses the base branch's workflow definition) and #1579 gets another push. It can also be deleted by hand in the meantime.