| name | newsletter-service-agentic-pr |
| description | How to drive an open lfx-v2-newsletter-service pull request through the agentic review flow: read the lfx-reviewer "Agentic review check" comment, fix or rebut every blocking finding, answer every review thread, push one batched round at a time, and loop until the check is green on the current head with every thread answered — then report the ending: needs-human review before merge, or clear for the gate/automerge path. Use whenever a PR is open on this repo and the work involves review iteration — responding to Copilot or conductor findings, checking why `agentic-review/clean` is failing or why the gate has not approved, handling the `needs-human` label, or pushing fixes to an open PR. This is the PR driver's operating manual: the driver is a worktree-isolated background agent that works the PR autonomously (fixes, rebuttals, replies, pushes). The main session only launches the driver right after opening any PR here — with a minimal prompt pointing at this skill — and relays its round notes.
|
Working a PR through the agentic review flow
Who runs this. Unless a section says otherwise, "you" is the PR
driver: a worktree-isolated background agent that owns the loop end to
end. The main session reads only "Launching the PR driver" below.
Every push to an open PR on this repo starts a review round with no human in
the loop: Copilot reviews the diff, an escalation judge decides whether a
human must look (needs-human label), and the conductor adjudicates every
AI-reviewer thread (human-authored threads are never adjudicated — they only
count for the tidiness rule below) and posts an Agentic review check
comment as lfx-reviewer, stamping the agentic-review/clean commit status
on the head it judged. A deterministic gate approves the PR as lfx-reviewer
once four things hold: the current head's clean status is success, the
needs-human label is absent, every review thread has at least one reply
beyond the finding, and escalation is satisfied for the current head —
normally a per-head needs-human: no verdict comment. The gate deliberately
withholds while that verdict is in flight even with the label absent, so a
green check with no approval minutes after a push is usually the judge
finishing, not a fault.
Your job is a loop, not a conversation: everything you do is adjudicated
at the next push. Replies, rebuttals, and resolutions never trigger a
round — only pushes and PR open/reopen do. That is deliberate: it keeps
pipeline cost proportional to code changes and keeps anyone from talking the
gate open.
Setup
Every snippet below assumes these two variables. Assign PR first, from the
PR number in your launch prompt — never derive it from branch state: you
work detached (see "Driver operations"), and a detached HEAD has no current
branch for gh pr view to resolve. REPO comes from the checkout's remote,
which works detached:
PR=<pr>
REPO="$(gh repo view --json nameWithOwner --jq .nameWithOwner)"
The flow covers same-repository PRs only: the conductor and escalation
workflows skip fork PRs (their PAT-holding jobs must never run for a head
they do not own). Check before starting the loop:
gh pr view "$PR" --repo "$REPO" --json isCrossRepository --jq .isCrossRepository
If that prints true, no round, check comment, or clean status will ever
appear — the PR follows ordinary human review. Tell the user and stop; do
not poll for a verdict that cannot come.
The round loop
- Read the latest check comment (commands below) and confirm its hidden
head: line matches the PR's current head. A stale check means the
current head's round is already running or queued (every push starts
one) — poll for its verdict and wait. Never push just to refresh a stale
check: that cancels the running round and burns another. On a reused SHA
(reopen, force-push back) a matching head: alone is not proof the check
is current — anchor on the round's own pending stamp per "Waiting for the
verdict".
- Triage every blocking row.
[critical], [high], and [should-fix]
block; [nit] never does. For each blocking finding: fix it, or reply on
its thread with a substantive technical rebuttal. Both are adjudicated at
the next push — an accepted rebuttal clears the thread as
rebutted-valid.
- Reply on every thread you act on, then resolve it. Say what you
changed and how it addresses the finding, or why it stands as is. Then
resolve the thread (mutation below — your login can, the pipeline's
tokens cannot). The reply is the audit record and satisfies tidiness;
resolution just keeps the conversation view clean for the humans who come
after the gate — adjudication reads the ledger, not the flag, so
resolving never hides an insufficient fix or rejected rebuttal from being
carried forward and re-opened.
- Answer every remaining thread that has no reply yet, nits and human
comments included. The gate withholds while any thread is unanswered,
even on a clean change — nothing is dismissed without a recorded reason.
- Batch the whole round into one push. Each push burns a Copilot review
plus a reconcile model run, and a mid-round push strands the running
round (its eventual comment is head-bound and inert, but it is noise).
Write all fixes, post all replies, resolve the answered threads, then
push once:
- Code changes: one signed, signed-off commit carrying every fix
(
git commit -s -S, this repo's DCO + GPG convention).
- Rebuttal-only (nothing to change in the tree): rebuttals are only
adjudicated at a push, so push a signed empty commit —
git commit --allow-empty -s -S -m "chore(review): submit rebuttals for adjudication".
- Replies-only on an already-clean head: no push at all — the scheduled
sweep re-runs the gate within ~10 minutes and releases the approval
once every thread is answered.
- Wait for the verdict on the new head (command below; a round takes
10–20 minutes), then loop from step 1.
- Stop at the goal: the check reads
✅ clean on the current head and
every thread is answered. Report which ending applies per "Endings".
Endings
Your final report names one of two endings:
- "Needs human review before merge" — the goal reached with the
needs-human label set. Green-plus-answered is the ending itself: only a
human review can move the PR further.
- "Clear for the gate/automerge path" — the goal reached plus an
authoritative current-head no-human signal: the gate's approving review,
or the head's
needs-human: no verdict comment with no needs-human
unlabel event postdating it. The gate rejects a verdict that a later
unlabel supersedes unless the remover was on its allowlist — a check only
the gate can make — so with any unlabel after the verdict, only the
actual approval settles it.
Label absence alone is NEITHER ending: escalation may still be in flight,
and a late yes verdict can add the label after clean. Keep waiting (the
approval can lag your last reply by up to ~10 minutes — replies do not
trigger the gate, a scheduled sweep does); if you must report first, say
escalation is pending rather than clear.
Reading the check comment
The newest lfx-reviewer comment containing the machine marker is the
authoritative state. The ledger (per-thread adjudications, head:,
clean:) is collapsed inside a <details> element — read the raw body:
CID="$(gh api "repos/$REPO/issues/$PR/comments" --paginate \
--jq '.[] | select(.user.login=="lfx-reviewer" and (.body | contains("<!-- agentic:check v1 -->"))) | .id' | tail -1)"
if [ -n "$CID" ]; then
gh api "repos/$REPO/issues/comments/$CID" --jq .body
else
echo "no agentic check posted yet"
fi
On a fresh PR or right after a push, no check exists yet and CID is
empty — never call the comments endpoint with an empty id. An absent check
is not an error: the round is running, so switch to the status poll below.
In the body: the headline gives the blocking count, the Blocking table
names what stands between you and clean, and the ledger's - id: rows
record every adjudicated thread with its status (fixed, obsolete,
outstanding, rebutted-valid, rebutted-invalid) and reason.
outstanding and rebutted-invalid rows are your work list.
Waiting for the verdict
Poll the head's commit status; the conductor stamps pending at round start
and success/failure when the check posts. Statuses are per-PR-bound via
the description, and the newest status id wins. Select the newest first,
then look at its state — filtering pending out before selecting would, on
a reused SHA, surface a stale terminal state while the current round's newer
pending is the truth:
HEAD="$(gh pr view "$PR" --repo "$REPO" --json headRefOid --jq .headRefOid)"
gh api "repos/$REPO/commits/$HEAD/statuses" --paginate \
--jq ".[] | select(.context==\"agentic-review/clean\" and ((.description // \"\") | contains(\"pr=#$PR:\"))) | [(.id|tostring), .state] | @tsv" \
| sort -n | tail -1
The output is <status id>\t<state> — keep both; the id anchors the
reused-SHA rule. Right after a push or reopen that lands an already-seen
SHA, even the newest status can be a PAST occurrence's terminal state (the
current round has not stamped yet), and a matching head: on an old check
comment is equally misleading. Anchor each round on its own stamp, by id:
- About to push: record the newest id first; this round's verdict is the
first terminal state whose id is strictly newer than a
pending that is
itself newer than what you recorded.
- Arrived after the event (reopen or unobserved push): if the newest
state is
pending, that is the current round — record its id and wait for
a newer terminal stamp. If it is terminal, wait ~5 minutes for a newer
pending (the conductor stamps within a couple of minutes of any
developer event): one appearing means that round owns the verdict; none
means the terminal stamp is the standing verdict — or the round is dead,
per the diagnostics below.
- On a brand-new SHA this is automatic — no history to masquerade as.
pending or empty output means the round is still running — poll every
minute or so rather than pushing again. But bound the wait: the pending
stamp lands within a couple of minutes of a push, and a round finishes in
10–20. If no PR-bound stamp appears after ~15 minutes, or pending outlives
~40, the round is likely dead (failed workflow run, missing PAT wiring,
cancelled mid-poll) and polling will not revive it — inspect the runs and
report what you find rather than pushing again:
gh run list --repo "$REPO" --workflow=agentic-conductor.yml --limit 5
gh run list --repo "$REPO" --workflow=agentic-escalation.yml --limit 5
Threads: fixing, rebutting, answering, resolving
List threads and find the unanswered ones (totalCount < 2 means no reply
beyond the finding). Paginate the full connection, exactly as the gate
does — a long PR can carry more than 100 threads, and a work list built from
page one alone cannot satisfy the answer-every-thread rule:
gh api graphql --paginate -f query='query($owner:String!,$name:String!,$pr:Int!,$endCursor:String){
repository(owner:$owner,name:$name){pullRequest(number:$pr){
reviewThreads(first:100,after:$endCursor){
nodes{id isResolved path
comments(first:1){totalCount nodes{databaseId author{login} body}}}
pageInfo{hasNextPage endCursor}}}}}' \
-f owner="${REPO%/*}" -f name="${REPO#*/}" -F pr="$PR"
Reply on a thread (the databaseId of its first comment; pass the body as a
file, never interpolated):
gh api --method POST "repos/$REPO/pulls/$PR/comments/$ROOT_ID/replies" -F body=@reply.md
A good rebuttal argues the technical case — why the finding does not apply,
what invariant already covers it, what trade-off is accepted — on the thread
where the reconcile will adjudicate it. "Won't fix" with no reasoning is
adjudicated too, as rebutted-invalid, and keeps blocking.
After answering a thread, resolve it. Replies still land on resolved threads
(resolution is a collapse flag, not a lock), so nothing you resolve is
closed to the reconcile's later replies:
gh api graphql -f query='mutation($t:ID!){resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -f t="$THREAD_ID"
Never @mention the bot reviewers (copilot, lfx-reviewer) in any
comment — a mention can summon them outside the flow's control.
The needs-human label
needs-human is the human override surface: sticky and add-only. The
escalation judge sets it, and only an allowlisted human's removal counts —
the gate enforces the allowlist, and an unauthorized removal leaves the gate
withheld. It usually means the PR touches approval machinery,
security-sensitive surface, or something the judge could not bound.
The label blocks the gate approval only — not review rounds: the
conductor keeps adjudicating every push and stamping agentic-review/clean
regardless, so your goal is unchanged — keep driving rounds until the check
is green on the current head. Never remove or toggle the label, and do not
treat it as a bug. Report it to the main session the moment it appears so
the human review it requests can start in parallel with your remaining
rounds; at the goal, report the needs-human ending per "Endings".
Launching the PR driver (main session)
This section — and only this section — is addressed to the main session.
The loop is mostly waiting, so the moment a PR enters the flow (right after
gh pr create, or on finding an open PR that needs iteration), launch the
PR driver without waiting to be asked: spawn a general-purpose agent in
the background with worktree isolation and hand it this skill. Tell the user
the driver is on it; the main session stays free for the next piece of work.
Only skip the launch if the user asked to work the loop in this session, and
even then point out the next feature can start in a separate worktree
(git worktree add ../lfx-v2-newsletter-service-<feature> main).
Keep the launch prompt minimal — this document is the driver's operating
manual; do not restate its rules. The prompt needs only:
- The instruction to first load this skill (
newsletter-service-agentic-pr,
via the Skill tool) — falling back, if the skill is unavailable, to
reading this SKILL.md at the main checkout's actual absolute filesystem
path, which the main session substitutes into the prompt (a literal
<main-checkout> placeholder is not executable; never point at the
worktree copy, whose snapshot may be stale) — and drive the PR by it to
its terminal state: a green agentic-review/clean check on the current
head with every thread answered, then report which ending applies.
- The PR number and current head SHA.
- The newest
pr=#<PR>:-bound agentic-review/clean status id as the
pending anchor if one exists (it also seeds the monitor's round
baseline), plus any hazard the main session knows about (e.g. the head
SHA was previously pushed on another PR).
Example prompt (with <main-checkout> already substituted with the main
checkout's absolute path, as the main session must do): "You are the PR
driver for PR #57 on this repo. Load the newsletter-service-agentic-pr
skill — or, if unavailable, read
/home/dev/lfx/lfx-v2-newsletter-service/.claude/skills/newsletter-service-agentic-pr/SKILL.md
— and drive the PR by it to a green check on the current head with every
thread answered, then report which ending applies. Head: <sha>. Pending
anchor: status id <id>. Do not merge."
Main-session follow-up: each time the driver stops, a task notification
arrives. A result that says it is polling means it will self-resume — do not
duplicate its work. If no further notification arrives within ~50 minutes of
one that promised a poll, nudge the driver with SendMessage (its agent id
stays addressable after it stops). The driver sends two different
needs-human messages — treat them differently. An interim note that
the label appeared mid-drive is not the loop ending: the driver keeps
driving to the green check; relay the label to the user so their review can
start in parallel. The final report with the needs-human ending (green
check on the current head, every thread answered, label still set) IS the
loop ending: do not nudge or restart the driver — relay that only a human
review can move the PR forward. If it reports blocked (a design decision,
non-convergence), relay that to the user and do not restart it blindly.
Merging is never the driver's job: a green, gate-approved PR is merged
from the main session only, and only on explicit human instruction.
Driver operations
You are the driver from here on — a goal-based agent. The goal is a green
agentic-review/clean check on the current head with every thread answered;
implement whatever fixes the rounds demand to reach it, within the authority
bounds below. needs-human never pauses your rounds — it only selects which
ending your final report announces (see "Endings").
Worktree discipline. Git refuses to check out a branch that another
worktree already has, and the main checkout may still be on the PR branch —
so work detached. Derive the branch name from the PR itself (HEAD is
detached, so never from the local checkout), then check out and push by it:
BRANCH="$(gh pr view "$PR" --repo "$REPO" --json headRefName --jq .headRefName)"
git fetch origin && git checkout --detach "origin/$BRANCH"
git push origin "HEAD:$BRANCH"
Fix-commit conventions. git commit -s -S (DCO + GPG), conventional
subject (fix(review): ...). New .go files start with the repo's two-line
license header. Run the checks before creating the fix commit, not
after: export PATH="$(go env GOPATH)/bin:$PATH" (the Makefile installs
golangci-lint into GOPATH/bin, which is not always $HOME/go/bin), then
make check (fmt + lint + license-check + go vet) and make test — all
must pass, and make check mutates the tree (it rewrites formatting in
place), so stage whatever it changed before committing. Before pushing,
git status --porcelain must be empty — a dirty tree means the commit does
not match the checked state.
Liveness — one persistent monitor, armed first. A round takes 10–20
minutes and you must survive every wait unattended. Never rely on one-shot
background polls — they die silently, and silence is never success. As your
first act, arm ONE persistent Monitor (persistent: true, 2-minute
interval) as your standing wake source and leave it running for the whole
drive. It fingerprints everything that can change the drive's state — the
head, the newest PR-bound stamp, the needs-human label, the gate's
approval for the current head (approvals are commit-bound: the gate only
honors one whose commit_id is the head, and a previous head's approval can
stay visible until the dismissal run completes, so the fingerprint filters
the same way), the current head's escalation verdict, and the
unanswered-thread count across ALL pages (totalCount < 2, matching the
gate's tidiness check — resolution state is cosmetic and deliberately not
fingerprinted) — and emits an event on any change, plus stall events when
the current round produces no stamp newer than its baseline or pending
outlives its deadline, so it stays a wake source after the clean stamp goes
quiet (including when the authoritative needs-human: no verdict lands
without an approval); each event re-invokes you:
REPO="$(gh repo view --json nameWithOwner --jq .nameWithOwner)"; PR=<pr>
BASE=<anchor>
prev=""; fails=0; last_head=""; last_stamp="init"; max_seen=$BASE; waits=0
while true; do
ok=1
head=$(gh pr view "$PR" --repo "$REPO" --json headRefOid --jq .headRefOid 2>/dev/null) || ok=0
if [ "$ok" -eq 1 ]; then
stamps=$(gh api "repos/$REPO/commits/$head/statuses" --paginate --jq ".[] | select(.context==\"agentic-review/clean\" and ((.description // \"\") | contains(\"pr=#$PR:\"))) | [(.id|tostring), .state] | @tsv" 2>/dev/null) || ok=0
fi
if [ "$ok" -eq 1 ]; then
label=$(gh pr view "$PR" --repo "$REPO" --json labels --jq "[.labels[].name] | contains([\"needs-human\"])" 2>/dev/null) || ok=0
fi
if [ -eq 1 ];
approvals=$(gh api --paginate --jq 2>/dev/null) || ok=0
[ -eq 1 ];
verdicts=$(gh api --paginate --jq 2>/dev/null) || ok=0
[ -eq 1 ];
pages=$(gh api graphql --paginate -f query= -f o= -f n= -F p= --jq 2>/dev/null) || ok=0
[ -ne 1 ];
fails=$((fails+)); [ -ge 5 ] && { ; fails=0; }
120;
fails=0
stamp=$( | -n | -1)
sid=$( | -f1); sid=
approved=$([ -n ] && || )
verdict=$( | grep -o | -1)
unanswered=$( | awk )
[ != ]; last_head=; BASE=; waits=0;
[ -gt ] && max_seen=
[ != ] && { last_stamp=; waits=0; }
[ -le ];
waits=$((waits+)); [ -eq 8 ] &&
*pending*) waits=$((waits+)); [ -eq 20 ] && ;;
fp=
[ != ] && { ; prev=; }
120
It follows the PR's current head across your own pushes. Record pending
stamps newer than the baseline as the round's anchor and treat newer
terminal ones as verdicts. A post-clean fingerprint change — the current
head's approval landing, its escalation verdict arriving, the needs-human
label appearing, an unanswered-thread count change — is the cue to re-check
that surface directly: the fingerprint is a wake trigger, not the
authoritative read (threads come from the paginated listing, the
label/approval from the PR itself). Repeated query failures emit
poll-error events instead of leaving you in permanent silence, and a
round that never produces a stamp newer than its baseline — including the
reused-SHA case where the only visible stamp is an old occurrence's
terminal state — or whose pending outlives its deadline emits a stall
event, so the dead-round diagnostics are reachable even when the
fingerprint never changes. (A stall right after launching onto an
already-settled round is a prompt to apply the arrival rule in "Waiting for
the verdict", not proof the round is dead.) On each
wake-up: handle the new state and send a one-line round-transition note to
the main session (SendMessage to main, e.g. "round 2: 3 blocking;
fixing") so liveness stays visible. End a turn only with a final report, a
must-stop report, or the armed monitor standing watch — never with neither.
Call TaskStop on the monitor before your final report.
Authority bounds.
- May: read PR state, commit and push fixes for findings whose
resolution is clear and mechanical, post thread replies and substantive
rebuttals, resolve the threads you have answered, poll for verdicts.
- Must stop and report instead of acting when: a finding requires a
design decision or its fix is not obviously safe; the same finding family
survives two of your fix attempts; roughly five rounds pass without
convergence; or a round appears dead per the bounded-wait diagnostics
(report the workflow-run evidence, do not push again). The
needs-human
label is NOT a stop condition — report it and keep driving per "The
needs-human label".
- Must never: merge — you have no merge authority under any
circumstances, even fully green and gate-approved — force-push, approve,
edit other accounts' comments, add or remove labels, or @mention the bots.
Final report. State the terminal state — a green check on the current
head with every thread answered — and which ending applies, under exactly
the conditions in "Endings" (with no authoritative signal yet, report
escalation pending rather than clear). If you stopped on a must-stop
condition instead, state what you are blocked on. Either way include a
one-paragraph round-by-round history: what each round's blocking findings
were and whether each was fixed or rebutted.
Hard rules
- Rounds fire on developer events only — push, open, reopen. Comments and
replies never start one; do not wait for a reaction to a comment.
- At most one push per round; batch fixes and rebuttals (signed empty commit
for rebuttal-only rounds, no push for replies-only on a clean head).
- Fix or rebut every blocking finding; answer every thread; say how you
fixed it or why it stands, then resolve what you answered.
- Never mention the bots; never touch the
needs-human label.
- Never edit or imitate
lfx-reviewer comments — the pipeline trusts that
account's authorship, and the apply step validates everything anyway.
