Skip to content

fix: drop the re-run helper's pull-requests permission (callers fail at startup) - #66

Merged
michieldegezelle merged 1 commit into
mainfrom
fix-helper-permissions
Oct 6, 2026
Merged

michieldegezelle merged 1 commit into
mainfrom
fix-helper-permissions

Conversation

@michieldegezelle

Copy link
Copy Markdown
Collaborator

Description

Urgent: every LU liquid-test run fails at startup since #63 merged (e.g. lu_market run 37429525996: startup_failure, zero jobs). The required check never reports, so LU PRs are blocked. nl/be would hit the same as soon as they switch.

Cause: #63 gave rerun-losing-run pull-requests: read. A called workflow can't request more than the calling job grants, and callers grant contents: read + actions: write, so GitHub rejects the whole run.

Fix: drop that permission. The PR-closed/moved check now reads the PR's current head from the losing run's own pull_requests field, which the runs API already returns. It lists only open PRs, with their live head (checked on real runs: open PR → current head, merged PR → empty), so no extra API call is needed.

Type of change

  • Bug fix
  • New feature
  • Breaking change

Test plan

  • actionlint: no new findings
  • lu_market's caller pointed at this branch on a throwaway branch and dispatched (run 37431706411): all 5 jobs created (no startup failure), the helper refused the dummy input as designed, no Slack page. Branch deleted.
  • After merge: the next LU PR run starts normally

🤖 Generated with Claude Code

…ller at startup

A called workflow can't request more than the calling job grants. The callers
grant contents: read + actions: write, so pull-requests: read on
rerun-losing-run made every run a startup_failure (zero jobs, the required
check never reports). Read the PR's current head from the run's own
pull_requests field instead, which needs no extra permission.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Note

Some coding-guideline sources were skipped; the reason is listed next to each one. Other guidelines were applied as usual. On a public repository, sources from your configuration are listed without their names.

Skipped guideline sources (1)
a configured source — invalid entry; expected repo:path or owner/repo:path with a relative file path

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: 4edc3ac7-cc8f-4a29-ae23-fa6f0ba6a01a
📥 Commits

Reviewing files that changed from the base of the PR and between a896208 and 88048eb.

📒 Files selected for processing (1)
  • .github/workflows/run_tests_inline_auth.yml

Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

📜 Recent review details
🔇 Additional comments (2)
.github/workflows/run_tests_inline_auth.yml (2)

131-131: LGTM!


231-231: 🎯 Functional Correctness

The behavior is undecidable from the supplied evidence. The comparison request exited before either API response was obtained because REPO was unset. The evidence therefore does not establish whether an older workflow-run response retains its original embedded PR head after a push.


Walkthrough

The workflow removes pull-request read permission from rerun-losing-run. Before rerunning failed jobs, it checks the run’s embedded pull-request head SHA against the run’s head_sha.

Changes

Inline test rerun validation

Layer / File(s) Summary
Gate reruns on the embedded PR head SHA
.github/workflows/run_tests_inline_auth.yml
The workflow grants only actions: write. The helper skips reruns if the run has no embedded pull-request head SHA or if that SHA differs from head_sha.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 88048

This change fixes the startup failure caused by the excess permission request. The remaining uncertainty is whether the embedded PR head stays current after a new push. It is worth confirming on the next LU PR run after merge.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the permission change and the startup failure it addresses.
Description check ✅ Passed The description explains the cause, fix, and validation results. It omits the template’s Fixes # reference and README/version/documentation checklist, but is otherwise complete.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@michieldegezelle michieldegezelle self-assigned this Oct 6, 2026
@michieldegezelle
michieldegezelle merged commit ff1929b into main Oct 6, 2026
1 check passed
michieldegezelle added a commit that referenced this pull request Oct 8, 2026
…nd this week's lessons

Drops examples that pointed at check_auth.yml, the old run_tests.yml,
refresh_token.yml and be_market's fork. Corrects two claims measured wrong in
lu_market: a re-run also reads the current secret, and a dispatched retry never
clears a PR's red check (only a re-run does). Adds checklist 18: a job-level
permission in a reusable workflow beyond what a caller grants fails that
caller's every run at startup (#66).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
michieldegezelle added a commit that referenced this pull request Oct 8, 2026
…#68)

* chore: remove the old run_tests.yml, check_auth.yml and update_templates_production.yml

All four markets now call run_tests_inline_auth.yml, so nothing calls the old
run_tests.yml or check_auth.yml any more (checked every workflow in
be/nl/lu/uk, plus an org-wide code search). update_templates_production.yml
only ran on this repo's own PRs, called the old run_tests.yml and failed on
every run.

README: drop the two old sections, describe run_tests_inline_auth.yml on its
own. Comments that pointed at the removed files are updated.

update_templates_review.yml and add_shared_parts_*.yml stay for now: a
colleague may reuse ideas from them for a deployment pipeline.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* chore: also remove update_templates_review.yml and add_shared_parts_*.yml

Dormant label-triggered workflows that only ran on this repo's own PRs and
predate the current login handling. push_to_review_firm.yml is the
maintained successor of update_templates_review.yml.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* docs(skill): update writing-bso-workflows for the removed workflows and this week's lessons

Drops examples that pointed at check_auth.yml, the old run_tests.yml,
refresh_token.yml and be_market's fork. Corrects two claims measured wrong in
lu_market: a re-run also reads the current secret, and a dispatched retry never
clears a PR's red check (only a re-run does). Adds checklist 18: a job-level
permission in a reusable workflow beyond what a caller grants fails that
caller's every run at startup (#66).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* docs: fix stale sampler references (check_auth cron, REPO_ACCESS_TOKEN scopes)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

* docs: point remaining run_tests.yml references at run_tests_inline_auth.yml; sampler token needs Pull requests write

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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.

1 participant