electric-sql / electric-sql/electric

PR Shepherd v1

Open
#4,412 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
10.4k
Forks
375
Avg merge
3d 1h
Merged PRs (30d)
18

Description

# PR Shepherd v1 — Design

**Status:** draft
**Authors:** Valter (with Claude)
**Parent RFC:** electric-sql/electric#4411 (Electric Factory)
**Last updated:** 2026-05-26

## Summary

PR Shepherd is the first Electric Factory role and the first agent in the v1 cohort beyond Horton. It is a default reviewer on team PRs in `electric-sql/electric`: one persistent entity per open PR, woken by GitHub webhook events routed through the existing `github-repo-electricsql-electric` ingress in Electric Cloud. On each wake, Shepherd loads the `reviewing-pull-requests` skill, fans out one Electric Agents worker per review aspect (correctness, conventions, security, perf, tests, architecture), aggregates findings, and posts an incremental review comment under the `## PR Shepherd Review` marker. Shepherd never merges and never pushes commits in v1. Replaces the existing `claude.yml` and `claude-code-review.yml` workflows.

## Motivation

Per parent RFC #4411, PR Shepherd is one of the three v1 Factory roles (with Ops Agent and Daily Digest). The team wants:

- A default reviewer that catches concerns reliably and incrementally across pushes.
- A reusable agent shape (persistent-per-target) that other roles inherit.
- A concrete consumer of the webhook event-source primitives merged in `electric-sql/electric#4407` and the webhook ingress merged in `stratovolt#1548`.
- A path off the existing Claude Code review GitHub Action onto Electric Agents.

## Scope

This sub-issue covers a full PR Shepherd v1 implementation matching the RFC's role section, plus prerequisites:

- A `horton` GitHub App, manually created at the `electric-sql` org level and installed on `electric-sql/electric`.
- Reuse of the existing `github-repo-electricsql-electric` webhook ingress in Electric Cloud — no changes to stratovolt code.
- A new `packages/agents` package in OpenFactory housing the `pr-shepherd` and `pr-review-worker` entities and the orchestration skill.
- Removal of `claude.yml` and `claude-code-review.yml` from `electric-sql/electric/.github/workflows/` in the same PR that lands Shepherd.

## Non-goals

- Other Factory roles (Ops Agent, Daily, Releaser, Issue Reproducer, Issue Groomer, Dependency Steward, Issue Enricher).
- Cross-repo support — only `electric-sql/electric` in v1.
- Auto-merge or auto-approve. Never, per RFC trust boundary.
- Real implementations of the `fix-ci` and `address-review` label-gated write actions. Shepherd recognizes the labels in v1; the actual fix/address logic is a follow-up sub-issue.
- Codebase vector index for Horton (separate Horton extension).
- Production deploy of OpenFactory entities. Running locally during v1 is acceptable; remote deploy is a follow-up.

## Repo layout

```
OpenFactory/
packages/
agents/ ← NEW
src/
agents/
pr-shepherd.ts ← Persistent-per-PR orchestrator
pr-review-worker.ts ← Ephemeral per-aspect worker
prompts/
pr-review/
correctness.md ← pr-review.md §2.1
conventions.md ← pr-review.md §2.2
security.md ← pr-review.md §2.3
perf.md ← pr-review.md §2.4
tests.md ← pr-review.md §2.5
architecture.md ← pr-review.md §2.6 + §2.7
github-events.ts ← Bucket constants + GH event schemas
index.ts ← Entity registry export
skills/
reviewing-pull-requests.md ← Orchestration skill loaded by Shepherd
package.json ← Depends on @electric-ax/agents-runtime
discord-bot/ ← Untouched
docs/
setup-gh-app.md ← One-time GH App setup guide
AGENTS.md ← Onboarding doc
```

Convention introduced here and propagated to future Factory roles:

- `skills/*.md` — runtime-loaded LLM playbooks (loaded via `use_skill`).
- `src/prompts//*.md` — static prompt content bundled with agent code, passed to LLM calls by the handler.

## Existing infrastructure (no changes required)

- **Webhook ingress** at `proj-eager-tahr-dynnheosru/env-inner-swallow-2x32fv3mfv/svc-old-ermine-krt5oggnwi`. Source key `github-repo-electricsql-electric`. Catalog buckets already configured:
- `issues` → `issues/:number` (match `x-github-event in {issues, issue_comment}`)
- `pull_requests` → `prs/:number` (match `x-github-event in {pull_request, pull_request_review, pull_request_review_comment, issue_comment}`)
- `pull_request_opened` → `prs/opened` (match `pull_request && body.action == "opened"`)
- **Stream-to-wake bridge** in cloud-agents-server (stratovolt#1548) — already routes webhook deliveries into entity wakes.
- **Runtime helpers** in `@electric-ax/agents-runtime` (electric#4407): `webhook(endpointKey, { bucket })`, `list_event_sources`, `subscribe_event_source`, `list_event_source_subscriptions`, `unsubscribe_event_source`.

## Entities

### `pr-shepherd` — persistent-per-PR orchestrator

- **Entity URL:** `/pr-shepherd/{repo}/{prNumber}` — one entity per open PR.
- **Spawn trigger:** an ephemeral dispatcher entity observing `webhook(..., { bucket: "prs/opened" })`. On `pull_request.opened`, the dispatcher resolves the Shepherd URL for that PR and sends it an `inbox` wake (creating the entity if it doesn't exist).
- **Per-PR observation:** once spawned, Shepherd observes `webhook(..., { bucket: \`prs/${prNumber}\` })` so every subsequent event for that PR (synchronize, label, comment, review) wakes the same entity.
- **Death:** PR `closed` event (merged or not) — finishes any in-flight aggregation, then exits.

**State collections:**

- `reviews` — `{ sha, commentId, postedAt, summary }`. One row per posted review; drives the incremental-review marker.
- `findings` — `{ wakeId, aspect, finding[] }`. Rows written by workers, read by Shepherd to aggregate.
- `meta` — single row: `{ repo, prNumber, isTeamPr, labels, status: 'active' | 'paused' }`.

**Gating logic (per RFC trust boundary):**

- On first wake: classify PR as team vs community using `gh api orgs/electric-sql/members/{login}`. Team → default-on. Community → only act if `shepherd` label present.
- On every wake: re-check `pause-shepherd` and `no-shepherd` labels in `meta`. Paused → no-op. `no-shepherd` → permanent exit.
- Write actions (`fix-ci`, `address-review`) only fire on label-add events whose label matches and where the entity is `active`. v1 stubs only — see Non-goals.

**Per-wake action:**

- `opened` → load skill → spawn the six per-aspect workers (one-time, persistent for the PR's lifetime) → send each a `review` message with `base..head` → wait → aggregate → post → record marker.
- `synchronize` → load skill → send the existing workers a `review` message with `lastMarkerSha..head`. Skip the message for aspects whose files weren't touched in the new range. Wait → aggregate → post → record marker.
- `pull_request_review` / `issue_comment` from a human → only act if the comment `@mentions` `horton` or `pr-shepherd`; otherwise update state and idle.
- `labeled` / `unlabeled` → update `meta`, react if action label.
- `closed` (merged or not) → send each worker a `shutdown` message, then exit.

### `pr-review-worker` — persistent-per-(PR, aspect) worker

- **Entity URL:** `/pr-review-worker/{repo}/{prNumber}/{aspect}` — one entity per PR per aspect; six per PR.
- **Spawn trigger:** Shepherd's first wake on a PR. Shepherd spawns the full set of workers and sends each its initial `review` message.
- **Spawn args:** `{ repo, prNumber, aspect, parentUrl }`. No diff range — that comes per wake in the inbox message.
- **Wake source:** inbox messages from Shepherd. Workers do not observe webhook buckets directly; all wake fan-out is mediated by Shepherd so aggregation timing stays in one place.
- **State collections:**
- `findings_history` — `{ wakeId, baseSha, headSha, postedAt, findings: Finding[] }`. One row per Shepherd-requested review pass. Lets the worker reference prior pushes when iterating ("flagged this in pass 1, author has not addressed").
- `meta` — single row: `{ repo, prNumber, aspect, promptPath, status: 'active' | 'shutdown' }`.
- **Inbox message types:**
- `{ type: 'review', wakeId, baseSha, headSha }` — perform a review pass on the given range. Worker reads its prior `findings_history`, fetches the diff via GH App Octokit, makes one LLM call with the aspect prompt + diff + prior-findings context, parses output, appends a new `findings_history` row, then `ctx.send(parentUrl, { type: 'worker-done', wakeId, aspect, findings })`.
- `{ type: 'shutdown' }` — set `status` to `shutdown` and exit. Sent by Shepherd when the PR closes.
- **Finding shape:**
```ts
type Finding = {
severity: 'critical' | 'important' | 'suggestion'
file: string
line?: number
message: string
suggestedFix?: string
}
```
- **Death:** on receipt of `shutdown` from Shepherd. Worker lifetime equals PR lifetime.
- **No GitHub writes.** Read-only. Only Shepherd ever posts comments.

## The orchestration skill

`packages/agents/skills/reviewing-pull-requests.md` — loaded by Shepherd on every wake via `use_skill`.

Adapted from `electric-sql/stratovolt/.claude/commands/pr-review.md`. The original prompt's structure (phases, severity definitions, output format, principles, example issue format) is preserved verbatim where possible. The single change is **Phase 2**: the skill no longer instructs the LLM to analyze all aspects itself — it instructs the LLM to message persistent per-aspect workers and aggregate their results.

**Phases:**

- **Phase 1 — Gather Context.** Same as `pr-review.md`. Sources: `gh pr view`, `gh pr diff`, `gh api` for prior Shepherd comments by marker. PRD/RFC references extracted by Phase 1 (rather than a workflow pre-step).
- **Phase 2 — Fan out.** On the first wake (PR opened), spawn the six per-aspect workers. On every wake (opened or synchronize), send each relevant worker a `review` message with the current diff range (skip the message for aspects whose files weren't touched). Wait via observation on the `findings` collection with a 5-minute wall-clock timeout.
- **Phase 3 — Iteration Awareness.** Unchanged. Marker `` instead of the file-based `previous_reviews.txt`.
- **Phase 4 — Compose Review.** Same template as `pr-review.md` Phase 4. Header swapped to `## PR Shepherd Review`. Sections per aspect, sorted by severity.
- **Phase 5 — Post.** Same `gh api` find-existing-comment + `gh pr comment` / `PATCH` logic.

**Severity, principles, example format** — copied verbatim from `pr-review.md`.

## GitHub identity (`horton` App)

Manually created at the `electric-sql` org. Documented in `docs/setup-gh-app.md`. Not automated.

- **App name:** `horton`. Reused across all Factory roles.
- **Webhook URL:** the existing `github-repo-electricsql-electric` ingress URL. The path secret is the App's webhook secret.
- **Permissions — read:** Contents, Issues, Pull requests, Checks, Metadata, Members.
- **Permissions — write:** Pull requests (comments + reviews), Issues (comments).
- **Subscribed events:** `pull_request`, `pull_request_review`, `pull_request_review_comment`, `issue_comment`, `issues`, `label`.
- **Install on:** `electric-sql/electric` only.
- **Credentials:** `GH_APP_ID`, `GH_APP_PRIVATE_KEY`, `GH_APP_INSTALLATION_ID`. Surfaced via env vars. 1Password for the team, `.env.local` for development.
- **Octokit construction:** `@octokit/auth-app` inline in `pr-shepherd.ts` and `pr-review-worker.ts`. No shared helper module in v1.

## Sub-issue text (filed under electric-sql/electric, child of #4411)

**Title:** PR Shepherd v1

**Body:**

> Implements the PR Shepherd role from #4411.
>
> **Scope:** see design doc at `OpenFactory/docs/superpowers/specs/2026-05-26-pr-shepherd-v1-design.md`. One persistent entity per open PR; six per-aspect workers; review posted under the `## PR Shepherd Review` marker; default-on for team PRs, opt-in (`shepherd` label) for community PRs.
>
> **Non-goals:** auto-merge, multi-repo, real `fix-ci`/`address-review` implementations (v1 stubs only), production deploy of OpenFactory.
>
> **Dependencies:**
> - `horton` GitHub App created at electric-sql org and installed on `electric-sql/electric` (manual).
> - `GH_APP_*` credentials provisioned to the runtime.
>
> **Exit criteria:**
> - Shepherd posts a review on a real PR in `electric-sql/electric`.
> - `claude.yml` and `claude-code-review.yml` removed from `.github/workflows/` in the same PR.
> - Design doc merged in OpenFactory.

## Replacing existing Claude integrations

- `claude-code-review.yml` and `claude.yml` removed in the same PR that lands Shepherd, per RFC §"Codebase impact".
- Old `## Claude Code Review` markers on closed PRs are left alone. On open PRs that already have a Claude comment, Shepherd posts a new `## PR Shepherd Review` comment alongside without touching the old one.

## Open questions

- Concrete relevance heuristics for the per-aspect "skip if no relevant files" rule. v1 starts conservative — never skip. Tune after first week of soak.
- The 5-minute aggregation timeout is a guess. Revisit after the first dozen real PRs.
- Whether `pr-review-worker` should be one entity type with an `aspect` discriminator, or one entity type per aspect. v1 uses a single type with discriminator — simpler registry, no measurable downside.

## Revision history

| Version | Date | Author | Changes |
| ------- | ---------- | ------ | ------------- |
| 0.1 | 2026-05-26 | Valter | Initial draft |

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.