Skip to content

Conversation

@PLeVasseur
Copy link
Collaborator

Summary

  • Fixes the burst assignment race by introducing a durable in-repo lease lock on state issue 📊 Reviewer Bot State (DO NOT CLOSE) #314 with optimistic concurrency (ETag plus If-Match).
  • Refactors state issue persistence to preserve explicit YAML and lock marker blocks so lock metadata cannot be clobbered by save_state().
  • Keeps read-level reviewers assignable as designated reviewers, adds truthful reviewer-request failure messaging for 422, and adds mandatory triage escalation handling.

Root Cause and Lock Strategy

  • Reviewer-bot runs were serialized per PR/issue key, but all runs still wrote to the same state issue body.
  • Parallel pull_request_target:labeled runs could overwrite each other in a last-writer-wins pattern.
  • This change adds lease acquire/release with retry/backoff, stale lease takeover, and hard failure on lock timeout or state persistence failure.

State Contract and Preserve Update Strategy

  • Adds explicit parse/render markers for state and lock blocks:
    • <!-- REVIEWER_BOT_STATE_START --> ... <!-- REVIEWER_BOT_STATE_END -->
    • <!-- REVIEWER_BOT_LOCK_START --> ... <!-- REVIEWER_BOT_LOCK_END -->
  • Adds parse_state_yaml_from_issue_body, parse_lock_metadata_from_issue_body, and render_state_issue_body helpers.
  • save_state() now re-reads latest body and uses conditional patching (If-Match) instead of blind body rewrites.

Mandatory Triage Approval Workflow

  • Adds idempotent label ensure-create behavior for triage approver required.
  • When a designated reviewer with read-level access approves:
    • marks review complete immediately
    • applies escalation label
    • posts one escalation ping comment per review cycle
  • First triage+ APPROVED satisfies mandatory approval, records satisfier metadata, and removes the escalation label.

Validation

  • uv run ruff check --fix
  • uv run pytest .github/reviewer-bot-tests (89 passed)

Runbook Metadata

@netlify
Copy link

netlify bot commented Feb 10, 2026

Deploy Preview for scrc-coding-guidelines ready!

Name Link
🔨 Latest commit a425804
🔍 Latest deploy log https://app.netlify.com/projects/scrc-coding-guidelines/deploys/698bac2e3a9991000853480d
😎 Deploy Preview https://deploy-preview-405--scrc-coding-guidelines.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@felix91gr
Copy link
Collaborator

@plaindocs would you be able to review this? I don't know much about GH Actions' concurrency primitives

@plaindocs
Copy link
Collaborator

@felix91gr I'm in the same boat. But if we need a scapegoat to click "approve" and hope I'm game :-D

@felix91gr
Copy link
Collaborator

I don't know much about GH Actions' concurrency primitives

Now I do - they suck 🤣 that's why Pete has to implement a mutex and a whole data structure just to make GHA behave.

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.

3 participants