Skip to content

ci: fix SQLFluff annotation permissions for fork PRs - #2148

Merged
alistairewj merged 6 commits into
MIT-LCP:mainfrom
Chessing234:ci/sqlfluff-annotate-permissions
Aug 8, 2026
Merged

ci: fix SQLFluff annotation permissions for fork PRs#2148
alistairewj merged 6 commits into
MIT-LCP:mainfrom
Chessing234:ci/sqlfluff-annotate-permissions

Conversation

@Chessing234

Copy link
Copy Markdown
Contributor

Summary

  • Split out of the too-broad fix: load paths, imports, transpile, and CI permissions #2128 per review: focus only on SQLFluff CI.
  • Give the workflow contents: read + checks: write, skip annotate when no concept SQL changed, and set ignore-unauthorized-error: true.
  • Document the contract in the workflow header + style guide, and add a local lint helper script.

Test plan

  • Open/update a PR that does not touch mimic-iv/concepts/**/*.sql — SQLFluff job should skip lint/annotate cleanly
  • Open a PR that changes a concept .sql file — annotations post (or are ignored gracefully on forks)
  • ./scripts/lint_changed_mimic_iv_concepts.sh against a branch with concept edits

Made with Cursor

Fork PRs need an explicit checks:write grant so the annotation action can
post findings without relying on the default GITHUB_TOKEN scope.
Avoid running the annotations action against a missing annotations.json on
PRs that do not touch mimic-iv/concepts, and tolerate unauthorized check
writes with ignore-unauthorized-error.
Spell out why the workflow sets checks:write and ignore-unauthorized-error so
future edits do not reintroduce continue-on-error.
Point contributors at the workflow that reviews changed MIMIC-IV concept SQL.
Mirrors the PR workflow so contributors can lint mimic-iv/concepts changes
before pushing.
@Chessing234

Copy link
Copy Markdown
Contributor Author

This unblocks fork PRs: every recent Chessing234 mimic-iv PR is red only on the Annotate step (Resource not accessible by integration), while Lint SQL itself succeeds. Happy to adjust if reviewers want a smaller permissions footprint.

ignore-unauthorized-error does not catch the fork 403
(Resource not accessible by integration). Skip Annotate when
head.repo != base repo so lint-mimic-iv can stay green.
Chessing234 added a commit to Chessing234/mimic-code that referenced this pull request Aug 2, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
@Chessing234

Copy link
Copy Markdown
Contributor Author

@alistairewj this is the focused SQLFluff annotate-permissions follow-up split out of #2128 (ignore-unauthorized-error / fork-safe annotate). Small and should be easy to land when you have a moment.

@Chessing234
Chessing234 force-pushed the ci/sqlfluff-annotate-permissions branch from 7435551 to d83dbaf Compare August 5, 2026 10:32
Chessing234 added a commit to Chessing234/mimic-code that referenced this pull request Aug 5, 2026
Chessing234 added a commit to Chessing234/mimic-code that referenced this pull request Aug 7, 2026
Chessing234 added a commit to Chessing234/mimic-code that referenced this pull request Aug 7, 2026
same fix as MIT-LCP#2148 — skip annotate when head repo is a fork
@Chessing234

Copy link
Copy Markdown
Contributor Author

@alistairewj small ci-only PR — skips sqlfluff annotate on fork PRs. green on my side, ready when you have a moment.

@alistairewj

Copy link
Copy Markdown
Member

@alistairewj small ci-only PR — skips sqlfluff annotate on fork PRs. green on my side, ready when you have a moment.

no need to tag me to bump, I follow the repo

@alistairewj
alistairewj merged commit ce97c45 into MIT-LCP:main Aug 8, 2026
21 of 22 checks passed
@Chessing234

Copy link
Copy Markdown
Contributor Author

understood, sorry for the noise — won't tag you to bump again.


Generated by Claude Code

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