Skip to content

feat(audits): audits.extraPrLabels for every PR the audits open - #124

Merged
allenhutchison merged 5 commits into
mainfrom
claude/project-thread-srw840
Nov 8, 2026
Merged

allenhutchison merged 5 commits into
mainfrom
claude/project-thread-srw840

Conversation

@allenhutchison

@allenhutchison allenhutchison commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Requested by Allen · project thread

Human overview

Audit PRs (architecture, tests, security, deps) only ever get their category label and automated, so a repo can't hand them to foreman's response loop, which follows foreman:watch. pepper#6103 sat for 8 hours with an unanswered Greptile P2 for that reason. This adds an optional audits.extraPrLabels setting (default empty) that every audit applies to the PRs it opens, the same way autoDev.extraPrLabels already works. A pepper config PR follows to set it to ["foreman:watch"].

AI reviewer

  • Schema: audits.extraPrLabels added to the canonical plugins/core/references/config-schema.md; vendored copies regenerated with scripts/sync-references.sh (no hand edits).
  • Skills: audit-architecture (create call, the REST-fallback ordering note, and a missing-label rule: open the PR without it and report, never create the label), audit-tests (the follow-up gh pr edit), audit-security and audit-deps (the label lists). PRs only; issues keep their existing labels, since a watch label on an issue means nothing to foreman.
  • Absent key behaves exactly as today (default []).
  • Version bumps are left to version-bump.yml.

Checklist (repo-specific)

  • Plugin manifests validate (N/A: no manifest changed)
  • Every skill has well-formed frontmatter (frontmatter untouched)
  • ./scripts/test-coverage.sh and ./scripts/test-profile.sh pass locally
  • ./scripts/sync-references.sh --check passes
  • python3 scripts/check-links.py passes

Generated by Claude Code

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; the previous finding is fixed and no new actionable issues were found.

Summary

Adds optional audits.extraPrLabels to PRs opened by the architecture, test, security, and dependency audits.

  • Audit PRs can now carry extra labels for downstream automation.

Reviews (2) · Last reviewed commit: "fix(audits): skip the extra-label check ..." · Reviewed by Greptile

The audits label their PRs with the category label and labels.automated
only, so a repo that hands PRs to a response loop by label (foreman's
foreman:watch) had no way to include audit PRs. pepper#6103 sat 8 hours
with an unanswered Greptile P2 because nothing watched it.

Adds an optional audits.extraPrLabels (default []) to the config schema
and applies it on PRs, never issues, in audit-architecture, audit-tests,
audit-security and audit-deps. Mirrors autoDev.extraPrLabels.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QNDeh8JQ33LnS9i2uXvUNV
@vycari-review

vycari-review Bot commented Nov 8, 2026 •

Copy link
Copy Markdown

Vycari Review: Vycari Review did not publish a review of 2a260ee: it could not be reviewed in time (the daily review budget is spent, so Vycari Review did not run on this commit). See the check run; the next push or sweep retries.
Results below are from round 2 on f1f0fc9.

Vycari Review

Round 2 on f1f0fc9. The verdict is the Vycari Review check run, and each open finding is an inline thread. Reply on every P0 and P1 thread: Fixed in <sha>, or Disputed: with a reason. P2s are the author's call: fix one, or reply Deferred: #<issue> (the PR's one follow-up issue) or Declined: with a reason, and VR stops re-raising it. From round 2 on, defer a new P2 unless its fix is a one-line correction.

Open: P0 0 · P1 1 · P2 3 · disputed 0 · confirmed fixed 3 · withdrawn 0

Hand-off: Not handed to Greptile yet: 1 P0/P1 finding(s) open with no reply (round 2 of 3).

Open P0 and P1 findings

  • P1 · VR-72910e45 · A failed label-list read drops every extra routing label and reports them as nonexistent · plugins/audits/references/extra-pr-labels.md lines 8–13
P2s, judge's notes and closed findings

Open P2 findings

  • P2 · VR-628187a9 · The once-per-run existence check that protects the PR creation is documented only, not invoked by any skill step · plugins/audits/references/extra-pr-labels.md lines 7–14
  • P2 · VR-a3f0dbd2 · Architecture PR create template passes every extraPrLabels entry unchecked, so one missing label fails gh pr create after the branch is pushed · plugins/audits/skills/audit-architecture/SKILL.md lines 200–206
  • P2 · VR-e850eb06 · The fix for the watch-label race rests on an unverified claim about foreman, so the late-label path still loses the PR · plugins/audits/references/extra-pr-labels.md lines 20–25 (deferred by the author)

P2s with no reply yet

  • P2 · VR-628187a9 · The once-per-run existence check that protects the PR creation is documented only, not invoked by any skill step · plugins/audits/references/extra-pr-labels.md lines 7–14

Judge's notes

One P1 is still open: a failed label-list read can drop the watch label and report it as missing, and the fix is narrow (fail closed on that read). The other prior findings are fixed or now only P2 (template/prose inconsistency, and the foreman ordering claim that this repo can't verify).

Confirmed fixed

  • VR-952be2a5 · Test-audit PR can be opened with its routing labels silently missing when one extraPrLabels name is absent
  • VR-d4cbbe4f · Audit PRs opened through create-pr never get audits.extraPrLabels
  • VR-400c4065 · REST label fallback can create the extra labels the audits are told never to create

Seen again (linked to an earlier finding, not reopened)

  • VR-e850eb06 · held · plugins/audits/references/extra-pr-labels.md lines 20–25

@allenhutchison allenhutchison self-assigned this Nov 8, 2026

@vycari-review vycari-review Bot Nov 8, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 · VR-e850eb06 · The fix for the watch-label race rests on an unverified claim about foreman, so the late-label path still loses the PR

On the REST path the watch label is still added just after the PR opens. That is safe only if foreman finds watched PRs by polling, which the author asserts but this repo cannot show.

Reply Fixed in <sha> or Disputed: with a reason.

Details for agents: evidence, failure scenario, verification, proving test

The round-f6b220f finding said that on the non-draft REST path, a foreman:watch label posted after the opened webhook can be missed by the response loop. The fix does not add a draft or ordering step. It rewrites rule 3 to assert, as fact, that foreman 'finds watched PRs by listing open PRs that carry the label on every tick, not from the opened webhook'. Nothing in this repo supports that: the only mention of foreman is config examples and the build-record marker, and foreman lives in another repository (Vycari/foreman). The reference then directs the agent to create the PR non-draft and label it immediately after, and says a label a review bot reads at open time 'belongs in the draft-then-stop branch... this key is the wrong place for it'. The config-schema example for this very key is foreman:watch 'so a response loop answers its review'. So the doc uses an unverified polling claim to classify the example label as routing-only, and the prior race stays open.

Evidence: extra-pr-labels.md:20-25 (the rule 3 polling claim and the 'wrong place' sentence). audit-architecture/SKILL.md:239-248 implements it: non-draft, label immediately after. gh-rest-fallbacks.md:325-337 says the review-gating vs routing distinction is the caller's call and offers the draft branch for labels that must be in place before review starts. I searched the checkout for any foreman polling implementation or design doc and found none, so the claim is unverified here. The config examples (plugins/audits/references/config-schema.md:114) use foreman:watch as the example for this key.

Failure scenario

Audit-architecture runs in a repo where GraphQL is blocked and extraPrLabels is ["foreman:watch"]. It POSTs /pulls with draft:false, the opened webhook fires, and a review or response loop reads labels when it handles opened. The agent then POSTs the foreman:watch label. If foreman handles only the opened event for this PR, the PR is never watched. Nothing re-triggers on the later label event, the report says the label was applied, and the PR sits unanswered.

How this was verified (read-only-reasoning)

Read rule 3 and the audit-architecture ordering text in the diff. The foreman source is not in this checkout, so the claim cannot be checked. Reachable from: audit-architecture REST fallback.

Proving test

Doc-level: a reviewer or foreman maintainer must show that foreman re-lists open PRs by label on each tick (the claim in extra-pr-labels.md:20-22). Behavioural: for a blocked-GraphQL run with extraPrLabels ["foreman:watch"], assert the PR is either opened as draft with the label then stopped for a human, or that the watch label is present at open time. On head the non-draft late-label path is the one the reference prescribes.

lens spec-adversary

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Disputed: foreman does not pick up watched PRs from the opened webhook. reconcile() in foreman/src/foreman/reconcile.py calls gh.list_watched(repo, label) on every tick, listing open PRs that carry the label, so a label added a moment after open is seen on the next tick. foreman:watch is a routing label, not a review gate. In f1f0fc9 the new references/extra-pr-labels.md says so, and says a label a bot reads at open time belongs in the draft-then-stop branch instead.


Generated by Claude Code

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Held @ f1f0fc9 after the dispute: Restated by a finding at this head. Left for Greptile to arbitrate.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seen again @ f1f0fc9: linked to this finding. Also raised at plugins/audits/references/extra-pr-labels.md lines 20–25.

One shared rule in references/extra-pr-labels.md: check the labels once
per run and drop missing ones (gh pr create and a combined gh pr edit
fail outright on an unknown name, and the REST labels call creates it),
apply the rest on every path including create-pr delegation, and why a
watch label landing after the opened webhook is fine for foreman.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QNDeh8JQ33LnS9i2uXvUNV
@vycari-review

vycari-review Bot commented Nov 8, 2026

Copy link
Copy Markdown

@greptileai review

@vycari-review

vycari-review Bot commented Nov 8, 2026

Copy link
Copy Markdown

@greptileai review

@allenhutchison
allenhutchison merged commit cfa1084 into main Nov 8, 2026
3 checks passed
@allenhutchison
allenhutchison deleted the claude/project-thread-srw840 branch November 8, 2026 19:59
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.

2 participants