Skip to content

feat: Add factory-approve action for auto-approving trivial PRs - #35

Open
mfori wants to merge 19 commits into
mainfrom
claude/session-5mui7r
Open

feat: Add factory-approve action for auto-approving trivial PRs#35
mfori wants to merge 19 commits into
mainfrom
claude/session-5mui7r

Conversation

@mfori

@mfori mfori commented Jul 16, 2026

Copy link
Copy Markdown
Member

Adds factory-approve: a label-gated pipeline that auto-approves trivial PRs (copy, comments, styling, small self-contained fixes) so they don't consume human review time. A repo consumes it from a thin pull_request_target workflow — apify/apify-core#29249 does that, and it already runs in apify-dev-sandbox where the whole thing was tested end to end.

How it decides: free deterministic gates run first (trusted author and actor, PR size, allowed file types, denied paths, no risky added lines, conventional title). Only when all of them pass, two Claude reviewers judge the diff independently — is this actually trivial, is it correct, does it match the conventions of the surrounding code? Both must approve. Posting happens in a separate deterministic step as apify-factory, and everything fails closed: a crash or malformed verdict never approves.

The decisions worth knowing about:

  • It lives here so other repos can reuse it. Defaults in policy.mts are a generic baseline; anything repo-specific comes from the consuming workflow through the policy input (strictly validated JSON — unknown keys or wrong types fail closed before any LLM runs). Limits and allowlists are up to the consuming repo; only the supply-chain deny globs and the built-in risky-content patterns can't be removed. The override surface is documented in the README.
  • Costs: gates are free and filter most non-candidates; the cheaper model (sonnet) runs before the expensive one (opus) and a rejection short-circuits; every verdict embeds a fingerprint of the reviewed content, so pushes that don't change the diff (develop syncs, rebases, empty commits) skip the review entirely; and the pipeline stands down completely when a human review is active. Full approval ≈ $0.15, typical rejection ≈ $0.05, skips $0.
  • Two different models, the second prompted adversarially — same-model jurors share blind spots.
  • The reviewer can only read code and write a verdict file, never touch the PR. PR content enters the prompt fenced as untrusted data, and instruction-like text in it is itself grounds for rejection (verified with live injection attempts in the sandbox).
  • Each run posts a new comment and folds older ones as outdated; superseded approvals are dismissed. History is never edited.

There's also a backtest CLI that replays the pipeline (including a repo's policy overrides) over recent PRs without posting anything.

Merge this first — the apify-core workflow references factory-approve@main.

https://claude.ai/code/session_01Qx1PzGEJfijvERu3LMBCMD

@mfori
mfori marked this pull request as ready for review July 24, 2026 22:49
@mfori
mfori force-pushed the claude/session-5mui7r branch from 9797855 to 8faf47d Compare July 25, 2026 21:34
Label-gated pipeline that auto-approves trivial PRs. Deterministic safety
gates run first; only when every gate passes do two independent Claude
reviewers (the second adversarial) judge the diff, and only unanimous
approval makes the factory account post an approving review locked to the
reviewed commit. Fails closed everywhere, never requests changes, never
merges.

Re-runs are cheap: a content fingerprint (insensitive only to hunk line
numbers) skips re-reviewing unchanged diffs, and an active human review
stands the pipeline down entirely. Includes a backtest CLI that replays
the pipeline against recent PRs without posting anything.

The built-in policy is a generic org-wide baseline; consuming repositories
tune it through the optional `policy` input, a strictly validated JSON
overrides document that can tighten anything but only loosen what is
explicitly loosenable (hard numeric ceilings, immutable core deny globs
and risky-content patterns, fail-closed on any invalid value). Design:
factory-approve/docs/policy-overrides-spec.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qx1PzGEJfijvERu3LMBCMD
@mfori
mfori force-pushed the claude/session-5mui7r branch from 8faf47d to 72579d1 Compare July 27, 2026 09:14
Label-gated pipeline that auto-approves trivial PRs. Deterministic safety
gates run first; only when every gate passes do two independent Claude
reviewers (the second adversarial) judge the diff, and only unanimous
approval makes the factory account post an approving review locked to the
reviewed commit. Fails closed everywhere, never requests changes, never
merges.

Re-runs are cheap: a content fingerprint (insensitive only to hunk line
numbers) skips re-reviewing unchanged diffs, and an active human review
stands the pipeline down entirely. Includes a backtest CLI that replays
the pipeline against recent PRs without posting anything.

The built-in policy is a generic org-wide baseline; consuming repositories
tune it through the optional `policy` input, a strictly validated JSON
overrides document that can tighten anything but only loosen what is
explicitly loosenable (hard numeric ceilings, immutable core deny globs
and risky-content patterns, fail-closed on any invalid value). Design:
factory-approve/docs/policy-overrides-spec.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qx1PzGEJfijvERu3LMBCMD
@mfori
mfori force-pushed the claude/session-5mui7r branch from 72579d1 to 74248b3 Compare July 27, 2026 09:27
mfori and others added 2 commits July 27, 2026 11:33
…de/session-5mui7r

# Conflicts:
#	factory-approve/action.yaml
#	factory-approve/scripts/github_api.mts
#	factory-approve/scripts/post_verdict.mts
Label-gated pipeline that auto-approves trivial PRs. Deterministic safety
gates run first; only when every gate passes do two independent Claude
reviewers (the second adversarial) judge the diff, and only unanimous
approval makes the factory account post an approving review locked to the
reviewed commit. Fails closed everywhere, never requests changes, never
merges.

Re-runs are cheap: a content fingerprint (insensitive only to hunk line
numbers) skips re-reviewing unchanged diffs, and an active human review
stands the pipeline down entirely. Includes a backtest CLI that replays
the pipeline against recent PRs without posting anything.

The built-in policy is a generic org-wide baseline; consuming repositories
tune it through the optional `policy` input, a strictly validated JSON
overrides document that can tighten anything but only loosen what is
explicitly loosenable (hard numeric ceilings, immutable core deny globs
and risky-content patterns, fail-closed on any invalid value). Design:
factory-approve/docs/policy-overrides-spec.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qx1PzGEJfijvERu3LMBCMD
@mfori
mfori force-pushed the claude/session-5mui7r branch from 74248b3 to 9fc686b Compare July 27, 2026 09:33
mfori and others added 2 commits July 27, 2026 11:36
…de/session-5mui7r

# Conflicts:
#	factory-approve/factory_approve.test.mts
#	factory-approve/scripts/prompt.mts
Label-gated pipeline that auto-approves trivial PRs. Deterministic safety
gates run first; only when every gate passes do two independent Claude
reviewers (the second adversarial) judge the diff, and only unanimous
approval makes the factory account post an approving review locked to the
reviewed commit. Fails closed everywhere, never requests changes, never
merges.

Re-runs are cheap: a content fingerprint (insensitive only to hunk line
numbers) skips re-reviewing unchanged diffs, and an active human review
stands the pipeline down entirely. Includes a backtest CLI that replays
the pipeline against recent PRs without posting anything.

The built-in policy is a generic org-wide baseline; consuming repositories
tune it through the optional `policy` input, a strictly validated JSON
overrides document that can tighten anything but only loosen what is
explicitly loosenable (hard numeric ceilings, immutable core deny globs
and risky-content patterns, fail-closed on any invalid value). Design:
factory-approve/docs/policy-overrides-spec.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qx1PzGEJfijvERu3LMBCMD
@mfori
mfori force-pushed the claude/session-5mui7r branch from 9fc686b to 895a116 Compare July 27, 2026 09:39
mfori and others added 2 commits July 27, 2026 11:46
…de/session-5mui7r

# Conflicts:
#	factory-approve/README.md
#	factory-approve/factory_approve.test.mts
#	factory-approve/scripts/prompt.mts
Label-gated pipeline that auto-approves trivial PRs. Deterministic safety
gates run first; only when every gate passes do two independent Claude
reviewers (the second adversarial) judge the diff, and only unanimous
approval makes the factory account post an approving review locked to the
reviewed commit. Fails closed everywhere, never requests changes, never
merges.

Re-runs are cheap: a content fingerprint (insensitive only to hunk line
numbers) skips re-reviewing unchanged diffs, and an active human review
stands the pipeline down entirely. Includes a backtest CLI that replays
the pipeline against recent PRs without posting anything.

The built-in policy is a generic org-wide baseline; consuming repositories
tune it through the optional `policy` input, a strictly validated JSON
overrides document that can tighten anything but only loosen what is
explicitly loosenable (hard numeric ceilings, immutable core deny globs
and risky-content patterns, fail-closed on any invalid value). Design:
factory-approve/docs/policy-overrides-spec.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qx1PzGEJfijvERu3LMBCMD
@mfori
mfori force-pushed the claude/session-5mui7r branch from 895a116 to 937487d Compare July 27, 2026 09:51
mfori added 2 commits July 27, 2026 11:54
…de/session-5mui7r

# Conflicts:
#	factory-approve/factory_approve.test.mts
@mfori mfori self-assigned this Jul 27, 2026
@mfori mfori added the adhoc label Jul 27, 2026
claude and others added 5 commits July 27, 2026 10:01
The max-lines test hardcoded a 110-line diff, which passes the raised
150-line default. Derive the limit-test rows from the policy so default
tweaks don't break them, and sync README/spec doc with the new defaults
(150 lines, 80k diff chars, claude-opus-5 as second reviewer).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qx1PzGEJfijvERu3LMBCMD
The README's Configure section documents the override surface; the
separate design document is not needed in the repo.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qx1PzGEJfijvERu3LMBCMD
A repo's workflow config is trusted — if a repo wants higher limits,
that's its call. Numeric overrides are still validated as positive
integers, and the reviewer-count maximum stays because the action wires
exactly two reviewer steps.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Qx1PzGEJfijvERu3LMBCMD

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Introduces a reusable factory-approve composite GitHub Action that can auto-approve label-gated “trivial” PRs using deterministic safety gates followed by unanimous LLM reviewer verdicts, plus a local backtest CLI to replay the pipeline without posting to GitHub.

Changes:

  • Add the factory-approve composite action with three-stage flow: static gates, one/two Claude reviewers, deterministic posting of approve/reject/error.
  • Add policy resolution/validation, fingerprint-based memoization, head-file materialization for reviewers, and markdown reporting.
  • Wire repo tooling to include/lint the new factory-approve/**/*.mts code and add Vitest coverage for core behaviors.

Reviewed changes

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

Show a summary per file
File Description
tsconfig.json Includes factory-approve/**/*.mts in TS project scope.
oxlint.config.ts Disables no-console for factory-approve/** scripts (CLI-style logging).
factory-approve/action.yaml New composite action wiring static gates, one/two Claude verdict steps, and deterministic posting.
factory-approve/README.md Documentation for usage, configuration, setup, and backtesting.
factory-approve/factory_approve.test.mts Vitest coverage for gates, policy resolution, fingerprinting, prompt/report building, and verdict parsing.
factory-approve/backtest/backtest.mts Backtest CLI to replay the pipeline over recent PRs without posting.
factory-approve/backtest/claude_cli.mts Local claude CLI runner that enforces verdict-file contract and fails closed.
factory-approve/scripts/checks.mts Static safety gates (author/actor, size, paths, risky-content scanning, title regex, etc.).
factory-approve/scripts/context_files.mts Fetches and writes post-change “head files” for complete-file review context.
factory-approve/scripts/fingerprint.mts Content fingerprinting + memo marker parsing to skip unchanged diffs and detect prior verdicts.
factory-approve/scripts/github_api.mts Dependency-free GitHub REST/GraphQL helpers for reads, approvals, comments, and minimizing outdated reports.
factory-approve/scripts/glob_match.mts Minimal dependency-free glob matcher for deny/allow path rules.
factory-approve/scripts/policy.mts Default policy + strict JSON override validation with fail-closed semantics.
factory-approve/scripts/post_verdict.mts Deterministically aggregates gates + reviewer verdict files and performs the only GitHub writes.
factory-approve/scripts/prepare_review.mts Stage 1: fetch PR context, run static gates, compute skip/fingerprint, emit reviewer prompts via outputs.
factory-approve/scripts/prompt.mts Prompt builder with nonce-fenced untrusted PR data and explicit injection rejection rules.
factory-approve/scripts/report.mts Markdown report builder for reject/error comments and minimal approve review body.
factory-approve/scripts/verdict.mts Strict verdict JSON parsing + unanimity aggregation logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread factory-approve/README.md Outdated
Comment thread factory-approve/README.md Outdated
Comment thread factory-approve/scripts/post_verdict.mts Outdated
mfori and others added 3 commits July 27, 2026 16:20
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

mfori commented Jul 27, 2026

Copy link
Copy Markdown
Member Author

For reference, the end-to-end test PRs in apify-dev-sandbox (all closed now, reports and reviews are still visible there):

  • #81 — approve (trivial comment fix), plus fingerprint skips on unchanged re-runs, comment folding, approval supersession, and an early prompt-injection rejection
  • #82 — reviewer reject: runtime-limit value change (needs a human)
  • #83 — approve (spelling fix)
  • #84 — reviewer reject: booby-trapped "fix" (inverted sort presented as a bugfix), caught with a precise explanation
  • #85 — gates reject on a denied path, $0, no LLM
  • #86 — human-review stand-down (pipeline goes silent once a human approves or requests changes)
  • #88 — injection attempt using spoofed section tags in the PR description, rejected with the mandated reason
  • #89 — convention-violation reject (var, snake_case, wrong quoting in an otherwise correct change)

Generated by Claude Code

@mfori
mfori requested review from Jkuzz and protoss70 July 27, 2026 14:44
@mfori mfori changed the title Add factory-approve action for auto-approving trivial PRs feat: Add factory-approve action for auto-approving trivial PRs Jul 27, 2026
@protoss70

Copy link
Copy Markdown

Looks good, I think combined with Apify's trust culture.

Security wise I feel like this could be available to a limited number of people maybe. As the company is growing it might not be okay that everyone can just add the label and skip review even if they just joined the company.

Also if someones account gets hacked they might be able to trick this to push content into even main just by making a small PR -> allowing the bot to review it -> changing the PR into a huge malicious one. Please feel free to disagree with me if you think these concerns are not an issue.

With all this regarding the AI review, because it can be tricked easily from a malicious Actor, I think this all comes down to trust on the person who will actually use this.

Removing the factory-approve label used to switch the pipeline off while an
already-posted approval kept counting toward required reviews — so pushes made
after the removal could merge under an approval that reviewed none of them.

The label is now treated as the standing mandate for the approval: every run
re-checks the live PR, and when the label is absent the pipeline reviews and
posts nothing but dismisses any active factory approval (a new dismissReason
gates field, checked before the human-review stand-down and the gates outcome).
Consuming workflows should add `unlabeled` to their trigger types and let
those runs through the label guard, drafts included — see the README.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01623uAcnTmjNLxzxTXK1duE
@mfori

mfori commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

Looks good, I think combined with Apify's trust culture.

Hey @protoss70 thank you for your perspective.

IMHO, as you said, this is all about trust, and in Apify, this applies to any tool/service you get access to.

Security wise I feel like this could be available to a limited number of people maybe. As the company is growing it might not be okay that everyone can just add the label and skip review even if they just joined the company.

There are already limitations that only people from product-engineering are allowed to use this, but I get your point about engineering new joiners. So I would again appeal to the trust, and that currently it's really strict to catch any more complex and suspicious PRs that need human review.

Also, in some of our non-essential repositories (like this one 😄), where we allow merging PRs to the main branch without review, people still wait for at least one review anyway.

Also if someones account gets hacked they might be able to trick this to push content into even main just by making a small PR -> allowing the bot to review it -> changing the PR into a huge malicious one. Please feel free to disagree with me if you think these concerns are not an issue.

This is actually handled, every new push to the PR branch will re-run this action and invalidate any stale review (based on a changes fingerprint to save some tokens on merging base branch in etc.). But changing a PR that has been approved to a malicious one while keeping the approval is not possible. There was one option to get around it by removing the label, but I fixed it, so removing the label also removes the approval.

With all this regarding the AI review, because it can be tricked easily from a malicious Actor, I think this all comes down to trust on the person who will actually use this.

Aggreed.

@mfori

mfori commented Aug 12, 2026

Copy link
Copy Markdown
Member Author

As a reaction to the points from @protoss70 above ⬆️, requesting review from @fnesveda or @mtrunkat
No need to check all the code, only the idea and security concerns

@mfori
mfori requested review from fnesveda and mtrunkat August 12, 2026 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants