feat(skills/pr-quality): post-PR coordinator + GitHub Action - #94
Conversation
Closes #88. Adds .claude/skills/pr-quality/ — a procedural coordinator skill that runs on pull_request: opened/synchronize/reopened, dispatches a fixed v1 judge set in parallel, and posts a single anchored PR comment with up to 5 findings ranked by severity. Suggestive only — never gates merge. Silence is the success state on clean PRs. ## v1 scope (locked by coral scope-cut) Dimensions: 2 (verbosity dispatches /brevity; convention adherence with 5 specific memory-encoded rules). Deferred dimensions (docs completeness, reference drift, commit message hygiene) carry un-defer triggers in references/rule-registry.md. Rules: - no_cpu_limits (mechanical YAML scan, awk indent + parent-block aware) - harbor_ecr_convention (mechanical diff-state-machine grep) - narration_comments (LLM-judged, function-doc style only, n=3 SC) - temporary_migration_notes (LLM-judged, durable docs only) - authoritative_voice (LLM-judged, .claude/skills/**/*.md only) - /brevity dispatch on PR body via skill-loaded subagent ## Architecture GitHub Action only (no local-invocable surface in v1). Workflow at .github/workflows/pr-quality.yml with: - Per-PR concurrency group + cancel-in-progress - Fork-PR guard (forks have read-only token; explicit skip rather than pull_request_target footgun) - ANTHROPIC_API_KEY pre-check (env-scoped, no template leak) - Artifact upload of full state for debugging Composition: subagent loads target skill (same pattern as /coral, /council). No skill-to-skill registry invented. ## Comment model Anchored marker `<!-- tide-pr-quality | sha=<SHA> | findings-hash=<H> -->`. - No prior + findings count == 0 → no comment posted (no thumbs-up) - No prior + findings > 0 → create new - Prior + hash matches → no-op (no churn on identical re-runs) - Prior + hash differs → PATCH in place - Prior + findings count == 0 → DELETE prior (clean PR after fixes) ## Cross-review product-engineer + reviewer + product-manager cleared after 2 review cycles. Round 1 HOLDs addressed: - Missing judge scripts (judge-mechanical.sh, judge-llm.sh, judge-skill-dispatch.sh) — written - JSON contamination via undisciplined stdout — every judge writes JSON to stdout only, logs to stderr via log() { ... >&2; } - Fork-PR write-permission reality — workflow guard added - SKILL.md step 6 vs format-spec rendering mismatch — SKILL.md defers to format-spec.md as single source of truth Round 2 HOLDs addressed: - no_cpu_limits awk was structurally broken on stdin/FILENAME — rewrote with proper file-arg + indent + parent-block tracking; PyYAML branch rejected per PE for multi-container miscite + early-return bugs (YAGNI) - Workflow secret-leak via template interpolation — switched to env: + ${ANTHROPIC_API_KEY:-} guard - judge-llm.sh --arg ARG_MAX risk — switched to --rawfile streaming ## Follow-ups (tracked in rule-registry.md un-defer triggers) - Documentation completeness dimension — un-defer on first missing-doc reviewer flag - Reference drift dimension — un-defer on first stale wikilink causing confusion - Commit message hygiene dimension — un-defer on first non-CC commit on main - Acceptance criterion deferred: dry-run on last 10 PRs (do post-merge) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR SummaryLow Risk Overview v1 checks: mechanical Docs: README catalog entry for Reviewed by Cursor Bugbot for commit 3fea141. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (current_file ~ /^clusters\/harbor\// && /ghcr\.io/) print current_file ":" line_num | ||
| next | ||
| } | ||
| /^[- ]/ { line_num++ } |
There was a problem hiding this comment.
Diff line counter inflated by counting removed lines
Medium Severity
The harbor_ecr_convention awk diff-state-machine increments line_num for both removed (-) and context ( ) lines via /^[- ]/. Removed lines don't exist in the new file, so they shouldn't increment the new-file line counter. Any findings appearing after a deletion in the same hunk will report an inflated line number, causing the span field to point at the wrong line.
Reviewed by Cursor Bugbot for commit c77d88e. Configure here.
…greement-wired Pivot per Brandon's "targeted and effective, no bloat" framing. The original v1 shipped a GitHub Actions workflow + 8 bash scripts + claude-code-action invocation + state directory + concurrency group + cost guardrail + self-consistency + anchored marker + hash dedupe + 5-finding cap. The action failed-closed on its own introduction PR because the Claude Code App wasn't installed on the repo — the inverse of the suggestive-only intent. Pivot: - Drop ALL CI infrastructure - Agent-invoked + user-invocable, no claude-code-action - Working-agreement reference in CLAUDE.md + AGENTS.md (same pattern as /brevity layer 2; PR #92) ## Final shape (8 files, 393 lines) .claude/skills/pr-quality/ ├── SKILL.md (88 lines) ├── references/ │ ├── rule-registry.md (87 — locked v1 set + 2 │ │ mechanical specs inline + │ │ deferred mechanisms with │ │ un-defer triggers) │ ├── format-spec.md (41) │ └── judges/ │ ├── narration_comments.md (33) │ ├── temporary_migration_notes.md (38) │ └── authoritative_voice.md (33) ├── scripts/ │ ├── scan-yaml-cpu.sh (39 — stateful awk, indent │ │ + parent-block tracking, │ │ multi-container aware) │ └── scan-harbor-ghcr.sh (34 — unified-diff state │ machine) └── evals/evals.json (happy + halt + pressure) ## Two invocation modes **Pre-PR (agent-fired)**: before `gh pr create`, agent runs /pr-quality against the staged diff + planned body. Findings surface inline for revision. No comment posted. **Post-PR (user-invocable)**: /pr-quality <PR> reads the existing PR via gh, dispatches the v1 judges, posts a fresh comment with findings. No comment on zero findings. ## Composition with /brevity Verbosity dimension dispatches /brevity via subagent-loads-target-skill (same pattern as /coral, /council). The verbosity judge in references/judges/ is intentionally NOT a re-implementation of brevity's rules — pr-quality detects the trigger, brevity owns the standard. PE flagged the coupling risk in scope review; the boundary is preserved. ## Deletions (was CI ceremony) - .github/workflows/pr-quality.yml - 9 scripts (check-optout, check-pr-size, fetch-context, dispatch-judges, aggregate, render-comment, post-or-update, judge-llm, judge-skill-dispatch — plus the now-split judge-mechanical) - scripts/README.md - references/guardrails.md (CI-specific content; what remained fits in SKILL.md guardrails stanza) - references/judge-prompt-template.md (without self-consistency, the structured-output contract collapses to 5 fields — inlined into rule- registry's per-judge schema) - references/judges/no_cpu_limits.md, harbor_ecr_convention.md (mechanical specs are in the script + 1 row in rule-registry) - state/.gitkeep (no state worth persisting; closer to a thin orchestrator than a stateful procedure) ## Working-agreement wiring (same-PR per layer-2 pattern) CLAUDE.md ## Working Agreements gains: - **PR-quality discipline:** Before invoking `gh pr create`, apply /pr-quality to the staged diff + planned body. Findings surface inline for revision. Post-PR: invoke /pr-quality <PR> to post a fresh comment with findings. (Brevity runs during authoring; pr-quality runs on the final diff — they don't chain.) AGENTS.md ## Working Agreement gains a parallel "Pre-PR review" paragraph after the existing "Output discipline" entry. ## Deferred mechanisms (un-defer triggers in rule-registry.md) - Self-consistency (n=3 sampling) → first real false-positive - 5-finding cap + severity rank → first PR producing >7 findings - Anchored marker + hash dedupe → first comment-spam complaint - Cost ceiling per PR → first session budget overrun Coral panel reviewed the trim plan: PE CLEAR, reviewer CLEAR (3 small adjustments incorporated), PM NEW with harder cuts (all incorporated; Brandon explicitly retained pre-PR mode as the customer override). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3fea141. Configure here.
| if (in_harbor && /ghcr\.io/) print current_file ":" line_num | ||
| next | ||
| } | ||
| /^[- ]/ && !/^---/ { line_num++ } |
There was a problem hiding this comment.
Removed diff lines incorrectly increment new-file line counter
High Severity
The character class /^[- ]/ matches both removed lines (-) and context lines ( ), but line_num tracks position in the new file (initialized from the +c side of @@ -a,b +c,d @@). Removed lines exist only in the old file and must not increment the new-file counter. Any hunk containing deletions before a ghcr.io addition will report an inflated line number, off by the count of preceding removed lines. The script's own header comment (lines 10–11) confirms the intent is to count only added and context lines. The pattern needs to match only context lines starting with a space, not the - prefix.
Reviewed by Cursor Bugbot for commit 3fea141. Configure here.
1 participant
Closes #88.
Summary
Two-mode PR convention coordinator. No CI infrastructure. Agent-invoked pre-PR (against the staged diff before
gh pr create; findings surface inline for revision) and user-invocable post-PR (/pr-quality <PR>; posts a fresh comment with findings). Suggestive only — never gates merge. Silence is the success state.Proactive trigger lives in
CLAUDE.md+AGENTS.mdworking-agreement references (same pattern as/brevitylayer 2 in PR #92).This closes Cycle 3 of the 3-cycle goal: #86 brevity → #87 GTM → #88 this.
Pivot story
The original v1 shipped a GitHub Actions workflow + 8 bash scripts +
claude-code-actioninvocation + state directory + concurrency group + cost guardrail + self-consistency + anchored marker + hash dedupe + 5-finding cap. It failed-closed on its own introduction PR because the Claude Code App wasn't installed on the repo — inverting the suggestive-only intent.Pivoted on Brandon's "targeted and effective, no bloat" framing. Coral panel review (PE + reviewer + PM) iterated to a substantially tighter v1:
.github/workflows/pr-quality.ymljudge-mechanical.sh(dispatcher)scan-yaml-cpu.sh+scan-harbor-ghcr.sh(single-purpose each)claude-code-action@v1skip-pr-qualitylabelreferences/guardrails.md(85 lines)references/judge-prompt-template.mdNet: 24 files / 1322 lines → 8 files / 393 lines. Each file earns its keep on a "Claude shouldn't redo this in reasoning" or "single source of truth" basis. Zero overlap with sibling files. Zero CI ceremony.
Final tree
v1 rules (locked, in
rule-registry.md)Mechanical
no_cpu_limits— YAML scan forcpu:set inside anylimits:blockharbor_ecr_convention— diff-line grep forghcr.ioinclusters/harbor/**LLM-judged
narration_comments— function-doc-style only (*.go/*.py/*.ts)temporary_migration_notes— durable docs only (CLAUDE.md,AGENTS.md,README.md,docs/**)authoritative_voice— skill content only (.claude/skills/**/*.md)Skill dispatch
/brevityvia subagent-loads-target-skill. Verbosity judge detects the trigger;/brevityowns the standard. No re-implementation of brevity rules inside pr-quality.Composition with /brevity
Verbosity dimension dispatches
/brevityvia subagent-loads-target-skill (same pattern as/coral,/council). The verbosity judge is intentionally not a re-implementation of brevity's rules — pr-quality detects the trigger that brevity matters here, brevity owns the standard. PE flagged the coupling risk in scope review; the boundary is preserved.Working-agreement wiring (same-PR per layer-2 pattern)
CLAUDE.md## Working Agreementsgains:AGENTS.md## Working Agreementgains a parallel "Pre-PR review" paragraph after the existing "Output discipline" entry.Deferred mechanisms — un-defer triggers documented in
rule-registry.mdCoral cross-review on the trim plan
rule-registry.md+ judge file).Test plan
scan-yaml-cpu.shagainst multi-container fixture (1 violation underlimits:, 1 non-violation underrequests:): correctly emits 1 finding./pr-qualityagainst a sample real PR; verify the 5 judges fire per scope filter, output renders per format-spec, no comment on zero findings./pr-quality 94(this PR) post-merge to validate the post-PR mode end-to-end./pr-qualityinvocation by sub-agents drafting PR bodies (council/coral sessions).Goal completion
Goal closes the moment #94 merges. Standing by.
🤖 Generated with Claude Code