Skip to content

Add /lgtm + /approve review gate - #2277

Closed
riaankleinhans wants to merge 2 commits into
mainfrom
lgtm-and-approve-tooling
Closed

Add /lgtm + /approve review gate#2277
riaankleinhans wants to merge 2 commits into
mainfrom
lgtm-and-approve-tooling

Conversation

@riaankleinhans

Copy link
Copy Markdown
Contributor

What this does

This PR adds an automated review gate for pull requests, driven by two chat commands in PR comments:

  • /lgtm — a code owner signals the change looks good (review sign-off).
  • /approve — a different code owner authorizes the merge.

A PR is considered "ready" only when both commands have been given by two different code owners. The result is published as a commit status check (lgtm/approved) and mirrored with two labels (lgtm, approved) plus a running status comment on the PR.

Only people listed in CODEOWNERS for the files a PR touches can give these sign-offs, so review authority follows the existing ownership rules.

How it works, step by step

  1. A reviewer comments /lgtm or /approve. A workflow triggers on the comment.
  2. The bot validates the commenter:
    • They must be a CODEOWNER for at least one file changed in the PR.
    • They cannot sign off on their own PR (the author is blocked).
    • The same person cannot do both /lgtm and /approve — those must come from two different people.
    • If any check fails, the bot reacts 👎 and leaves a short comment explaining why. Valid commands get a 👍.
  3. The bot recomputes the overall state and requires:
    • at least the configured number of /lgtm sign-offs, and
    • at least the configured number of /approve sign-offs (from different people), and
    • that every changed file in the PR is owned by at least one of the people who signed off (so a sign-off can't "cover" files that person doesn't own).
  4. It publishes the result:
    • a commit status lgtm/approved (green when all requirements are met, red otherwise),
    • the lgtm and approved labels,
    • a single sticky comment that lists who has signed off, what's still missing, and which files still need an owner.
  5. /remove-lgtm and /remove-approve let a reviewer retract their own sign-off; the PR author can also clear a sign-off axis.
  6. Pushing a new commit resets everything. When new code is pushed, prior /lgtm and /approve sign-offs are cleared and the gate goes back to red, so approvals always reflect the latest code. The bot posts a note when this happens.

Files in this PR

  • lgtm.yml — the workflow. Runs on PR comments (to process commands) and on PR open/reopen/new-commit (to initialize and reset). The header documents all configuration.
  • index.js — all the logic (parsing commands, reading CODEOWNERS, resolving team membership, computing coverage, updating labels/status/comment). It has no external dependencies.
  • labels.yaml — adds the lgtm and approved label definitions so they aren't auto-pruned.

Configuration (repository variables)

Behavior is controlled by repo variables, so no code changes are needed to tune or disable it:

  • LGTM_GATE_ENABLED — master on/off switch. When false, the status is always green (observe-only). This PR ships with it set to false.
  • LGTM_MIN / APPROVE_MIN — how many of each sign-off are required (default 1 each).
  • LGTM_OWNERSHIP_MODE — ownership evaluation mode (coverage).

Rollout / how to review

  • The gate is currently in observe-only mode (LGTM_GATE_ENABLED=false): it will post the status comment and labels and show what it would decide, but it will not block any merges.
  • After we've watched it on a few PRs and are comfortable, enabling enforcement is two steps: flip LGTM_GATE_ENABLED to true and add lgtm/approved as a required status check in branch protection.

Security notes

  • The workflow reads CODEOWNERS from the PR's base branch (trusted), never from the PR's own changes — so a PR can't grant itself ownership by editing CODEOWNERS or the gate logic.
  • It never checks out or runs code from the PR's branch.
  • Team-based owners (e.g. @org/team) are resolved via the GitHub API; if the token lacks org read permission, the run fails visibly rather than silently miscounting.

Signed-off-by: Riaan Kleinhans <riaankleinhans@gmail.com>
@riaankleinhans
riaankleinhans requested review from a team as code owners August 13, 2026 12:56
@github-actions github-actions Bot added needs-triage Indicates an issue or PR that has not been triaged yet (has a 'triage/foo' label applied) needs-kind Indicates an issue or PR that is missing an issue type or kind (a kind/foo label) labels Aug 13, 2026
@github-actions github-actions Bot added the needs-group Indicates an issue or PR that has not been assigned a group (toc or tag/foo label applied) label Aug 13, 2026
@riaankleinhans riaankleinhans added kind/enhancement General items related to enhancements or improvements. toc toc specific issue triage/valid Issue or PR is valid with enough information to be actionable and removed needs-group Indicates an issue or PR that has not been assigned a group (toc or tag/foo label applied) needs-triage Indicates an issue or PR that has not been triaged yet (has a 'triage/foo' label applied) needs-kind Indicates an issue or PR that is missing an issue type or kind (a kind/foo label) labels Aug 13, 2026
@riaankleinhans
riaankleinhans requested a lite review from Copilot August 13, 2026 13:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a CODEOWNERS-driven merge gate for PRs that tracks /lgtm and /approve slash commands, publishes the combined result as a commit status (lgtm/approved), and mirrors state via labels and a sticky status comment.

Changes:

  • Introduces a new GitHub Actions workflow that reacts to PR lifecycle events and PR comment slash commands to compute/update gate state.
  • Adds a JS “core” module that loads CODEOWNERS from the PR base commit, validates sign-offs, computes coverage, and updates labels/status/sticky comment.
  • Registers lgtm and approved labels in .github/labels.yaml.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
.github/workflows/lgtm.yml New workflow to process /lgtm//approve commands and reset state on PR lifecycle events.
.github/actions/lgtm-core/index.js Implements CODEOWNERS parsing, signer validation, coverage computation, and GitHub updates (labels/status/sticky comment).
.github/labels.yaml Adds lgtm and approved label definitions for repo label automation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/lgtm.yml Outdated
Comment thread .github/actions/lgtm-core/index.js
Comment thread .github/actions/lgtm-core/index.js Outdated
Comment thread .github/actions/lgtm-core/index.js
- Narrow comment trigger to the four handled commands (avoid runs on
  unrelated slash commands like /assign, /retest)
- Validate LGTM_OWNERSHIP_MODE and fail loudly on unsupported values
- Sync in-code LABEL_DEFS with .github/labels.yaml (color/description)
- findStickyComment now keeps the newest match to avoid stale status

Signed-off-by: Riaan Kleinhans <riaankleinhans@gmail.com>
@jeefy

jeefy commented Aug 13, 2026

Copy link
Copy Markdown
Member

Couple things. First, this type of work should live in cncf/automation. This isn't a pattern exclusive to the TOC repo.

Second... https://github.com/marketplace/actions/prow-github-actions already exists. :) I'd rather not vibe out a solution for an already solved problem.

@riaankleinhans

Copy link
Copy Markdown
Contributor Author

@jeefy We have been vibing /commands for a while now.
https://github.com/marketplace/actions/prow-github-actions sure seem like a more reliable way.

@github-project-automation github-project-automation Bot moved this from New to Done in CNCF TOC Board Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement General items related to enhancements or improvements. toc toc specific issue triage/valid Issue or PR is valid with enough information to be actionable

Projects

Status: Done
Status: No status
Status: No status
Status: No status

Development

Successfully merging this pull request may close these issues.

6 participants