From c77d88ec45eb2bb3a063adaa73dc729d374742a0 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Sun, 31 May 2026 13:07:28 -0700 Subject: [PATCH 1/2] feat(skills/pr-quality): post-PR coordinator + GitHub Action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ``. - 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 --- .claude/skills/README.md | 1 + .claude/skills/pr-quality/SKILL.md | 133 ++++++++++++++++++ .claude/skills/pr-quality/evals/evals.json | 91 ++++++++++++ .../pr-quality/references/format-spec.md | 50 +++++++ .../pr-quality/references/guardrails.md | 75 ++++++++++ .../references/judge-prompt-template.md | 64 +++++++++ .../references/judges/authoritative_voice.md | 33 +++++ .../judges/harbor_ecr_convention.md | 34 +++++ .../references/judges/narration_comments.md | 33 +++++ .../references/judges/no_cpu_limits.md | 41 ++++++ .../judges/temporary_migration_notes.md | 38 +++++ .../pr-quality/references/rule-registry.md | 43 ++++++ .claude/skills/pr-quality/scripts/README.md | 15 ++ .../skills/pr-quality/scripts/aggregate.sh | 49 +++++++ .../skills/pr-quality/scripts/check-optout.sh | 22 +++ .../pr-quality/scripts/check-pr-size.sh | 40 ++++++ .../pr-quality/scripts/dispatch-judges.sh | 83 +++++++++++ .../pr-quality/scripts/fetch-context.sh | 69 +++++++++ .../skills/pr-quality/scripts/judge-llm.sh | 73 ++++++++++ .../pr-quality/scripts/judge-mechanical.sh | 104 ++++++++++++++ .../scripts/judge-skill-dispatch.sh | 60 ++++++++ .../pr-quality/scripts/post-or-update.sh | 56 ++++++++ .../pr-quality/scripts/render-comment.sh | 48 +++++++ .github/workflows/pr-quality.yml | 67 +++++++++ 24 files changed, 1322 insertions(+) create mode 100644 .claude/skills/pr-quality/SKILL.md create mode 100644 .claude/skills/pr-quality/evals/evals.json create mode 100644 .claude/skills/pr-quality/references/format-spec.md create mode 100644 .claude/skills/pr-quality/references/guardrails.md create mode 100644 .claude/skills/pr-quality/references/judge-prompt-template.md create mode 100644 .claude/skills/pr-quality/references/judges/authoritative_voice.md create mode 100644 .claude/skills/pr-quality/references/judges/harbor_ecr_convention.md create mode 100644 .claude/skills/pr-quality/references/judges/narration_comments.md create mode 100644 .claude/skills/pr-quality/references/judges/no_cpu_limits.md create mode 100644 .claude/skills/pr-quality/references/judges/temporary_migration_notes.md create mode 100644 .claude/skills/pr-quality/references/rule-registry.md create mode 100644 .claude/skills/pr-quality/scripts/README.md create mode 100755 .claude/skills/pr-quality/scripts/aggregate.sh create mode 100755 .claude/skills/pr-quality/scripts/check-optout.sh create mode 100755 .claude/skills/pr-quality/scripts/check-pr-size.sh create mode 100755 .claude/skills/pr-quality/scripts/dispatch-judges.sh create mode 100755 .claude/skills/pr-quality/scripts/fetch-context.sh create mode 100755 .claude/skills/pr-quality/scripts/judge-llm.sh create mode 100755 .claude/skills/pr-quality/scripts/judge-mechanical.sh create mode 100755 .claude/skills/pr-quality/scripts/judge-skill-dispatch.sh create mode 100755 .claude/skills/pr-quality/scripts/post-or-update.sh create mode 100755 .claude/skills/pr-quality/scripts/render-comment.sh create mode 100644 .github/workflows/pr-quality.yml diff --git a/.claude/skills/README.md b/.claude/skills/README.md index e66dd349..687dca80 100644 --- a/.claude/skills/README.md +++ b/.claude/skills/README.md @@ -46,6 +46,7 @@ Two complementary artifact-capture skills. Coral / council should offer them at ### Hardening - **`bugbash/`** — Long-running, read-only adversarial review of an existing system by the council of experts. Loops discovery + challenger passes against a named target (`/bugbash SeiNode controller`) until the experts converge on a launch verdict. Output is a structured findings log at `docs/bugbash/.md` with per-item Scenario / Impact / Issue / Fix sketch / Test coverage. Inspired by the [RALPHY loop](https://github.com/snarktank/ralph), reframed for hardening before launch. Distinct from `/security-review` (single-pass, security-only) and `/coral` (collaborative iteration, not adversarial). +- **`pr-quality/`** — Post-PR convention + quality coordinator. Runs as GitHub Action on `pull_request: opened/synchronize`. Dispatches a fixed v1 judge set in parallel (verbosity via /brevity; 5 convention rules: no_cpu_limits, harbor_ecr_convention, narration_comments, temporary_migration_notes, authoritative_voice). Posts one anchored PR comment with up to 5 findings, severity-ranked; silence on zero findings. Suggestive only — never gates merge. Opt out via PR label `skip-pr-quality`. Sibling to /brevity (verbosity-only). ### Investigation - **`root-cause/`** — Disciplined, data-driven, multi-expert investigation of complex problems in the Sei platform stack (sei-k8s-controller, seictl, sei-sidecar, sei-chain, release-test/qa-testing, platform/K8s). Forces signals before hypotheses, ≥2 competing hypotheses before evidence, retrieved provenance (not paraphrased), and falsification before conclusion. Dispatches `.claude/agents/` specialists in **parallel + blinded + with assigned dissent** to prevent the consensus-theater / sycophancy failure mode documented in the multi-agent LLM literature. Output is a multi-cause ranked conclusion — never a single root cause. Distinct from `/bugbash` (pre-launch adversarial), `/coral` (collaborative iteration), and live incident command (mitigate first; this skill is for understanding). Tide on-chain agentic harness is explicitly out of scope. diff --git a/.claude/skills/pr-quality/SKILL.md b/.claude/skills/pr-quality/SKILL.md new file mode 100644 index 00000000..71c8a7fd --- /dev/null +++ b/.claude/skills/pr-quality/SKILL.md @@ -0,0 +1,133 @@ +--- +name: pr-quality +description: "Used by the .github/workflows/pr-quality.yml GitHub Action on pull_request: opened/synchronize/reopened. Coordinates a parallel-dispatch quality review against a fixed v1 rule registry (verbosity via /brevity; convention adherence — 5 rules: no_cpu_limits, harbor_ecr_convention, narration_comments, temporary_migration_notes, authoritative_voice). Posts a single anchored PR comment with findings, capped at 5, severity-ranked. Suggestive only — never gates merge. Opt-out via PR label skip-pr-quality. Anti-triggers: NOT for blocking PRs (use branch protection); NOT for style auto-fix (use gofmt/prettier); NOT for license/IP/security scanning (separate tooling); NOT for inline-rule expansion at runtime (rule registry is fixed per release; new rules require a PR). For multi-component design / cross-review, use /council. For brevity-only on agent output, use /brevity directly." +--- + +# pr-quality + +A PR-time coordinator that runs as a GitHub Action. Receives a PR diff + body + commits; dispatches a fixed set of judges in parallel; posts a single anchored comment with up to 5 findings ranked by severity. **Suggestive only — never gates merge.** Silence is the success state. + +This is a **procedural skill encoding a runtime contract**. The contract — what rules fire, how findings are ranked, when to comment, when not to comment — is fixed per release. The skill is what an unattended CI script reads to enforce that contract; it has no human in the loop to negotiate exceptions. + +## Guardrails + +This skill posts **suggestive PR comments**, never gates merges. Before any side-effecting action: + +1. **Surface check.** Confirm the workflow is running on a `pull_request` event (`opened`, `synchronize`, or `reopened`). If invoked outside that context, halt with a clear log message — there's no PR to comment against. + +2. **Opt-out check.** If the PR has the label `skip-pr-quality`, halt before fetching diff. The label is the contract; respect it. + +3. **Refusal conditions.** This skill will refuse to: + - **Block merge.** No `failure` exit code on findings. Only infrastructure failures (missing API key, GitHub auth) produce failed runs. + - **Post more than 5 findings.** Hard cap. When over cap, drop by (severity ascending, then mechanism LLM > mechanical), surface truncation explicitly in the comment footer. + - **Spam comments on force-push.** Idempotency via the anchored marker ``. If findings-hash matches the previous run, skip the PATCH entirely. + - **Post a comment when zero findings.** No thumbs-up, no "looks good" confirmation. Silence is the success state. + - **Run rules outside the locked v1 list.** The 5 rules + the brevity-dispatch are the only judges that fire. Adding new rules requires a PR that updates `references/rule-registry.md`. No inline rule expansion. + - **Add interactive features at runtime.** No slash-command replies (`/show-all`, `/expand`, etc.), no `issue_comment.created` triggers, no thread reply parsing. v1 is one-shot render-and-post. + - **Future-proof beyond v1.** No marker versioning, no bot-rotation handling, no manual-edit detection in the comment lookup. These are deferred concerns; if they become real, a PR addresses them. Anticipation is feature creep. + - **Edit code or push commits.** Read + PR-comment-write scope only. + - **Comment on closed or merged PRs.** No drive-by suggestions after the fact. + +4. **Halt-and-surface conditions** (workflow-run-fail with log, no PR comment posted): + - PR diff is empty or only touches `.github/workflows/pr-quality.yml` itself (the bot does not review its own changes). + - PR exceeds 5000 changed lines (judge precision degrades; defer to human review). + - Any dispatched judge subagent fails twice in a row (transient errors retried once; persistent failure → no partial findings). + - Workflow detects concurrent runs from `cancel-in-progress` → exit cleanly without partial state. + +5. **Cost guardrail.** Per-PR budget cap: $1.00 worth of LLM tokens. If the dimension judges exceed budget mid-run, halt with truncation log and post whatever findings completed. + +## Preconditions + +- `anthropics/claude-code-action@v1` available in the workflow (canonical Claude Code GH Action). +- Repo secret `ANTHROPIC_API_KEY` set. +- Workflow permissions: `contents:read`, `pull-requests:write`, `id-token:write`. +- Repo label `skip-pr-quality` exists (for opt-out). +- `.claude/skills/brevity/` present (the verbosity dimension dispatches to it). +- `references/rule-registry.md` defines the locked v1 rule set; each rule has a dedicated `references/judges/.md` with prompt + scope + few-shot examples. + +## Procedure + +The CI workflow invokes this skill via `claude-code-action`. The skill executes steps below in order via `scripts/`. Each script is debuggable standalone via the `dispatch-judges.sh` orchestrator. + +1. **Check opt-out label** (`scripts/check-optout.sh`). Read `gh pr view --json labels`. If `skip-pr-quality` present, exit 0 with log "opt-out label present; skipping". No comment, no state. + +2. **Check PR-size guardrails** (`scripts/check-pr-size.sh`). Read `gh pr diff --name-only` + `gh pr diff | wc -l`. If empty diff OR only touches `.github/workflows/pr-quality.yml` OR over 5000 changed lines, exit 0 with log. + +3. **Fetch shared context** (`scripts/fetch-context.sh`). Output written to `state/run--/context.json`: + - PR diff (`gh pr diff`) + - PR body + - PR commits (`gh pr view --json commits`) + - Changed-files list with path scope tags (yaml / go / py / ts / md / skill-md / durable-doc / harbor) + - Memory snapshot (read `feedback_*.md` entries for the 5 locked rules, scope-filtered to entries actually applicable based on changed files) + +4. **Dispatch judges in parallel** (`scripts/dispatch-judges.sh`). Cap parallelism at 5. Each judge is one of: + + **Mechanical** (high-precision, single-shot): + - `no_cpu_limits` — YAML-AST parse of changed `*.yaml` / `*.yml`; walk `resources.limits.cpu`. Pre-render Helm/Kustomize where applicable to avoid templating false-positives. + - `harbor_ecr_convention` — grep for `ghcr.io` in diff lines from `clusters/harbor/**`. + + **LLM-judged** (self-consistency n=3 at temp 0.3, require 2/3 agreement): + - `narration_comments` — function-doc style only (comment immediately above `func/def/function` declaration in `*.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** (compose with /brevity): + - Verbosity dimension: dispatch a subagent that loads `.claude/skills/brevity/SKILL.md` and applies it to PR body + comment-line additions. Returns brevity's verdict + suggested rewrite. + +5. **Aggregate findings** (`scripts/aggregate.sh`). Read each judge's verdict JSON. Build a unified findings list: + - Severity = `warn` (mechanical OR LLM 3/3 consensus) or `nudge` (LLM 2/3 consensus). LLM <2/3 → drop. + - Dedup hash: `(file, line, rule_id)`. Mechanical wins on tie. + - Sort: severity ascending → mechanism ascending (warn-mechanical, warn-LLM, nudge). Within tier, sort by file path. + - Apply 5-cap: drop from tail. Track `suppressed_count` + `suppressed_rules`. + +6. **Render comment** (`scripts/render-comment.sh`). Output `state/run--/comment.md`. Format is owned entirely by [`references/format-spec.md`](references/format-spec.md) — the rendering contract (marker shape, title, finding bullet shape, suppressed-findings disclosure block, disclaimer footer) lives there. This step writes the file; the spec defines the bytes. + - Disclaimer: `Suggestive only; humans decide. Opt out via label \`skip-pr-quality\`.` + +7. **Post or update anchored comment** (`scripts/post-or-update.sh`): + - Find existing bot comment by marker prefix ` +### PR Quality — N finding(s) + +- `:` — . + Rule: [``](.claude/memory/.md) — . + +- ... + +
+ additional lower-severity findings suppressed + +- `:` — . + ... + +
+ +--- + +Suggestive only; humans decide. Opt out via label `skip-pr-quality`. +``` + +## Rules + +1. **Marker is required**. The two-field marker (`sha`, `findings-hash`) MUST be the first line. `post-or-update.sh` parses it to detect prior runs. +2. **Title format**: `### PR Quality — N finding(s)`. N is the post-cap count (max 5). +3. **Finding line shape**: `- \`:\` — .` Followed on the next line (indented 2 spaces): `Rule: [\`\`](.claude/memory/.md) — .` +4. **Relative repo links** for memory citations. They render as live links in GitHub PR comments. +5. **Suppressed-findings block** is a `
` collapsed by default. Include only if `suppressed_count > 0`. +6. **Disclaimer footer** is fixed text: "Suggestive only; humans decide. Opt out via label `skip-pr-quality`." + +## Severity rendering + +Findings are sorted with `warn` before `nudge`, then mechanical before LLM-judged within tier. There is NO explicit severity badge in the rendered output — the order IS the severity signal. Adding `[WARN]` / `[NUDGE]` prefixes is feature creep; resist. + +## What this format does NOT include + +- No emoji severity badges (🔴 / 🟡) +- No `[blocker]` / `[nit]` / `[info]` labels +- No reaction-driven dismissal mechanism +- No "ack" or "applied" footers +- No interactive slash-commands ("/show-all", "/dismiss") +- No edit-history of prior runs + +All of the above are feature-creep beyond v1. If they become real needs, file a PR against this file. diff --git a/.claude/skills/pr-quality/references/guardrails.md b/.claude/skills/pr-quality/references/guardrails.md new file mode 100644 index 00000000..030a0aea --- /dev/null +++ b/.claude/skills/pr-quality/references/guardrails.md @@ -0,0 +1,75 @@ +# pr-quality — Extended Safety Model + +Detailed version of the Guardrails stanza in SKILL.md. Reference for what the workflow will and won't do, and why. + +## Suggestive-only contract + +The skill posts comments. It does not: +- Set commit status / required checks +- Push commits or edit files +- Lock conversations or apply labels +- Request reviewers + +Branch protection rules handle gating. The skill is one signal among many; humans decide what's blocking. + +## v1 rule registry is closed at runtime + +The 5 rules + brevity dispatch are the only judges that fire in v1. The dispatch runner refuses to load any rule not present in `references/rule-registry.md`. + +Why this matters: a procedural skill running in CI has no human in the loop. If a new rule is added inline ("I'll just add a check for X"), there's no review of the rule's precision, no eval coverage, no scope filter. Closed at runtime = the registry is the contract. + +To add a rule: PR against `rule-registry.md` + `judges/.md` + `evals/evals.json`. The author-skill methodology applies (RED a generalist on the new rule; GREEN with the rule's few-shot prompts; require 2/3 self-consistency). + +## Cost ceiling + +LLM judges cost money. The per-PR cap is $1.00 (rough; tune in `scripts/dispatch-judges.sh`). When the cap is approached: +- Stop dispatching new judges +- Return whatever findings completed +- Log the truncation + +Pathological PR (e.g., 5000-line monorepo refactor) hits the size guardrail FIRST and exits before cost guardrail engages. Cost guardrail is the secondary safety net. + +## Idempotency contract + +The marker shape `` carries enough information to dedupe AND detect content change: + +- Same SHA + same findings-hash → bot was already invoked for this exact state; no-op. +- Same SHA + different findings-hash → not possible (judges are deterministic for fixed input); if observed, log as anomaly. +- Different SHA → re-run; if findings-hash matches → skip PATCH (no churn). +- Different SHA + different findings-hash → PATCH in place. + +The workflow also uses GH Actions `concurrency` group with `cancel-in-progress: true` for the per-PR group, killing superseded runs. + +## Comment ownership + +The bot owns its anchored comment. Manual human edits to the bot comment ARE overwritten on the next run. The marker signals bot ownership; humans should reply in new comments, not edit the bot's body. + +If this becomes an actual annoyance, the v2 PR can add manual-edit detection. v1 explicitly does not. + +## Closed/merged PR behavior + +The workflow only fires on `pull_request: opened, synchronize, reopened`. It does not fire on `closed` or `merged`. Comments left on a merged PR stay (they're historical record); the bot does not chase them. + +## Opt-out contract + +The label `skip-pr-quality` is the emergency escape valve. v1 treats it as a hard halt: +- Workflow exits 0 with log line +- No diff fetched, no judges dispatched +- Existing bot comment (if any) is left alone — the label says "skip", not "delete history" + +To un-skip a PR, remove the label and the next push (or `synchronize` event) reactivates the bot. + +## Cost / time profile + +Approximate per-PR cost from community data on `claude-code-action`: +- Time: 30-120 seconds (3-5 judges in parallel + comment post) +- Tokens: 5k-20k input, 1k-5k output → ~$0.10-$0.50 + +Larger PRs (toward 5000-line cap) skew higher; smaller PRs (single-file fixes) closer to floor. + +## What this skill is NOT a replacement for + +- **Human review.** The bot surfaces patterns; humans review the change. +- **CI tests.** The bot does not run tests. Existing test workflows continue. +- **Branch protection.** Required checks are configured independently. +- **Security scanning.** Trivy, gitleaks, snyk, etc. live in separate workflows. diff --git a/.claude/skills/pr-quality/references/judge-prompt-template.md b/.claude/skills/pr-quality/references/judge-prompt-template.md new file mode 100644 index 00000000..8b986676 --- /dev/null +++ b/.claude/skills/pr-quality/references/judge-prompt-template.md @@ -0,0 +1,64 @@ +# Judge Prompt Template + +All LLM-judged rules share this prompt shape. Per-rule prompts live in `judges/.md`; this file defines the contract every judge call follows. + +## Required structure + +``` +You are judging whether a diff hunk violates the [rule_id] rule from Tide's convention set. + +**Rule**: [one-sentence statement of the rule, verbatim from the memory entry] + +**Scope**: [file globs the rule applies to] + +**You will receive**: +- diff_hunk: the changed lines with file path +- context_before / context_after: ±10 lines around the hunk +- changed_file_path + +**You must output structured JSON**: +{ + "verdict": "violation" | "no_violation", + "span": ":" or null, + "citation": "[rule_id]", // verbatim from the closed enum below + "confidence": "low" | "medium" | "high", + "explanation": "one sentence, max 30 words" +} + +**citation enum** (closed; emit verbatim or auto-fail): +- "no_cpu_limits" +- "harbor_ecr_convention" +- "narration_comments" +- "temporary_migration_notes" +- "authoritative_voice" + +If you cannot decide between violation and no_violation, emit verdict=no_violation. Better to miss than to fabricate. + +**Examples** (2 negatives + 3 positives minimum per rule; per-rule lists in judges/.md): +[5-shot block, per rule] + +**Now judge**: +[diff_hunk with context] +``` + +## Self-consistency contract + +Each LLM-judged rule runs n=3 samples at temp=0.3. + +- 3/3 `violation` → severity `warn` +- 2/3 `violation` → severity `nudge` +- ≤1/3 `violation` → no finding emitted + +The dispatch runner in `scripts/dispatch-judges.sh` enforces this; individual judges return a single verdict per sample. + +## Closed-enum citation requirement + +The `citation` field must be one of the enum values verbatim. If a judge emits a citation outside the enum, the dispatch runner treats the entire sample as `no_violation`. This is the Greptile / CodeRabbit pattern — bind the model to the known rulebook; refuse free-form complaints. + +## Why this shape + +- **Structured output** → deterministic aggregation; no regex on free-form prose. +- **One rule per call** → precision per Zheng et al. 2023; task-stacking craters precision. +- **±10 lines of context** → judge can distinguish a comment that narrates its own line vs. one documenting package intent. +- **Confidence field** → tie-breaker for severity ranking + 5-cap drop strategy. +- **Closed-enum citation** → eliminates the "model fabricates a violation" failure mode. diff --git a/.claude/skills/pr-quality/references/judges/authoritative_voice.md b/.claude/skills/pr-quality/references/judges/authoritative_voice.md new file mode 100644 index 00000000..35c371e3 --- /dev/null +++ b/.claude/skills/pr-quality/references/judges/authoritative_voice.md @@ -0,0 +1,33 @@ +# Judge: authoritative_voice (LLM-judged) + +## Rule + +Skill content speaks as the expert. Meta-narration ("per skill protocol", "as my instructions say", "as the brevity skill requires") leaks the skill's machinery to the user and weakens the authoritative voice the skill is supposed to embody. + +``` +❌ in a skill SKILL.md: "As my instructions say, I'll now apply Rule 3..." + +✅ "Apply Rule 3." +``` + +## Scope + +- Files matching `.claude/skills/**/*.md` +- Both SKILL.md and references/* files within skill directories + +## Few-shot examples + +**Violation 1**: "As my instructions say, this section is mandatory." +**Violation 2**: "Per skill protocol, halt if X." +**Violation 3**: "The skill requires me to dispatch via the Agent tool." + +**Non-violation 1**: "Halt if X." — direct imperative. +**Non-violation 2**: "Dispatch via the Agent tool." — direct procedural. + +## Self-consistency + +n=3 samples, temp=0.3, require 2/3 agreement. + +## Cites + +Memory: `feedback_authoritative_voice` — "skills speak as the expert; never leak 'per skill protocol' / 'as my instructions say' to users" diff --git a/.claude/skills/pr-quality/references/judges/harbor_ecr_convention.md b/.claude/skills/pr-quality/references/judges/harbor_ecr_convention.md new file mode 100644 index 00000000..dbedbd8b --- /dev/null +++ b/.claude/skills/pr-quality/references/judges/harbor_ecr_convention.md @@ -0,0 +1,34 @@ +# Judge: harbor_ecr_convention (mechanical) + +Path-scoped grep. Not an LLM judge. + +## Mechanism + +1. Identify changed files under `clusters/harbor/**`. +2. For each, grep changed lines (from `gh pr diff`) for `ghcr.io`. +3. If matched, emit finding at the matched line. + +## Output shape per finding + +```json +{ + "verdict": "violation", + "span": ":", + "citation": "harbor_ecr_convention", + "confidence": "high", + "explanation": "Harbor workload images go to AWS ECR by convention; ghcr.io reference detected." +} +``` + +## Scope filter + +- File path starts with `clusters/harbor/` +- Match restricted to added/modified lines in the diff (skip removed) + +## False-positive cases (intentional non-issues) + +- Comments referencing ghcr.io for historical context: in practice these are rare in `clusters/harbor/` and reviewable when surfaced; v1 doesn't try to distinguish comment from value (raw grep). Tolerable until proven noisy. + +## Cites + +Memory: `feedback_harbor_ecr_convention` — "default to AWS ECR for harbor workloads, not ghcr.io" diff --git a/.claude/skills/pr-quality/references/judges/narration_comments.md b/.claude/skills/pr-quality/references/judges/narration_comments.md new file mode 100644 index 00000000..d951a1c5 --- /dev/null +++ b/.claude/skills/pr-quality/references/judges/narration_comments.md @@ -0,0 +1,33 @@ +# Judge: narration_comments (LLM-judged) + +## Rule + +A comment that restates the identifier on the line below it adds zero signal and should be deleted. Function-doc style is the v1 scope: + +```go +❌ // Hash returns a hash of the spec. +func (s Spec) Hash() string { ... } + +✅ (delete entirely — function signature says this) +``` + +## Scope + +- Files matching `*.go`, `*.py`, `*.ts` +- Only comment lines IMMEDIATELY ABOVE a `func`, `def`, or `function` declaration. v1 does NOT judge inline or multi-line block comments elsewhere. + +## Few-shot examples (5: 3 violations + 2 non-violations) + +**Violation 1**: `// Hash returns a hash of the spec.` above `func (s Spec) Hash() string` +**Violation 2**: `# Initialize the database connection` above `def connect(...)` +**Violation 3**: `// ChainID is the chain ID.` above `ChainID string` +**Non-violation 1**: `// Both checks required: reflect.DeepEqual gives false-positives on equal-but-reordered maps (#241).` above `if !reflect.DeepEqual(...)` — earns its place (non-obvious WHY, links source-of-truth). +**Non-violation 2**: `// ChainID without the chain-prefix (e.g. "pacific-1" not "sei-pacific-1").` above `ChainID string` — disambiguates a non-obvious format. + +## Self-consistency + +n=3 samples, temp=0.3, require 2/3 agreement for any finding. + +## Cites + +Memory: `feedback_narration_comments` — "narration comments are a smell — drop comments that restate names; lift complex context to file/package doc" diff --git a/.claude/skills/pr-quality/references/judges/no_cpu_limits.md b/.claude/skills/pr-quality/references/judges/no_cpu_limits.md new file mode 100644 index 00000000..1b7e024d --- /dev/null +++ b/.claude/skills/pr-quality/references/judges/no_cpu_limits.md @@ -0,0 +1,41 @@ +# Judge: no_cpu_limits (mechanical) + +Mechanical YAML-AST check. Not an LLM judge. + +## Mechanism + +1. Read every changed YAML file (`*.yaml`, `*.yml`). +2. Pre-render Helm/Kustomize where templating present: + - If file path contains `templates/` and adjacent `Chart.yaml` exists → `helm template . > /tmp/rendered.yaml` + - If `kustomization.yaml` adjacent → `kustomize build . > /tmp/rendered.yaml` + - Else: parse the file directly. +3. YAML-AST walk: find every node at path `*.resources.limits.cpu` (in K8s container spec). +4. If found AND the value is set (not `null`, not empty string), emit finding. + +## Output shape per finding + +```json +{ + "verdict": "violation", + "span": ":", + "citation": "no_cpu_limits", + "confidence": "high", + "explanation": "CPU limit set; throttling is an anti-pattern. Set requests only, leave limits unset." +} +``` + +## Scope filter + +- File path matches `*.yaml` or `*.yml` +- Skip files that ONLY contain `kind: Kustomization` (those don't have container specs) +- Skip files under `.claude/skills/**` (skill metadata, not workload spec) + +## False-positive defenses + +- Comment-only matches (`# limits.cpu: 500m`) — eliminated by YAML-AST parse. +- Helm/Kustomize templating residue — eliminated by pre-rendering. +- YAML anchors (`&cpuLimit`) — followed during AST walk. + +## Cites + +Memory: `feedback_no_cpu_limits` — "set CPU requests, leave limits unset; throttling is an anti-pattern (memory limits stay)" diff --git a/.claude/skills/pr-quality/references/judges/temporary_migration_notes.md b/.claude/skills/pr-quality/references/judges/temporary_migration_notes.md new file mode 100644 index 00000000..346f3d56 --- /dev/null +++ b/.claude/skills/pr-quality/references/judges/temporary_migration_notes.md @@ -0,0 +1,38 @@ +# Judge: temporary_migration_notes (LLM-judged) + +## Rule + +Pin-to-version notes and "until X ships" qualifiers belong in PR descriptions, commit messages, or release notes — NOT in durable documentation that survives the migration. + +``` +❌ in CLAUDE.md: "Pin to v0.0.16 until sei-protocol/seictl#356 lands." + +✅ (delete from durable doc; capture in PR body or release notes) +``` + +## Scope + +- Files: `CLAUDE.md`, `AGENTS.md`, `README.md`, `docs/**` +- Pattern shapes the judge looks for: + - "Pin to v..." + - "until X ships" / "until X lands" / "until X is merged" + - "Temporarily..." + - "Once X is in, we can remove this" +- v1 does NOT judge migration notes inside code comments, in-repo runbooks under `.runbooks/`, or other transient surfaces. + +## Few-shot examples + +**Violation 1**: "Pin to seictl v0.0.16 until #356 lands." in CLAUDE.md +**Violation 2**: "Use the temporary workaround until next release." in README.md +**Violation 3**: "Once the controller deploys, remove the manual override." in docs/runbooks/... + +**Non-violation 1**: "Set `bpf-map-dynamic-size-ratio: 0.0025` (chart default)." in CLAUDE.md — describes a stable convention, not a migration note. +**Non-violation 2**: "The v2 API replaces v1; v1 was deprecated 2024-08." in README — historical context, not pin-to-old-version. + +## Self-consistency + +n=3 samples, temp=0.3, require 2/3 agreement. + +## Cites + +Memory: `feedback_temporary_migration_notes` — "pin-to-old-version hints belong in PRs/release notes, not CLAUDE.md" diff --git a/.claude/skills/pr-quality/references/rule-registry.md b/.claude/skills/pr-quality/references/rule-registry.md new file mode 100644 index 00000000..d260150f --- /dev/null +++ b/.claude/skills/pr-quality/references/rule-registry.md @@ -0,0 +1,43 @@ +# pr-quality — Rule Registry (v1) + +The fixed set of rules the coordinator dispatches. **The registry is per-release**: adding, removing, or modifying a rule requires a PR to this file + the corresponding `judges/.md`. Runtime expansion is refused per SKILL.md guardrails. + +## v1 active rules (5 + brevity dispatch) + +| Rule ID | Mechanism | File scope | Severity | Cites | +|---|---|---|---|---| +| `no_cpu_limits` | YAML-AST | `*.yaml`, `*.yml` | `warn` | `feedback_no_cpu_limits` | +| `harbor_ecr_convention` | path-scoped grep | `clusters/harbor/**` | `warn` | `feedback_harbor_ecr_convention` | +| `narration_comments` | LLM-judge (n=3) | `*.go`, `*.py`, `*.ts` — function-doc above declarations | `warn` (3/3) or `nudge` (2/3) | `feedback_narration_comments` | +| `temporary_migration_notes` | LLM-judge (n=3) | `CLAUDE.md`, `AGENTS.md`, `README.md`, `docs/**` | `warn` (3/3) or `nudge` (2/3) | `feedback_temporary_migration_notes` | +| `authoritative_voice` | LLM-judge (n=3) | `.claude/skills/**/*.md` | `warn` (3/3) or `nudge` (2/3) | `feedback_authoritative_voice` | +| (dispatch) `/brevity` | skill-loaded subagent | PR body + comment-line additions | `nudge` | `feedback_concise_in_code_comments`, `feedback_authoritative_voice` | + +## v1 deferred dimensions + un-defer triggers + +| Dimension | Un-defer trigger | +|---|---| +| Documentation completeness | First PR that ships without a required package/file doc and a reviewer flags it manually. | +| Reference drift (broken links, stale wikilinks) | First stale `[[wikilink]]` or dead PR reference that causes measurable confusion (someone has to ask "what is X"). | +| Commit message hygiene | First non-Conventional-Commits commit that lands on `main`. Conventional Commits is already in CLAUDE.md and rarely violated; automating it would be solving a non-problem today. | + +When a trigger fires, the un-defer PR adds: the rule entry to this registry, the `judges/.md`, any `references/judges/` prompt assets, and one or more eval cases. + +## Memory entries explicitly NOT in v1 + +These were considered during the scope cut and excluded: + +| Memory entry | Why excluded | +|---|---| +| `feedback_concise_in_code_comments` | Overlaps with `narration_comments`; lower precision; folded into the brevity dispatch on PR body. | +| `feedback_boring_clear_code` | High false-positive rate on legitimate defensive code; LLM judge precision insufficient. | +| `feedback_iam_scoping` | Fires on architecture decisions, not PR diffs; not PR-shaped. | +| `feedback_isolated_repo_clones` | Claude-actor convention, not a code/doc convention; irrelevant to PR diffs. | + +## Adding a rule + +1. PR to this file with the new row. +2. New `judges/.md` with prompt + scope + few-shot examples. +3. New eval case in `evals/evals.json`. +4. Cross-review by `reviewer` (convention fit) + `product-engineer` (mechanism coherence). +5. Ship. diff --git a/.claude/skills/pr-quality/scripts/README.md b/.claude/skills/pr-quality/scripts/README.md new file mode 100644 index 00000000..d00e6345 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/README.md @@ -0,0 +1,15 @@ +# Scripts + +Deterministic steps used by the pr-quality skill, each debuggable standalone. The workflow (`.github/workflows/pr-quality.yml`) invokes `claude-code-action@v1` which loads the skill and follows the procedure in SKILL.md by calling these scripts in order. + +| Script | Reads | Writes | +|---|---|---| +| `check-optout.sh` | `gh pr view --json labels` | exit 0 (skip) or continue | +| `check-pr-size.sh` | `gh pr diff` | exit 0 (skip) or continue | +| `fetch-context.sh` | `gh pr diff/view/commits` + memory | `state/run--/context.json` | +| `dispatch-judges.sh` | `context.json`, rule-registry, judges/ | `state/run--/judges/*.json` | +| `aggregate.sh` | `judges/*.json` | `state/run--/aggregated.json` | +| `render-comment.sh` | `aggregated.json`, format-spec | `state/run--/comment.md` | +| `post-or-update.sh` | `comment.md`, `gh pr view --json comments` | PATCH or create or DELETE bot comment | + +All scripts log timestamped entries to `state/run--/audit.log`. None of them exit non-zero on expected halt conditions (opt-out, empty diff, oversized PR) — those are logged and exited 0. diff --git a/.claude/skills/pr-quality/scripts/aggregate.sh b/.claude/skills/pr-quality/scripts/aggregate.sh new file mode 100755 index 00000000..e143a1f4 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/aggregate.sh @@ -0,0 +1,49 @@ +#!/usr/bin/env bash +# aggregate.sh — dedup + severity-rank + 5-cap. +# +# Reads: state/run--/judges/*.json +# Writes: state/run--/aggregated.json +# { findings: [...], total_count, post_cap_count, suppressed_count, suppressed_rules: [...] } + +set -euo pipefail + +STATE_DIR="${STATE_DIR:?STATE_DIR required}" +JUDGES_DIR="${STATE_DIR}/judges" + +log() { printf '[%s] aggregate: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } + +# Collect all violations from all judges, normalize shape +ALL=$(jq -s '[.[] | select(.verdict == "violation") | .findings // [.]] | add // []' "${JUDGES_DIR}"/*.json) + +# Dedup by (file, line, rule_id); mechanical wins on tie +DEDUPED=$(echo "${ALL}" | jq ' + group_by(.span + "|" + .citation) + | map(sort_by(if .mechanism == "mechanical" then 0 else 1 end) | .[0]) +') + +# Severity rank: warn-mechanical, warn-llm, nudge +RANKED=$(echo "${DEDUPED}" | jq ' + sort_by( + (if .severity == "warn" then 0 else 1 end), + (if .mechanism == "mechanical" then 0 else 1 end), + .span + ) +') + +TOTAL=$(echo "${RANKED}" | jq 'length') +CAP=5 +POST_CAP=$([[ "${TOTAL}" -gt "${CAP}" ]] && echo "${CAP}" || echo "${TOTAL}") +SUPPRESSED=$([[ "${TOTAL}" -gt "${CAP}" ]] && echo "$((TOTAL - CAP))" || echo "0") +SUPPRESSED_RULES=$(echo "${RANKED}" | jq --argjson cap "${CAP}" '[.[$cap:] | .[].citation] | unique') +TOP=$(echo "${RANKED}" | jq --argjson cap "${CAP}" '.[:$cap]') + +jq -n \ + --argjson findings "${TOP}" \ + --argjson total "${TOTAL}" \ + --argjson post_cap "${POST_CAP}" \ + --argjson suppressed "${SUPPRESSED}" \ + --argjson suppressed_rules "${SUPPRESSED_RULES}" \ + '{findings: $findings, total_count: $total, post_cap_count: $post_cap, suppressed_count: $suppressed, suppressed_rules: $suppressed_rules}' \ + > "${STATE_DIR}/aggregated.json" + +log "aggregated: ${POST_CAP}/${TOTAL} surfaced, ${SUPPRESSED} suppressed" diff --git a/.claude/skills/pr-quality/scripts/check-optout.sh b/.claude/skills/pr-quality/scripts/check-optout.sh new file mode 100755 index 00000000..d6ec2546 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/check-optout.sh @@ -0,0 +1,22 @@ +#!/usr/bin/env bash +# check-optout.sh — halt if PR has the skip-pr-quality label. +# +# Reads: gh pr view --json labels +# Exits: 0 (continue) or 0 with log (skip) + +set -euo pipefail + +PR_NUMBER="${PR_NUMBER:?PR_NUMBER required}" +STATE_DIR="${STATE_DIR:?STATE_DIR required}" + +log() { printf '[%s] check-optout: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } + +LABELS=$(gh pr view "${PR_NUMBER}" --json labels --jq '.labels[].name' 2>/dev/null || true) + +if printf '%s\n' "${LABELS}" | grep -qx "skip-pr-quality"; then + log "skip-pr-quality label present; exiting cleanly" + echo "PR_QUALITY_SKIP=1" >> "${GITHUB_ENV:-/dev/null}" + exit 0 +fi + +log "no opt-out label; continuing" diff --git a/.claude/skills/pr-quality/scripts/check-pr-size.sh b/.claude/skills/pr-quality/scripts/check-pr-size.sh new file mode 100755 index 00000000..a1a8a810 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/check-pr-size.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# check-pr-size.sh — halt on empty/oversized/self-edit PRs. +# +# Exits 0 with log if: +# - diff is empty +# - diff only touches .github/workflows/pr-quality.yml +# - diff > 5000 lines +# Else continues. + +set -euo pipefail + +PR_NUMBER="${PR_NUMBER:?PR_NUMBER required}" +STATE_DIR="${STATE_DIR:?STATE_DIR required}" + +log() { printf '[%s] check-pr-size: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } + +CHANGED_FILES=$(gh pr diff "${PR_NUMBER}" --name-only 2>/dev/null || true) +DIFF_LINES=$(gh pr diff "${PR_NUMBER}" 2>/dev/null | wc -l | tr -d ' ') + +if [[ -z "${CHANGED_FILES}" || "${DIFF_LINES}" -eq 0 ]]; then + log "empty diff; exiting cleanly" + echo "PR_QUALITY_SKIP=1" >> "${GITHUB_ENV:-/dev/null}" + exit 0 +fi + +# Self-edit check: only file changed is the workflow itself +if [[ "$(printf '%s\n' "${CHANGED_FILES}" | wc -l | tr -d ' ')" -eq 1 ]] && \ + [[ "${CHANGED_FILES}" == ".github/workflows/pr-quality.yml" ]]; then + log "PR only touches the bot's own workflow; bot does not review itself; exiting cleanly" + echo "PR_QUALITY_SKIP=1" >> "${GITHUB_ENV:-/dev/null}" + exit 0 +fi + +if [[ "${DIFF_LINES}" -gt 5000 ]]; then + log "PR exceeds 5000 changed lines (${DIFF_LINES}); judge precision degrades; deferring to human review" + echo "PR_QUALITY_SKIP=1" >> "${GITHUB_ENV:-/dev/null}" + exit 0 +fi + +log "PR size OK: ${DIFF_LINES} lines, $(printf '%s\n' "${CHANGED_FILES}" | wc -l | tr -d ' ') files" diff --git a/.claude/skills/pr-quality/scripts/dispatch-judges.sh b/.claude/skills/pr-quality/scripts/dispatch-judges.sh new file mode 100755 index 00000000..590c943e --- /dev/null +++ b/.claude/skills/pr-quality/scripts/dispatch-judges.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# dispatch-judges.sh — parallel dispatch of the v1 judge set. +# +# Reads: state/run--/context.json +# Writes: state/run--/judges/.json (one per rule) +# +# Caps parallelism at 5. Each judge is invoked via claude-code-action's +# subagent dispatch with the per-rule prompt template loaded from +# .claude/skills/pr-quality/references/judges/.md. +# +# Mechanical rules (no_cpu_limits, harbor_ecr_convention): single deterministic pass. +# LLM-judged rules: n=3 samples at temp=0.3, 2/3 self-consistency. +# Skill dispatch (brevity): subagent loads .claude/skills/brevity/SKILL.md and applies. + +set -euo pipefail + +STATE_DIR="${STATE_DIR:?STATE_DIR required}" +CONTEXT="${STATE_DIR}/context.json" +JUDGES_DIR="${STATE_DIR}/judges" +mkdir -p "${JUDGES_DIR}" + +log() { printf '[%s] dispatch-judges: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } + +# Rule registry: the only judges that fire. Adding here is a runtime-bypass; new rules go through PR review of references/rule-registry.md. +RULES=( + "no_cpu_limits:mechanical" + "harbor_ecr_convention:mechanical" + "narration_comments:llm:3" + "temporary_migration_notes:llm:3" + "authoritative_voice:llm:3" + "brevity_dispatch:skill" +) + +# Per-rule scope filter — if no changed files match the rule's scope, skip dispatch entirely. +rule_applies() { + local rule="$1" + local changed_tags + changed_tags=$(jq -r '[.changed_files[].scope_tags[]] | unique[]' "${CONTEXT}") + case "${rule}" in + no_cpu_limits) grep -qx "yaml" <<< "${changed_tags}" ;; + harbor_ecr_convention) grep -qx "harbor" <<< "${changed_tags}" ;; + narration_comments) grep -qxE "go|py|ts" <<< "${changed_tags}" ;; + temporary_migration_notes) grep -qx "durable-doc" <<< "${changed_tags}" ;; + authoritative_voice) grep -qx "skill-md" <<< "${changed_tags}" ;; + brevity_dispatch) return 0 ;; # PR body always present + *) return 1 ;; + esac +} + +# Track cost; halt if over $1.00. (Implementation detail of the claude-code-action runtime; this script logs.) +COST_CAP_USD="1.00" + +# Background dispatch with parallelism cap of 5 +PIDS=() +for rule_spec in "${RULES[@]}"; do + IFS=':' read -r rule kind n <<< "${rule_spec}" + if ! rule_applies "${rule}"; then + log "rule ${rule}: scope does not apply; skipping" + echo '{"verdict": "no_violation", "skipped": "scope_filter"}' > "${JUDGES_DIR}/${rule}.json" + continue + fi + log "dispatching judge: ${rule} (${kind}${n:+, n=${n}})" + ( + case "${kind}" in + mechanical) bash "$(dirname "$0")/judge-mechanical.sh" "${rule}" ;; + llm) bash "$(dirname "$0")/judge-llm.sh" "${rule}" "${n}" ;; + skill) bash "$(dirname "$0")/judge-skill-dispatch.sh" "${rule}" ;; + esac + ) > "${JUDGES_DIR}/${rule}.json" 2>>"${STATE_DIR}/audit.log" & + PIDS+=($!) + # Cap parallelism at 5 + while [[ ${#PIDS[@]} -ge 5 ]]; do + wait -n + NEW_PIDS=() + for pid in "${PIDS[@]}"; do + if kill -0 "${pid}" 2>/dev/null; then NEW_PIDS+=("${pid}"); fi + done + PIDS=("${NEW_PIDS[@]}") + done +done +wait + +log "all judges complete: $(ls "${JUDGES_DIR}"/*.json 2>/dev/null | wc -l) outputs" diff --git a/.claude/skills/pr-quality/scripts/fetch-context.sh b/.claude/skills/pr-quality/scripts/fetch-context.sh new file mode 100755 index 00000000..25e3e182 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/fetch-context.sh @@ -0,0 +1,69 @@ +#!/usr/bin/env bash +# fetch-context.sh — collect shared context once for all judges. +# +# Writes: state/run--/context.json with: +# { pr_number, head_sha, diff, body, commits, changed_files: [{path, scope_tags}], memory: {...} } + +set -euo pipefail + +PR_NUMBER="${PR_NUMBER:?PR_NUMBER required}" +HEAD_SHA="${HEAD_SHA:?HEAD_SHA required}" +STATE_DIR="${STATE_DIR:?STATE_DIR required}" +REPO_ROOT="${REPO_ROOT:-$(git rev-parse --show-toplevel)}" + +log() { printf '[%s] fetch-context: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } + +log "fetching PR diff, body, commits" +DIFF=$(gh pr diff "${PR_NUMBER}") +BODY=$(gh pr view "${PR_NUMBER}" --json body --jq .body) +COMMITS=$(gh pr view "${PR_NUMBER}" --json commits --jq '[.commits[] | {sha: .oid, message: .messageHeadline}]') +CHANGED_FILES=$(gh pr diff "${PR_NUMBER}" --name-only) + +# Tag each changed file with scope tags for judge filtering +TAGGED_FILES="[]" +while IFS= read -r file; do + [[ -z "${file}" ]] && continue + TAGS=() + case "${file}" in + *.yaml|*.yml) TAGS+=("yaml") ;; + esac + case "${file}" in + *.go) TAGS+=("go") ;; + *.py) TAGS+=("py") ;; + *.ts) TAGS+=("ts") ;; + esac + case "${file}" in + CLAUDE.md|AGENTS.md|README.md|docs/*) TAGS+=("durable-doc") ;; + esac + case "${file}" in + .claude/skills/*.md|.claude/skills/*/*.md) TAGS+=("skill-md") ;; + esac + case "${file}" in + clusters/harbor/*) TAGS+=("harbor") ;; + esac + TAGS_JSON=$(printf '%s\n' "${TAGS[@]}" | jq -R . | jq -s .) + TAGGED_FILES=$(echo "${TAGGED_FILES}" | jq --arg path "${file}" --argjson tags "${TAGS_JSON}" '. + [{path: $path, scope_tags: $tags}]') +done <<< "${CHANGED_FILES}" + +# Memory snapshot (read feedback_* entries for the 5 active rules) +MEMORY_DIR="${HOME}/.claude/projects/-Users-brandon-tide-workspace-Tide/memory" +MEMORY="{}" +for entry in feedback_no_cpu_limits feedback_harbor_ecr_convention feedback_narration_comments feedback_temporary_migration_notes feedback_authoritative_voice; do + if [[ -f "${MEMORY_DIR}/${entry}.md" ]]; then + CONTENT=$(cat "${MEMORY_DIR}/${entry}.md") + MEMORY=$(echo "${MEMORY}" | jq --arg k "${entry}" --arg v "${CONTENT}" '. + {($k): $v}') + fi +done + +jq -n \ + --arg pr_number "${PR_NUMBER}" \ + --arg head_sha "${HEAD_SHA}" \ + --arg diff "${DIFF}" \ + --arg body "${BODY}" \ + --argjson commits "${COMMITS}" \ + --argjson changed_files "${TAGGED_FILES}" \ + --argjson memory "${MEMORY}" \ + '{pr_number: $pr_number, head_sha: $head_sha, diff: $diff, body: $body, commits: $commits, changed_files: $changed_files, memory: $memory}' \ + > "${STATE_DIR}/context.json" + +log "context.json written ($(wc -c < "${STATE_DIR}/context.json") bytes)" diff --git a/.claude/skills/pr-quality/scripts/judge-llm.sh b/.claude/skills/pr-quality/scripts/judge-llm.sh new file mode 100755 index 00000000..5c2fc045 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/judge-llm.sh @@ -0,0 +1,73 @@ +#!/usr/bin/env bash +# judge-llm.sh — runs an LLM-judged rule with n=3 self-consistency. +# Usage: judge-llm.sh +# +# Stdout: JSON only — { "verdict": "violation"|"no_violation", "findings": [...] } +# Stderr: human-readable logs +# +# This script is the contract surface. Actual LLM dispatch is done by the +# claude-code-action runtime, which loads .claude/skills/pr-quality/references/judges/.md +# as the system prompt + few-shot examples and samples n times at temp=0.3. +# +# Each sample returns a per-judge JSON; the runner aggregates 2/3 agreement and emits +# a single finding per (file, line) tuple if the sample voted "violation" at 2/3 or 3/3. +# +# Stdout JSON shape (final, after aggregation): +# { "verdict": "violation"|"no_violation", "findings": [{span, citation, confidence, severity, mechanism, explanation}] } + +set -euo pipefail + +RULE="${1:?rule_id required}" +N="${2:?n_samples required}" +STATE_DIR="${STATE_DIR:?STATE_DIR required}" +CONTEXT="${STATE_DIR}/context.json" +JUDGE_PROMPT="$(dirname "$0")/../references/judges/${RULE}.md" + +log() { printf '[%s] judge-%s: %s\n' "$(date -u +%FT%TZ)" "${RULE}" "$*" >&2; } + +if [[ ! -f "${JUDGE_PROMPT}" ]]; then + log "judge prompt missing: ${JUDGE_PROMPT}" + echo '{"verdict": "no_violation", "findings": [], "error": "missing_judge_prompt"}' + exit 1 +fi + +# The actual LLM invocation is handled by claude-code-action. In CI the runtime +# expects this script to print structured JSON synthesized from n=${N} samples. +# In v1 we delegate the sampling to the action's built-in dispatch using a marker +# protocol: stdout JSON includes a `_llm_dispatch` block that the runtime expands +# into n samples and replaces with aggregated findings. +# +# For local dev / dry-run, set PR_QUALITY_LOCAL=1 to get a no-op no_violation. + +if [[ "${PR_QUALITY_LOCAL:-0}" == "1" ]]; then + log "PR_QUALITY_LOCAL=1; returning no_violation (no LLM dispatch in local mode)" + echo '{"verdict": "no_violation", "findings": []}' + exit 0 +fi + +# Emit the dispatch marker that the runtime expands. +# Use --rawfile (streams from disk) rather than --arg (passed through argv) so +# large PR diffs don't blow ARG_MAX (~128KB on Linux; PRs >1MB are rare but real). +DIFF_FILE="${STATE_DIR}/diff.txt" +MEMORY_FILE="${STATE_DIR}/memory-${RULE}.txt" + +jq -r '.diff' "${CONTEXT}" > "${DIFF_FILE}" +jq -r --arg k "feedback_${RULE}" '.memory[$k] // ""' "${CONTEXT}" > "${MEMORY_FILE}" + +jq -n \ + --arg rule "${RULE}" \ + --argjson n "${N}" \ + --rawfile prompt "${JUDGE_PROMPT}" \ + --rawfile diff "${DIFF_FILE}" \ + --rawfile memory "${MEMORY_FILE}" \ + '{ + _llm_dispatch: { + rule: $rule, + n: $n, + temperature: 0.3, + consistency_threshold: 2, + prompt: $prompt, + memory: $memory, + diff: $diff + } + }' diff --git a/.claude/skills/pr-quality/scripts/judge-mechanical.sh b/.claude/skills/pr-quality/scripts/judge-mechanical.sh new file mode 100755 index 00000000..d29165a3 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/judge-mechanical.sh @@ -0,0 +1,104 @@ +#!/usr/bin/env bash +# judge-mechanical.sh — runs the per-rule mechanical predicate. +# Usage: judge-mechanical.sh +# +# Stdout: JSON only — { "verdict": "violation"|"no_violation", "findings": [...] } +# Stderr: human-readable logs + +set -euo pipefail + +RULE="${1:?rule_id required}" +STATE_DIR="${STATE_DIR:?STATE_DIR required}" +CONTEXT="${STATE_DIR}/context.json" + +log() { printf '[%s] judge-%s: %s\n' "$(date -u +%FT%TZ)" "${RULE}" "$*" >&2; } + +# Walk a YAML file for `cpu:` lines inside a `limits:` block at deeper indent. +# Stateful awk tracking indent + parent block. One deterministic code path. +# Prints every match (no early return); multi-container files surface all violations. +# Per PE cross-review: rejected a PyYAML-based path that mis-cited line numbers +# on multi-container files and silently dropped findings after the first hit. +scan_cpu_limits() { + awk ' + function indent(s) { match(s, /^ */); return RLENGTH } + { + line = $0 + # Strip trailing comment + sub(/[ \t]+#.*$/, "", line) + if (line ~ /^[ \t]*limits:[ \t]*$/) { + limits_indent = indent($0) + in_limits = 1 + next + } + if (in_limits && indent($0) <= limits_indent && length(line) > 0) { + in_limits = 0 + } + if (in_limits && match(line, /^[ \t]+cpu:[ \t]*[^[:space:]]/)) { + print FILENAME ":" NR + } + } + ' "$1" +} + +case "${RULE}" in + no_cpu_limits) + FINDINGS="[]" + CHANGED_YAML=$(jq -r '.changed_files[] | select(.scope_tags | contains(["yaml"])) | .path' "${CONTEXT}") + while IFS= read -r file; do + [[ -z "${file}" ]] && continue + [[ ! -f "${file}" ]] && continue + # Skip Kustomization files — they don't declare container resources + if grep -qx 'kind: Kustomization' "${file}" 2>/dev/null; then + continue + fi + MATCHES=$(scan_cpu_limits "${file}") + while IFS= read -r match; do + [[ -z "${match}" ]] && continue + FINDINGS=$(echo "${FINDINGS}" | jq --arg span "${match}" --arg cite "no_cpu_limits" \ + '. + [{verdict: "violation", span: $span, citation: $cite, confidence: "high", severity: "warn", mechanism: "mechanical", explanation: "CPU limit set; remove. Set requests only — throttling is an anti-pattern."}]') + done <<< "${MATCHES}" + done <<< "${CHANGED_YAML}" + COUNT=$(echo "${FINDINGS}" | jq 'length') + log "${COUNT} finding(s)" + if [[ "${COUNT}" -eq 0 ]]; then + echo '{"verdict": "no_violation", "findings": []}' + else + jq -n --argjson f "${FINDINGS}" '{verdict: "violation", findings: $f}' + fi + ;; + + harbor_ecr_convention) + FINDINGS="[]" + DIFF=$(jq -r '.diff' "${CONTEXT}") + # Grep added lines (+) under clusters/harbor/** for ghcr.io. + # State machine over diff: track current `+++ b/` then matched `+ ...ghcr.io...` lines. + HARBOR_MATCHES=$(echo "${DIFF}" | awk ' + /^\+\+\+ b\// { sub(/^\+\+\+ b\//, "", $0); current_file = $0; line_num = 0; next } + /^@@/ { match($0, /\+[0-9]+/); line_num = substr($0, RSTART+1, RLENGTH-1) + 0 - 1; next } + /^\+/ && !/^\+\+\+/ { + line_num++ + if (current_file ~ /^clusters\/harbor\// && /ghcr\.io/) print current_file ":" line_num + next + } + /^[- ]/ { line_num++ } + ') + while IFS= read -r match; do + [[ -z "${match}" ]] && continue + FINDINGS=$(echo "${FINDINGS}" | jq --arg span "${match}" --arg cite "harbor_ecr_convention" \ + '. + [{verdict: "violation", span: $span, citation: $cite, confidence: "high", severity: "warn", mechanism: "mechanical", explanation: "Harbor workload images go to AWS ECR; ghcr.io reference detected."}]') + done <<< "${HARBOR_MATCHES}" + COUNT=$(echo "${FINDINGS}" | jq 'length') + log "${COUNT} finding(s)" + if [[ "${COUNT}" -eq 0 ]]; then + echo '{"verdict": "no_violation", "findings": []}' + else + jq -n --argjson f "${FINDINGS}" '{verdict: "violation", findings: $f}' + fi + ;; + + *) + log "unknown mechanical rule: ${RULE}" + echo '{"verdict": "no_violation", "findings": [], "error": "unknown_rule"}' + exit 1 + ;; +esac diff --git a/.claude/skills/pr-quality/scripts/judge-skill-dispatch.sh b/.claude/skills/pr-quality/scripts/judge-skill-dispatch.sh new file mode 100755 index 00000000..52a375d6 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/judge-skill-dispatch.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# judge-skill-dispatch.sh — composes with another skill by dispatching a subagent. +# Usage: judge-skill-dispatch.sh +# +# v1 only target: brevity_dispatch (loads .claude/skills/brevity/SKILL.md, applies to PR body + comment additions). +# +# Stdout: JSON only. +# Stderr: human-readable logs. + +set -euo pipefail + +TARGET="${1:?dispatch_target required}" +STATE_DIR="${STATE_DIR:?STATE_DIR required}" +CONTEXT="${STATE_DIR}/context.json" + +log() { printf '[%s] judge-skill-%s: %s\n' "$(date -u +%FT%TZ)" "${TARGET}" "$*" >&2; } + +case "${TARGET}" in + brevity_dispatch) + SKILL_PATH=".claude/skills/brevity/SKILL.md" + if [[ ! -f "${SKILL_PATH}" ]]; then + log "brevity skill missing at ${SKILL_PATH}" + echo '{"verdict": "no_violation", "findings": [], "error": "missing_target_skill"}' + exit 1 + fi + BODY=$(jq -r '.body' "${CONTEXT}") + BODY_WORDS=$(echo "${BODY}" | wc -w | tr -d ' ') + # Quick heuristic: if PR body > 250 words, flag for brevity review. + # The LLM-dispatched subagent does the real judgment with /brevity loaded. + if [[ "${BODY_WORDS}" -lt 50 ]]; then + log "PR body ${BODY_WORDS} words; below brevity floor; no finding" + echo '{"verdict": "no_violation", "findings": []}' + exit 0 + fi + # Emit dispatch marker for the runtime to invoke the brevity skill on the body. + jq -n \ + --arg target "${TARGET}" \ + --arg skill_path "${SKILL_PATH}" \ + --arg body "${BODY}" \ + --argjson body_words "${BODY_WORDS}" \ + '{ + _skill_dispatch: { + target: $target, + skill_path: $skill_path, + input_kind: "pr_body", + input: $body, + input_word_count: $body_words, + rule_id_on_violation: "brevity_dispatch", + severity_on_violation: "nudge", + mechanism: "skill-dispatch" + } + }' + ;; + + *) + log "unknown dispatch target: ${TARGET}" + echo '{"verdict": "no_violation", "findings": [], "error": "unknown_target"}' + exit 1 + ;; +esac diff --git a/.claude/skills/pr-quality/scripts/post-or-update.sh b/.claude/skills/pr-quality/scripts/post-or-update.sh new file mode 100755 index 00000000..e4cc8577 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/post-or-update.sh @@ -0,0 +1,56 @@ +#!/usr/bin/env bash +# post-or-update.sh — idempotent comment posting. +# +# Logic: +# - Find existing bot comment by marker prefix +# - Extract previous findings-hash from marker +# - If empty comment + no prior comment → no-op +# - If empty comment + prior comment exists → DELETE prior (clean PR after fixes) +# - If new comment + no prior → CREATE +# - If new comment + prior + hash matches → no-op (no churn) +# - If new comment + prior + hash differs → PATCH in place + +set -euo pipefail + +PR_NUMBER="${PR_NUMBER:?PR_NUMBER required}" +STATE_DIR="${STATE_DIR:?STATE_DIR required}" +GH_REPO="${GH_REPO:?GH_REPO required (owner/repo)}" + +log() { printf '[%s] post-or-update: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } + +COMMENT_FILE="${STATE_DIR}/comment.md" +NEW_BODY=$([ -s "${COMMENT_FILE}" ] && cat "${COMMENT_FILE}" || echo "") +NEW_HASH=$(echo "${NEW_BODY}" | grep -oE 'findings-hash=[a-f0-9]+' | head -1 | cut -d= -f2 || echo "") + +# Find prior bot comment by marker prefix +PRIOR=$(gh api "repos/${GH_REPO}/issues/${PR_NUMBER}/comments" \ + --jq '.[] | select(.body | startswith("" + echo "### PR Quality — ${COUNT} finding(s)" + echo + jq -r '.findings[] | "- `\(.span)` — \(.explanation)\n Rule: [`\(.citation)`](.claude/memory/feedback_\(.citation).md) — see memory entry."' "${AGG}" + if [[ "${SUPPRESSED}" -gt 0 ]]; then + echo + echo "
+${SUPPRESSED} additional lower-severity findings suppressed (${SUPPRESSED_RULES})" + echo + echo "Full set in CI artifact \`pr-quality-artifacts\`." + echo "
" + fi + echo + echo "---" + echo + echo "Suggestive only; humans decide. Opt out via label \`skip-pr-quality\`." +} > "${STATE_DIR}/comment.md" + +log "comment.md rendered ($(wc -c < "${STATE_DIR}/comment.md") bytes, ${SUPPRESSED} suppressed)" diff --git a/.github/workflows/pr-quality.yml b/.github/workflows/pr-quality.yml new file mode 100644 index 00000000..e3fdbe98 --- /dev/null +++ b/.github/workflows/pr-quality.yml @@ -0,0 +1,67 @@ +name: pr-quality + +on: + pull_request: + types: [opened, synchronize, reopened] + +permissions: + contents: read + pull-requests: write + id-token: write + +concurrency: + group: pr-quality-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + review: + # Skip on fork PRs — `pull_request` from a fork has a read-only token, so + # `pull-requests: write` is not honored. Posting a comment would silently + # fail. Switching to `pull_request_target` is a security footgun (the fork's + # workflow code would run with write tokens). v1 explicitly skips forks. + if: github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Verify required secrets + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + run: | + if [[ -z "${ANTHROPIC_API_KEY:-}" ]]; then + echo "ANTHROPIC_API_KEY secret missing; skipping pr-quality run" + exit 0 + fi + + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Run pr-quality skill via claude-code-action + uses: anthropics/claude-code-action@v1 + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + prompt: | + Apply the .claude/skills/pr-quality/ skill against this PR. + Follow the procedure in SKILL.md exactly. Do not invent rules + beyond the v1 registry. Do not post a comment when findings + count is zero. Do not future-proof. + claude_args: | + { + "model": "claude-opus-4-7", + "max_tokens": 8000 + } + env: + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + GH_REPO: ${{ github.repository }} + STATE_DIR: ${{ github.workspace }}/pr-quality-state/run-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} + + - name: Upload pr-quality artifacts + if: always() + uses: actions/upload-artifact@v4 + with: + name: pr-quality-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} + path: pr-quality-state/ + retention-days: 14 + if-no-files-found: ignore From 3fea14193daf94b58e10c91c03fb8623a4be4a24 Mon Sep 17 00:00:00 2001 From: bdchatham Date: Sun, 31 May 2026 20:11:46 -0700 Subject: [PATCH 2/2] refactor(skills/pr-quality): drop CI infra; agent-invoked + working-agreement-wired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 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 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 --- .claude/skills/README.md | 2 +- .claude/skills/pr-quality/SKILL.md | 153 +++++++----------- .claude/skills/pr-quality/evals/evals.json | 91 ++++++----- .../pr-quality/references/format-spec.md | 33 ++-- .../pr-quality/references/guardrails.md | 75 --------- .../references/judge-prompt-template.md | 64 -------- .../judges/harbor_ecr_convention.md | 34 ---- .../references/judges/no_cpu_limits.md | 41 ----- .../pr-quality/references/rule-registry.md | 90 ++++++++--- .claude/skills/pr-quality/scripts/README.md | 15 -- .../skills/pr-quality/scripts/aggregate.sh | 49 ------ .../skills/pr-quality/scripts/check-optout.sh | 22 --- .../pr-quality/scripts/check-pr-size.sh | 40 ----- .../pr-quality/scripts/dispatch-judges.sh | 83 ---------- .../pr-quality/scripts/fetch-context.sh | 69 -------- .../skills/pr-quality/scripts/judge-llm.sh | 73 --------- .../pr-quality/scripts/judge-mechanical.sh | 104 ------------ .../scripts/judge-skill-dispatch.sh | 60 ------- .../pr-quality/scripts/post-or-update.sh | 56 ------- .../pr-quality/scripts/render-comment.sh | 48 ------ .../pr-quality/scripts/scan-harbor-ghcr.sh | 34 ++++ .../pr-quality/scripts/scan-yaml-cpu.sh | 39 +++++ .github/workflows/pr-quality.yml | 67 -------- AGENTS.md | 2 + CLAUDE.md | 1 + 25 files changed, 258 insertions(+), 1087 deletions(-) delete mode 100644 .claude/skills/pr-quality/references/guardrails.md delete mode 100644 .claude/skills/pr-quality/references/judge-prompt-template.md delete mode 100644 .claude/skills/pr-quality/references/judges/harbor_ecr_convention.md delete mode 100644 .claude/skills/pr-quality/references/judges/no_cpu_limits.md delete mode 100644 .claude/skills/pr-quality/scripts/README.md delete mode 100755 .claude/skills/pr-quality/scripts/aggregate.sh delete mode 100755 .claude/skills/pr-quality/scripts/check-optout.sh delete mode 100755 .claude/skills/pr-quality/scripts/check-pr-size.sh delete mode 100755 .claude/skills/pr-quality/scripts/dispatch-judges.sh delete mode 100755 .claude/skills/pr-quality/scripts/fetch-context.sh delete mode 100755 .claude/skills/pr-quality/scripts/judge-llm.sh delete mode 100755 .claude/skills/pr-quality/scripts/judge-mechanical.sh delete mode 100755 .claude/skills/pr-quality/scripts/judge-skill-dispatch.sh delete mode 100755 .claude/skills/pr-quality/scripts/post-or-update.sh delete mode 100755 .claude/skills/pr-quality/scripts/render-comment.sh create mode 100755 .claude/skills/pr-quality/scripts/scan-harbor-ghcr.sh create mode 100755 .claude/skills/pr-quality/scripts/scan-yaml-cpu.sh delete mode 100644 .github/workflows/pr-quality.yml diff --git a/.claude/skills/README.md b/.claude/skills/README.md index 687dca80..5cafbbb8 100644 --- a/.claude/skills/README.md +++ b/.claude/skills/README.md @@ -46,7 +46,7 @@ Two complementary artifact-capture skills. Coral / council should offer them at ### Hardening - **`bugbash/`** — Long-running, read-only adversarial review of an existing system by the council of experts. Loops discovery + challenger passes against a named target (`/bugbash SeiNode controller`) until the experts converge on a launch verdict. Output is a structured findings log at `docs/bugbash/.md` with per-item Scenario / Impact / Issue / Fix sketch / Test coverage. Inspired by the [RALPHY loop](https://github.com/snarktank/ralph), reframed for hardening before launch. Distinct from `/security-review` (single-pass, security-only) and `/coral` (collaborative iteration, not adversarial). -- **`pr-quality/`** — Post-PR convention + quality coordinator. Runs as GitHub Action on `pull_request: opened/synchronize`. Dispatches a fixed v1 judge set in parallel (verbosity via /brevity; 5 convention rules: no_cpu_limits, harbor_ecr_convention, narration_comments, temporary_migration_notes, authoritative_voice). Posts one anchored PR comment with up to 5 findings, severity-ranked; silence on zero findings. Suggestive only — never gates merge. Opt out via PR label `skip-pr-quality`. Sibling to /brevity (verbosity-only). +- **`pr-quality/`** — Two-mode PR convention coordinator. Pre-PR (agent-invoked before `gh pr create`; findings surface inline against the staged diff for revision) and post-PR (user-invocable via `/pr-quality `; posts a fresh comment with findings). No CI infrastructure — proactive trigger lives in `CLAUDE.md` / `AGENTS.md` working agreements (same pattern as `/brevity`). v1 dispatches verbosity via `/brevity` (subagent-loads-target-skill; pr-quality detects the trigger, brevity owns the standard) + 5 convention rules (`no_cpu_limits`, `harbor_ecr_convention`, `narration_comments`, `temporary_migration_notes`, `authoritative_voice`). Suggestive only — never gates merge. Sibling to `/brevity` (verbosity-only). ### Investigation - **`root-cause/`** — Disciplined, data-driven, multi-expert investigation of complex problems in the Sei platform stack (sei-k8s-controller, seictl, sei-sidecar, sei-chain, release-test/qa-testing, platform/K8s). Forces signals before hypotheses, ≥2 competing hypotheses before evidence, retrieved provenance (not paraphrased), and falsification before conclusion. Dispatches `.claude/agents/` specialists in **parallel + blinded + with assigned dissent** to prevent the consensus-theater / sycophancy failure mode documented in the multi-agent LLM literature. Output is a multi-cause ranked conclusion — never a single root cause. Distinct from `/bugbash` (pre-launch adversarial), `/coral` (collaborative iteration), and live incident command (mitigate first; this skill is for understanding). Tide on-chain agentic harness is explicitly out of scope. diff --git a/.claude/skills/pr-quality/SKILL.md b/.claude/skills/pr-quality/SKILL.md index 71c8a7fd..a3cc863c 100644 --- a/.claude/skills/pr-quality/SKILL.md +++ b/.claude/skills/pr-quality/SKILL.md @@ -1,133 +1,88 @@ --- name: pr-quality -description: "Used by the .github/workflows/pr-quality.yml GitHub Action on pull_request: opened/synchronize/reopened. Coordinates a parallel-dispatch quality review against a fixed v1 rule registry (verbosity via /brevity; convention adherence — 5 rules: no_cpu_limits, harbor_ecr_convention, narration_comments, temporary_migration_notes, authoritative_voice). Posts a single anchored PR comment with findings, capped at 5, severity-ranked. Suggestive only — never gates merge. Opt-out via PR label skip-pr-quality. Anti-triggers: NOT for blocking PRs (use branch protection); NOT for style auto-fix (use gofmt/prettier); NOT for license/IP/security scanning (separate tooling); NOT for inline-rule expansion at runtime (rule registry is fixed per release; new rules require a PR). For multi-component design / cross-review, use /council. For brevity-only on agent output, use /brevity directly." +description: "Use when about to open a PR or reviewing one — 'opening a PR', 'run pr-quality on this', 'check my PR', '/pr-quality', '/pr-quality 94'. Fires before `gh pr create` (pre-PR mode — agent surfaces findings inline against the staged diff so the author can revise) and on demand against an existing PR (post-PR mode — posts a fresh PR comment with findings). Suggestive only — never gates merge. Anti-triggers: NOT for blocking PRs (use branch protection if you need gating); NOT for style auto-fix (use gofmt / prettier); NOT for license / IP / security scanning (separate tooling). For verbosity-only on agent output, use /brevity directly. For multi-component design / cross-review, use /council." --- # pr-quality -A PR-time coordinator that runs as a GitHub Action. Receives a PR diff + body + commits; dispatches a fixed set of judges in parallel; posts a single anchored comment with up to 5 findings ranked by severity. **Suggestive only — never gates merge.** Silence is the success state. +A two-mode coordinator that runs a fixed v1 judge set against a PR's diff + body and surfaces findings — either inline (pre-PR) or as a single fresh PR comment (post-PR). Suggestive only. Silence on zero findings. -This is a **procedural skill encoding a runtime contract**. The contract — what rules fire, how findings are ranked, when to comment, when not to comment — is fixed per release. The skill is what an unattended CI script reads to enforce that contract; it has no human in the loop to negotiate exceptions. +This skill is **agent-invoked and user-invocable**. No CI workflow, no GitHub App install, no secret management. Proactive trigger lives in `CLAUDE.md` / `AGENTS.md` working-agreement references. ## Guardrails -This skill posts **suggestive PR comments**, never gates merges. Before any side-effecting action: +Before any side-effecting action: -1. **Surface check.** Confirm the workflow is running on a `pull_request` event (`opened`, `synchronize`, or `reopened`). If invoked outside that context, halt with a clear log message — there's no PR to comment against. +1. **Mode check.** Pre-PR mode reads the staged diff (`git diff --cached` or the planned diff if PR isn't created yet) + the agent's planned body. Post-PR mode reads an existing PR via `gh pr view --json body` + `gh pr diff `. If neither is determinable, halt. -2. **Opt-out check.** If the PR has the label `skip-pr-quality`, halt before fetching diff. The label is the contract; respect it. +2. **Refusal conditions.** This skill will refuse to: + - **Block merge.** No exit code on findings; the skill is suggestive by contract. + - **Run rules outside the locked v1 set** documented in [`references/rule-registry.md`](references/rule-registry.md). Adding a rule is a PR against that file, not a runtime override. + - **Edit code or push commits.** Pre-PR surfaces findings to the agent for revision; agent decides what to apply. Post-PR posts a comment only. + - **Comment on closed or merged PRs.** Post-PR mode silently skips. + - **Comment on someone else's PR without explicit user invocation.** Pre-PR is the agent's own pre-flight; post-PR requires the user to name the PR (`/pr-quality `). -3. **Refusal conditions.** This skill will refuse to: - - **Block merge.** No `failure` exit code on findings. Only infrastructure failures (missing API key, GitHub auth) produce failed runs. - - **Post more than 5 findings.** Hard cap. When over cap, drop by (severity ascending, then mechanism LLM > mechanical), surface truncation explicitly in the comment footer. - - **Spam comments on force-push.** Idempotency via the anchored marker ``. If findings-hash matches the previous run, skip the PATCH entirely. - - **Post a comment when zero findings.** No thumbs-up, no "looks good" confirmation. Silence is the success state. - - **Run rules outside the locked v1 list.** The 5 rules + the brevity-dispatch are the only judges that fire. Adding new rules requires a PR that updates `references/rule-registry.md`. No inline rule expansion. - - **Add interactive features at runtime.** No slash-command replies (`/show-all`, `/expand`, etc.), no `issue_comment.created` triggers, no thread reply parsing. v1 is one-shot render-and-post. - - **Future-proof beyond v1.** No marker versioning, no bot-rotation handling, no manual-edit detection in the comment lookup. These are deferred concerns; if they become real, a PR addresses them. Anticipation is feature creep. - - **Edit code or push commits.** Read + PR-comment-write scope only. - - **Comment on closed or merged PRs.** No drive-by suggestions after the fact. - -4. **Halt-and-surface conditions** (workflow-run-fail with log, no PR comment posted): - - PR diff is empty or only touches `.github/workflows/pr-quality.yml` itself (the bot does not review its own changes). - - PR exceeds 5000 changed lines (judge precision degrades; defer to human review). - - Any dispatched judge subagent fails twice in a row (transient errors retried once; persistent failure → no partial findings). - - Workflow detects concurrent runs from `cancel-in-progress` → exit cleanly without partial state. - -5. **Cost guardrail.** Per-PR budget cap: $1.00 worth of LLM tokens. If the dimension judges exceed budget mid-run, halt with truncation log and post whatever findings completed. - -## Preconditions - -- `anthropics/claude-code-action@v1` available in the workflow (canonical Claude Code GH Action). -- Repo secret `ANTHROPIC_API_KEY` set. -- Workflow permissions: `contents:read`, `pull-requests:write`, `id-token:write`. -- Repo label `skip-pr-quality` exists (for opt-out). -- `.claude/skills/brevity/` present (the verbosity dimension dispatches to it). -- `references/rule-registry.md` defines the locked v1 rule set; each rule has a dedicated `references/judges/.md` with prompt + scope + few-shot examples. +3. **Halt conditions** (exit cleanly, no comment): + - PR diff is empty. + - PR exceeds the size threshold in `references/rule-registry.md` (default 5000 lines). + - Any LLM judge subagent returns malformed output twice — log and abort that judge; continue with others. ## Procedure -The CI workflow invokes this skill via `claude-code-action`. The skill executes steps below in order via `scripts/`. Each script is debuggable standalone via the `dispatch-judges.sh` orchestrator. - -1. **Check opt-out label** (`scripts/check-optout.sh`). Read `gh pr view --json labels`. If `skip-pr-quality` present, exit 0 with log "opt-out label present; skipping". No comment, no state. - -2. **Check PR-size guardrails** (`scripts/check-pr-size.sh`). Read `gh pr diff --name-only` + `gh pr diff | wc -l`. If empty diff OR only touches `.github/workflows/pr-quality.yml` OR over 5000 changed lines, exit 0 with log. - -3. **Fetch shared context** (`scripts/fetch-context.sh`). Output written to `state/run--/context.json`: - - PR diff (`gh pr diff`) - - PR body - - PR commits (`gh pr view --json commits`) - - Changed-files list with path scope tags (yaml / go / py / ts / md / skill-md / durable-doc / harbor) - - Memory snapshot (read `feedback_*.md` entries for the 5 locked rules, scope-filtered to entries actually applicable based on changed files) +The skill runs from a Claude Code session (interactive or agent-driven). Steps below are executed by Claude using the `Bash` and `Agent` tools; mechanical scans are deterministic scripts, LLM judges are subagent dispatches loading the per-rule prompt files. -4. **Dispatch judges in parallel** (`scripts/dispatch-judges.sh`). Cap parallelism at 5. Each judge is one of: +### Pre-PR mode - **Mechanical** (high-precision, single-shot): - - `no_cpu_limits` — YAML-AST parse of changed `*.yaml` / `*.yml`; walk `resources.limits.cpu`. Pre-render Helm/Kustomize where applicable to avoid templating false-positives. - - `harbor_ecr_convention` — grep for `ghcr.io` in diff lines from `clusters/harbor/**`. +Triggered when an agent is about to invoke `gh pr create` (per the working agreement in `CLAUDE.md` / `AGENTS.md`). - **LLM-judged** (self-consistency n=3 at temp 0.3, require 2/3 agreement): - - `narration_comments` — function-doc style only (comment immediately above `func/def/function` declaration in `*.go` / `*.py` / `*.ts`). - - `temporary_migration_notes` — durable docs only (`CLAUDE.md`, `AGENTS.md`, `README.md`, `docs/**`). - - `authoritative_voice` — skill content only (`.claude/skills/**/*.md`). +1. **Read the staged diff.** `git diff --cached` (or `git diff HEAD origin/main` if the agent has already pushed but not created the PR). Read the agent's planned PR body. +2. **For each rule** in [`references/rule-registry.md`](references/rule-registry.md) whose file scope matches the staged changes: + - Mechanical rules: run the predicate script (`scripts/scan-yaml-cpu.sh`, `scripts/scan-harbor-ghcr.sh`). + - LLM-judged rules: dispatch a subagent with the corresponding `references/judges/.md` as the prompt + the diff slice as input. + - Brevity dispatch: dispatch a subagent that loads `.claude/skills/brevity/SKILL.md` and applies it to the planned PR body. (This judge is a *trigger detector*, not a re-implementation of brevity's rules — the standard lives in `/brevity` itself.) +3. **Surface findings inline.** Group by severity (`warn` first), sort by file path within tier. No comment posted — the agent uses findings to revise before `gh pr create`. +4. **The agent then revises** the body / code / both and re-runs the skill, or proceeds to `gh pr create` if findings count is acceptable. - **Skill-dispatch** (compose with /brevity): - - Verbosity dimension: dispatch a subagent that loads `.claude/skills/brevity/SKILL.md` and applies it to PR body + comment-line additions. Returns brevity's verdict + suggested rewrite. +### Post-PR mode -5. **Aggregate findings** (`scripts/aggregate.sh`). Read each judge's verdict JSON. Build a unified findings list: - - Severity = `warn` (mechanical OR LLM 3/3 consensus) or `nudge` (LLM 2/3 consensus). LLM <2/3 → drop. - - Dedup hash: `(file, line, rule_id)`. Mechanical wins on tie. - - Sort: severity ascending → mechanism ascending (warn-mechanical, warn-LLM, nudge). Within tier, sort by file path. - - Apply 5-cap: drop from tail. Track `suppressed_count` + `suppressed_rules`. +Triggered by `/pr-quality` (current PR by branch) or `/pr-quality ` (explicit PR number). -6. **Render comment** (`scripts/render-comment.sh`). Output `state/run--/comment.md`. Format is owned entirely by [`references/format-spec.md`](references/format-spec.md) — the rendering contract (marker shape, title, finding bullet shape, suppressed-findings disclosure block, disclaimer footer) lives there. This step writes the file; the spec defines the bytes. - - Disclaimer: `Suggestive only; humans decide. Opt out via label \`skip-pr-quality\`.` +1. **Read the PR.** `gh pr view --json body,headRefOid` + `gh pr diff `. +2. **For each rule in scope** — same dispatch as pre-PR mode. +3. **Aggregate findings.** Group by severity, sort by file path. +4. **Post a fresh comment** if findings count > 0: + ``` + gh pr comment --body-file <(...) + ``` + The comment body follows [`references/format-spec.md`](references/format-spec.md). No comment when zero findings — silence is the success state. -7. **Post or update anchored comment** (`scripts/post-or-update.sh`): - - Find existing bot comment by marker prefix ` ### PR Quality — N finding(s) - `:` — . @@ -13,38 +12,30 @@ How the rendered PR comment looks. Matches Tide's broader convention (see `.clau - ... -
+ additional lower-severity findings suppressed - -- `:` — . - ... - -
- --- -Suggestive only; humans decide. Opt out via label `skip-pr-quality`. +Suggestive only; humans decide. ``` ## Rules -1. **Marker is required**. The two-field marker (`sha`, `findings-hash`) MUST be the first line. `post-or-update.sh` parses it to detect prior runs. -2. **Title format**: `### PR Quality — N finding(s)`. N is the post-cap count (max 5). -3. **Finding line shape**: `- \`:\` — .` Followed on the next line (indented 2 spaces): `Rule: [\`\`](.claude/memory/.md) — .` -4. **Relative repo links** for memory citations. They render as live links in GitHub PR comments. -5. **Suppressed-findings block** is a `
` collapsed by default. Include only if `suppressed_count > 0`. -6. **Disclaimer footer** is fixed text: "Suggestive only; humans decide. Opt out via label `skip-pr-quality`." +1. **Title format**: `### PR Quality — N finding(s)`. N is the total finding count (uncapped in v1). +2. **Finding line shape**: `- \`:\` — .` Followed on the next line (indented 2 spaces): `Rule: [\`\`](.claude/memory/.md) — .` +3. **Relative repo links** for memory citations. They render as live links in GitHub PR comments. +4. **Disclaimer footer** is fixed text: "Suggestive only; humans decide." ## Severity rendering -Findings are sorted with `warn` before `nudge`, then mechanical before LLM-judged within tier. There is NO explicit severity badge in the rendered output — the order IS the severity signal. Adding `[WARN]` / `[NUDGE]` prefixes is feature creep; resist. +Findings are sorted `warn` before `nudge`, then mechanical before LLM-judged within tier. There is NO explicit severity badge — the order IS the signal. Adding `[WARN]` / `[NUDGE]` prefixes is feature creep; resist. ## What this format does NOT include +- No anchored marker / hash dedupe (v1 posts fresh) +- No 5-finding cap or suppressed-block disclosure (v1 uncapped) - No emoji severity badges (🔴 / 🟡) - No `[blocker]` / `[nit]` / `[info]` labels - No reaction-driven dismissal mechanism -- No "ack" or "applied" footers -- No interactive slash-commands ("/show-all", "/dismiss") -- No edit-history of prior runs +- No interactive slash-commands +- No opt-out label reference (local invocation; user just doesn't invoke) -All of the above are feature-creep beyond v1. If they become real needs, file a PR against this file. +All of the above are feature-creep beyond v1. Un-defer triggers documented in `rule-registry.md` deferred-mechanisms table. diff --git a/.claude/skills/pr-quality/references/guardrails.md b/.claude/skills/pr-quality/references/guardrails.md deleted file mode 100644 index 030a0aea..00000000 --- a/.claude/skills/pr-quality/references/guardrails.md +++ /dev/null @@ -1,75 +0,0 @@ -# pr-quality — Extended Safety Model - -Detailed version of the Guardrails stanza in SKILL.md. Reference for what the workflow will and won't do, and why. - -## Suggestive-only contract - -The skill posts comments. It does not: -- Set commit status / required checks -- Push commits or edit files -- Lock conversations or apply labels -- Request reviewers - -Branch protection rules handle gating. The skill is one signal among many; humans decide what's blocking. - -## v1 rule registry is closed at runtime - -The 5 rules + brevity dispatch are the only judges that fire in v1. The dispatch runner refuses to load any rule not present in `references/rule-registry.md`. - -Why this matters: a procedural skill running in CI has no human in the loop. If a new rule is added inline ("I'll just add a check for X"), there's no review of the rule's precision, no eval coverage, no scope filter. Closed at runtime = the registry is the contract. - -To add a rule: PR against `rule-registry.md` + `judges/.md` + `evals/evals.json`. The author-skill methodology applies (RED a generalist on the new rule; GREEN with the rule's few-shot prompts; require 2/3 self-consistency). - -## Cost ceiling - -LLM judges cost money. The per-PR cap is $1.00 (rough; tune in `scripts/dispatch-judges.sh`). When the cap is approached: -- Stop dispatching new judges -- Return whatever findings completed -- Log the truncation - -Pathological PR (e.g., 5000-line monorepo refactor) hits the size guardrail FIRST and exits before cost guardrail engages. Cost guardrail is the secondary safety net. - -## Idempotency contract - -The marker shape `` carries enough information to dedupe AND detect content change: - -- Same SHA + same findings-hash → bot was already invoked for this exact state; no-op. -- Same SHA + different findings-hash → not possible (judges are deterministic for fixed input); if observed, log as anomaly. -- Different SHA → re-run; if findings-hash matches → skip PATCH (no churn). -- Different SHA + different findings-hash → PATCH in place. - -The workflow also uses GH Actions `concurrency` group with `cancel-in-progress: true` for the per-PR group, killing superseded runs. - -## Comment ownership - -The bot owns its anchored comment. Manual human edits to the bot comment ARE overwritten on the next run. The marker signals bot ownership; humans should reply in new comments, not edit the bot's body. - -If this becomes an actual annoyance, the v2 PR can add manual-edit detection. v1 explicitly does not. - -## Closed/merged PR behavior - -The workflow only fires on `pull_request: opened, synchronize, reopened`. It does not fire on `closed` or `merged`. Comments left on a merged PR stay (they're historical record); the bot does not chase them. - -## Opt-out contract - -The label `skip-pr-quality` is the emergency escape valve. v1 treats it as a hard halt: -- Workflow exits 0 with log line -- No diff fetched, no judges dispatched -- Existing bot comment (if any) is left alone — the label says "skip", not "delete history" - -To un-skip a PR, remove the label and the next push (or `synchronize` event) reactivates the bot. - -## Cost / time profile - -Approximate per-PR cost from community data on `claude-code-action`: -- Time: 30-120 seconds (3-5 judges in parallel + comment post) -- Tokens: 5k-20k input, 1k-5k output → ~$0.10-$0.50 - -Larger PRs (toward 5000-line cap) skew higher; smaller PRs (single-file fixes) closer to floor. - -## What this skill is NOT a replacement for - -- **Human review.** The bot surfaces patterns; humans review the change. -- **CI tests.** The bot does not run tests. Existing test workflows continue. -- **Branch protection.** Required checks are configured independently. -- **Security scanning.** Trivy, gitleaks, snyk, etc. live in separate workflows. diff --git a/.claude/skills/pr-quality/references/judge-prompt-template.md b/.claude/skills/pr-quality/references/judge-prompt-template.md deleted file mode 100644 index 8b986676..00000000 --- a/.claude/skills/pr-quality/references/judge-prompt-template.md +++ /dev/null @@ -1,64 +0,0 @@ -# Judge Prompt Template - -All LLM-judged rules share this prompt shape. Per-rule prompts live in `judges/.md`; this file defines the contract every judge call follows. - -## Required structure - -``` -You are judging whether a diff hunk violates the [rule_id] rule from Tide's convention set. - -**Rule**: [one-sentence statement of the rule, verbatim from the memory entry] - -**Scope**: [file globs the rule applies to] - -**You will receive**: -- diff_hunk: the changed lines with file path -- context_before / context_after: ±10 lines around the hunk -- changed_file_path - -**You must output structured JSON**: -{ - "verdict": "violation" | "no_violation", - "span": ":" or null, - "citation": "[rule_id]", // verbatim from the closed enum below - "confidence": "low" | "medium" | "high", - "explanation": "one sentence, max 30 words" -} - -**citation enum** (closed; emit verbatim or auto-fail): -- "no_cpu_limits" -- "harbor_ecr_convention" -- "narration_comments" -- "temporary_migration_notes" -- "authoritative_voice" - -If you cannot decide between violation and no_violation, emit verdict=no_violation. Better to miss than to fabricate. - -**Examples** (2 negatives + 3 positives minimum per rule; per-rule lists in judges/.md): -[5-shot block, per rule] - -**Now judge**: -[diff_hunk with context] -``` - -## Self-consistency contract - -Each LLM-judged rule runs n=3 samples at temp=0.3. - -- 3/3 `violation` → severity `warn` -- 2/3 `violation` → severity `nudge` -- ≤1/3 `violation` → no finding emitted - -The dispatch runner in `scripts/dispatch-judges.sh` enforces this; individual judges return a single verdict per sample. - -## Closed-enum citation requirement - -The `citation` field must be one of the enum values verbatim. If a judge emits a citation outside the enum, the dispatch runner treats the entire sample as `no_violation`. This is the Greptile / CodeRabbit pattern — bind the model to the known rulebook; refuse free-form complaints. - -## Why this shape - -- **Structured output** → deterministic aggregation; no regex on free-form prose. -- **One rule per call** → precision per Zheng et al. 2023; task-stacking craters precision. -- **±10 lines of context** → judge can distinguish a comment that narrates its own line vs. one documenting package intent. -- **Confidence field** → tie-breaker for severity ranking + 5-cap drop strategy. -- **Closed-enum citation** → eliminates the "model fabricates a violation" failure mode. diff --git a/.claude/skills/pr-quality/references/judges/harbor_ecr_convention.md b/.claude/skills/pr-quality/references/judges/harbor_ecr_convention.md deleted file mode 100644 index dbedbd8b..00000000 --- a/.claude/skills/pr-quality/references/judges/harbor_ecr_convention.md +++ /dev/null @@ -1,34 +0,0 @@ -# Judge: harbor_ecr_convention (mechanical) - -Path-scoped grep. Not an LLM judge. - -## Mechanism - -1. Identify changed files under `clusters/harbor/**`. -2. For each, grep changed lines (from `gh pr diff`) for `ghcr.io`. -3. If matched, emit finding at the matched line. - -## Output shape per finding - -```json -{ - "verdict": "violation", - "span": ":", - "citation": "harbor_ecr_convention", - "confidence": "high", - "explanation": "Harbor workload images go to AWS ECR by convention; ghcr.io reference detected." -} -``` - -## Scope filter - -- File path starts with `clusters/harbor/` -- Match restricted to added/modified lines in the diff (skip removed) - -## False-positive cases (intentional non-issues) - -- Comments referencing ghcr.io for historical context: in practice these are rare in `clusters/harbor/` and reviewable when surfaced; v1 doesn't try to distinguish comment from value (raw grep). Tolerable until proven noisy. - -## Cites - -Memory: `feedback_harbor_ecr_convention` — "default to AWS ECR for harbor workloads, not ghcr.io" diff --git a/.claude/skills/pr-quality/references/judges/no_cpu_limits.md b/.claude/skills/pr-quality/references/judges/no_cpu_limits.md deleted file mode 100644 index 1b7e024d..00000000 --- a/.claude/skills/pr-quality/references/judges/no_cpu_limits.md +++ /dev/null @@ -1,41 +0,0 @@ -# Judge: no_cpu_limits (mechanical) - -Mechanical YAML-AST check. Not an LLM judge. - -## Mechanism - -1. Read every changed YAML file (`*.yaml`, `*.yml`). -2. Pre-render Helm/Kustomize where templating present: - - If file path contains `templates/` and adjacent `Chart.yaml` exists → `helm template . > /tmp/rendered.yaml` - - If `kustomization.yaml` adjacent → `kustomize build . > /tmp/rendered.yaml` - - Else: parse the file directly. -3. YAML-AST walk: find every node at path `*.resources.limits.cpu` (in K8s container spec). -4. If found AND the value is set (not `null`, not empty string), emit finding. - -## Output shape per finding - -```json -{ - "verdict": "violation", - "span": ":", - "citation": "no_cpu_limits", - "confidence": "high", - "explanation": "CPU limit set; throttling is an anti-pattern. Set requests only, leave limits unset." -} -``` - -## Scope filter - -- File path matches `*.yaml` or `*.yml` -- Skip files that ONLY contain `kind: Kustomization` (those don't have container specs) -- Skip files under `.claude/skills/**` (skill metadata, not workload spec) - -## False-positive defenses - -- Comment-only matches (`# limits.cpu: 500m`) — eliminated by YAML-AST parse. -- Helm/Kustomize templating residue — eliminated by pre-rendering. -- YAML anchors (`&cpuLimit`) — followed during AST walk. - -## Cites - -Memory: `feedback_no_cpu_limits` — "set CPU requests, leave limits unset; throttling is an anti-pattern (memory limits stay)" diff --git a/.claude/skills/pr-quality/references/rule-registry.md b/.claude/skills/pr-quality/references/rule-registry.md index d260150f..4fae87d6 100644 --- a/.claude/skills/pr-quality/references/rule-registry.md +++ b/.claude/skills/pr-quality/references/rule-registry.md @@ -1,43 +1,87 @@ # pr-quality — Rule Registry (v1) -The fixed set of rules the coordinator dispatches. **The registry is per-release**: adding, removing, or modifying a rule requires a PR to this file + the corresponding `judges/.md`. Runtime expansion is refused per SKILL.md guardrails. +The fixed set of rules the coordinator dispatches. **The registry is per-release**: adding, removing, or modifying a rule requires a PR to this file. Runtime expansion is refused per SKILL.md guardrails. + +## Knobs + +| Knob | Value | What it does | +|---|---|---| +| `size_threshold_lines` | 5000 | Skill halts cleanly on PRs larger than this (judge precision degrades; defer to human review) | ## v1 active rules (5 + brevity dispatch) -| Rule ID | Mechanism | File scope | Severity | Cites | -|---|---|---|---|---| -| `no_cpu_limits` | YAML-AST | `*.yaml`, `*.yml` | `warn` | `feedback_no_cpu_limits` | -| `harbor_ecr_convention` | path-scoped grep | `clusters/harbor/**` | `warn` | `feedback_harbor_ecr_convention` | -| `narration_comments` | LLM-judge (n=3) | `*.go`, `*.py`, `*.ts` — function-doc above declarations | `warn` (3/3) or `nudge` (2/3) | `feedback_narration_comments` | -| `temporary_migration_notes` | LLM-judge (n=3) | `CLAUDE.md`, `AGENTS.md`, `README.md`, `docs/**` | `warn` (3/3) or `nudge` (2/3) | `feedback_temporary_migration_notes` | -| `authoritative_voice` | LLM-judge (n=3) | `.claude/skills/**/*.md` | `warn` (3/3) or `nudge` (2/3) | `feedback_authoritative_voice` | -| (dispatch) `/brevity` | skill-loaded subagent | PR body + comment-line additions | `nudge` | `feedback_concise_in_code_comments`, `feedback_authoritative_voice` | +### Mechanical rules + +| Rule | File scope | Predicate | Cites | +|---|---|---|---| +| `no_cpu_limits` | `*.yaml`, `*.yml` (excluding `kind: Kustomization`) | `scripts/scan-yaml-cpu.sh` walks parsed YAML for `cpu:` set inside any `limits:` block at deeper indent; emits `:` per match | `feedback_no_cpu_limits` — CPU limits cause throttling; set requests only | +| `harbor_ecr_convention` | added/modified diff lines from `clusters/harbor/**` | `scripts/scan-harbor-ghcr.sh` walks the unified diff, tracking `+++ b/` + `@@` hunk headers; matches `+` lines containing `ghcr.io` | `feedback_harbor_ecr_convention` — Harbor workload images go to AWS ECR, not ghcr.io | + +Both scripts produce stdout-only output (one `:` per finding), stderr-only logs. Severity = `warn` (high-precision mechanical). + +### LLM-judged rules + +| Rule | File scope | Cites | Prompt + few-shot | +|---|---|---|---| +| `narration_comments` | `*.go`, `*.py`, `*.ts` — function-doc style only | `feedback_narration_comments` | [`judges/narration_comments.md`](judges/narration_comments.md) | +| `temporary_migration_notes` | `CLAUDE.md`, `AGENTS.md`, `README.md`, `docs/**` | `feedback_temporary_migration_notes` | [`judges/temporary_migration_notes.md`](judges/temporary_migration_notes.md) | +| `authoritative_voice` | `.claude/skills/**/*.md` | `feedback_authoritative_voice` | [`judges/authoritative_voice.md`](judges/authoritative_voice.md) | + +Per-judge output schema (every LLM-judge subagent returns): + +```json +{ + "verdict": "violation" | "no_violation", + "span": ":" | null, + "citation": "", + "confidence": "low" | "medium" | "high", + "explanation": "one sentence, max 30 words" +} +``` + +`citation` MUST be the verbatim `rule_id` (closed enum: `narration_comments`, `temporary_migration_notes`, `authoritative_voice`). A citation outside the enum auto-coerces to `no_violation` — the judge is bound to the known rulebook, not free-form complaint. + +Severity = `warn` for confidence=high; `nudge` for confidence=medium; `no finding` for confidence=low. + +### Skill-dispatch + +| Dispatch | Target | Applies to | +|---|---|---| +| Verbosity | `.claude/skills/brevity/SKILL.md` | PR body + comment-line additions | + +The judge subagent loads brevity's SKILL.md (subagent-loads-target-skill — same pattern as `/coral`, `/council`) and returns findings against the input slice. **The judge file in `references/judges/` does NOT re-implement brevity's rules** — the standard lives in `/brevity` itself; the judge merely detects that verbosity matters here. ## v1 deferred dimensions + un-defer triggers | Dimension | Un-defer trigger | |---|---| -| Documentation completeness | First PR that ships without a required package/file doc and a reviewer flags it manually. | -| Reference drift (broken links, stale wikilinks) | First stale `[[wikilink]]` or dead PR reference that causes measurable confusion (someone has to ask "what is X"). | -| Commit message hygiene | First non-Conventional-Commits commit that lands on `main`. Conventional Commits is already in CLAUDE.md and rarely violated; automating it would be solving a non-problem today. | +| Documentation completeness | First PR that ships without a required package/file doc and a reviewer flags it manually | +| Reference drift (broken links, stale wikilinks) | First stale `[[wikilink]]` or dead PR reference that causes measurable confusion | +| Commit message hygiene | First non-Conventional-Commits commit that lands on `main` | -When a trigger fires, the un-defer PR adds: the rule entry to this registry, the `judges/.md`, any `references/judges/` prompt assets, and one or more eval cases. +## v1 deferred mechanisms + un-defer triggers -## Memory entries explicitly NOT in v1 +| Mechanism | Un-defer trigger | +|---|---| +| Self-consistency (n=3 sampling, 2/3 agreement) | First false-positive in real-PR output that 3-sample voting would have prevented | +| Severity-rank + 5-cap | Real PR produces >7 findings and reviewer reports the output reads as wallpaper | +| Anchored marker + hash dedupe | Comment spam complaint on repeated invocations against the same PR | +| Cost ceiling per PR | Single invocation budget overrun in real use | -These were considered during the scope cut and excluded: +## Memory entries explicitly NOT in v1 | Memory entry | Why excluded | |---|---| -| `feedback_concise_in_code_comments` | Overlaps with `narration_comments`; lower precision; folded into the brevity dispatch on PR body. | -| `feedback_boring_clear_code` | High false-positive rate on legitimate defensive code; LLM judge precision insufficient. | -| `feedback_iam_scoping` | Fires on architecture decisions, not PR diffs; not PR-shaped. | -| `feedback_isolated_repo_clones` | Claude-actor convention, not a code/doc convention; irrelevant to PR diffs. | +| `feedback_concise_in_code_comments` | Overlaps with `narration_comments`; lower precision; folded into brevity dispatch | +| `feedback_boring_clear_code` | High false-positive rate on legitimate defensive code | +| `feedback_iam_scoping` | Fires on architecture decisions, not PR diffs | +| `feedback_isolated_repo_clones` | Claude-actor convention, not a code/doc convention; irrelevant to PR diffs | ## Adding a rule 1. PR to this file with the new row. -2. New `judges/.md` with prompt + scope + few-shot examples. -3. New eval case in `evals/evals.json`. -4. Cross-review by `reviewer` (convention fit) + `product-engineer` (mechanism coherence). -5. Ship. +2. If LLM-judged: new `judges/.md` with prompt + scope + few-shot examples. +3. If mechanical: new `scripts/scan-.sh` predicate (single-purpose; stdout = findings, stderr = logs). +4. New eval case in `evals/evals.json`. +5. Cross-review by `reviewer` (convention fit) + `product-engineer` (mechanism coherence). +6. Ship. diff --git a/.claude/skills/pr-quality/scripts/README.md b/.claude/skills/pr-quality/scripts/README.md deleted file mode 100644 index d00e6345..00000000 --- a/.claude/skills/pr-quality/scripts/README.md +++ /dev/null @@ -1,15 +0,0 @@ -# Scripts - -Deterministic steps used by the pr-quality skill, each debuggable standalone. The workflow (`.github/workflows/pr-quality.yml`) invokes `claude-code-action@v1` which loads the skill and follows the procedure in SKILL.md by calling these scripts in order. - -| Script | Reads | Writes | -|---|---|---| -| `check-optout.sh` | `gh pr view --json labels` | exit 0 (skip) or continue | -| `check-pr-size.sh` | `gh pr diff` | exit 0 (skip) or continue | -| `fetch-context.sh` | `gh pr diff/view/commits` + memory | `state/run--/context.json` | -| `dispatch-judges.sh` | `context.json`, rule-registry, judges/ | `state/run--/judges/*.json` | -| `aggregate.sh` | `judges/*.json` | `state/run--/aggregated.json` | -| `render-comment.sh` | `aggregated.json`, format-spec | `state/run--/comment.md` | -| `post-or-update.sh` | `comment.md`, `gh pr view --json comments` | PATCH or create or DELETE bot comment | - -All scripts log timestamped entries to `state/run--/audit.log`. None of them exit non-zero on expected halt conditions (opt-out, empty diff, oversized PR) — those are logged and exited 0. diff --git a/.claude/skills/pr-quality/scripts/aggregate.sh b/.claude/skills/pr-quality/scripts/aggregate.sh deleted file mode 100755 index e143a1f4..00000000 --- a/.claude/skills/pr-quality/scripts/aggregate.sh +++ /dev/null @@ -1,49 +0,0 @@ -#!/usr/bin/env bash -# aggregate.sh — dedup + severity-rank + 5-cap. -# -# Reads: state/run--/judges/*.json -# Writes: state/run--/aggregated.json -# { findings: [...], total_count, post_cap_count, suppressed_count, suppressed_rules: [...] } - -set -euo pipefail - -STATE_DIR="${STATE_DIR:?STATE_DIR required}" -JUDGES_DIR="${STATE_DIR}/judges" - -log() { printf '[%s] aggregate: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } - -# Collect all violations from all judges, normalize shape -ALL=$(jq -s '[.[] | select(.verdict == "violation") | .findings // [.]] | add // []' "${JUDGES_DIR}"/*.json) - -# Dedup by (file, line, rule_id); mechanical wins on tie -DEDUPED=$(echo "${ALL}" | jq ' - group_by(.span + "|" + .citation) - | map(sort_by(if .mechanism == "mechanical" then 0 else 1 end) | .[0]) -') - -# Severity rank: warn-mechanical, warn-llm, nudge -RANKED=$(echo "${DEDUPED}" | jq ' - sort_by( - (if .severity == "warn" then 0 else 1 end), - (if .mechanism == "mechanical" then 0 else 1 end), - .span - ) -') - -TOTAL=$(echo "${RANKED}" | jq 'length') -CAP=5 -POST_CAP=$([[ "${TOTAL}" -gt "${CAP}" ]] && echo "${CAP}" || echo "${TOTAL}") -SUPPRESSED=$([[ "${TOTAL}" -gt "${CAP}" ]] && echo "$((TOTAL - CAP))" || echo "0") -SUPPRESSED_RULES=$(echo "${RANKED}" | jq --argjson cap "${CAP}" '[.[$cap:] | .[].citation] | unique') -TOP=$(echo "${RANKED}" | jq --argjson cap "${CAP}" '.[:$cap]') - -jq -n \ - --argjson findings "${TOP}" \ - --argjson total "${TOTAL}" \ - --argjson post_cap "${POST_CAP}" \ - --argjson suppressed "${SUPPRESSED}" \ - --argjson suppressed_rules "${SUPPRESSED_RULES}" \ - '{findings: $findings, total_count: $total, post_cap_count: $post_cap, suppressed_count: $suppressed, suppressed_rules: $suppressed_rules}' \ - > "${STATE_DIR}/aggregated.json" - -log "aggregated: ${POST_CAP}/${TOTAL} surfaced, ${SUPPRESSED} suppressed" diff --git a/.claude/skills/pr-quality/scripts/check-optout.sh b/.claude/skills/pr-quality/scripts/check-optout.sh deleted file mode 100755 index d6ec2546..00000000 --- a/.claude/skills/pr-quality/scripts/check-optout.sh +++ /dev/null @@ -1,22 +0,0 @@ -#!/usr/bin/env bash -# check-optout.sh — halt if PR has the skip-pr-quality label. -# -# Reads: gh pr view --json labels -# Exits: 0 (continue) or 0 with log (skip) - -set -euo pipefail - -PR_NUMBER="${PR_NUMBER:?PR_NUMBER required}" -STATE_DIR="${STATE_DIR:?STATE_DIR required}" - -log() { printf '[%s] check-optout: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } - -LABELS=$(gh pr view "${PR_NUMBER}" --json labels --jq '.labels[].name' 2>/dev/null || true) - -if printf '%s\n' "${LABELS}" | grep -qx "skip-pr-quality"; then - log "skip-pr-quality label present; exiting cleanly" - echo "PR_QUALITY_SKIP=1" >> "${GITHUB_ENV:-/dev/null}" - exit 0 -fi - -log "no opt-out label; continuing" diff --git a/.claude/skills/pr-quality/scripts/check-pr-size.sh b/.claude/skills/pr-quality/scripts/check-pr-size.sh deleted file mode 100755 index a1a8a810..00000000 --- a/.claude/skills/pr-quality/scripts/check-pr-size.sh +++ /dev/null @@ -1,40 +0,0 @@ -#!/usr/bin/env bash -# check-pr-size.sh — halt on empty/oversized/self-edit PRs. -# -# Exits 0 with log if: -# - diff is empty -# - diff only touches .github/workflows/pr-quality.yml -# - diff > 5000 lines -# Else continues. - -set -euo pipefail - -PR_NUMBER="${PR_NUMBER:?PR_NUMBER required}" -STATE_DIR="${STATE_DIR:?STATE_DIR required}" - -log() { printf '[%s] check-pr-size: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } - -CHANGED_FILES=$(gh pr diff "${PR_NUMBER}" --name-only 2>/dev/null || true) -DIFF_LINES=$(gh pr diff "${PR_NUMBER}" 2>/dev/null | wc -l | tr -d ' ') - -if [[ -z "${CHANGED_FILES}" || "${DIFF_LINES}" -eq 0 ]]; then - log "empty diff; exiting cleanly" - echo "PR_QUALITY_SKIP=1" >> "${GITHUB_ENV:-/dev/null}" - exit 0 -fi - -# Self-edit check: only file changed is the workflow itself -if [[ "$(printf '%s\n' "${CHANGED_FILES}" | wc -l | tr -d ' ')" -eq 1 ]] && \ - [[ "${CHANGED_FILES}" == ".github/workflows/pr-quality.yml" ]]; then - log "PR only touches the bot's own workflow; bot does not review itself; exiting cleanly" - echo "PR_QUALITY_SKIP=1" >> "${GITHUB_ENV:-/dev/null}" - exit 0 -fi - -if [[ "${DIFF_LINES}" -gt 5000 ]]; then - log "PR exceeds 5000 changed lines (${DIFF_LINES}); judge precision degrades; deferring to human review" - echo "PR_QUALITY_SKIP=1" >> "${GITHUB_ENV:-/dev/null}" - exit 0 -fi - -log "PR size OK: ${DIFF_LINES} lines, $(printf '%s\n' "${CHANGED_FILES}" | wc -l | tr -d ' ') files" diff --git a/.claude/skills/pr-quality/scripts/dispatch-judges.sh b/.claude/skills/pr-quality/scripts/dispatch-judges.sh deleted file mode 100755 index 590c943e..00000000 --- a/.claude/skills/pr-quality/scripts/dispatch-judges.sh +++ /dev/null @@ -1,83 +0,0 @@ -#!/usr/bin/env bash -# dispatch-judges.sh — parallel dispatch of the v1 judge set. -# -# Reads: state/run--/context.json -# Writes: state/run--/judges/.json (one per rule) -# -# Caps parallelism at 5. Each judge is invoked via claude-code-action's -# subagent dispatch with the per-rule prompt template loaded from -# .claude/skills/pr-quality/references/judges/.md. -# -# Mechanical rules (no_cpu_limits, harbor_ecr_convention): single deterministic pass. -# LLM-judged rules: n=3 samples at temp=0.3, 2/3 self-consistency. -# Skill dispatch (brevity): subagent loads .claude/skills/brevity/SKILL.md and applies. - -set -euo pipefail - -STATE_DIR="${STATE_DIR:?STATE_DIR required}" -CONTEXT="${STATE_DIR}/context.json" -JUDGES_DIR="${STATE_DIR}/judges" -mkdir -p "${JUDGES_DIR}" - -log() { printf '[%s] dispatch-judges: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } - -# Rule registry: the only judges that fire. Adding here is a runtime-bypass; new rules go through PR review of references/rule-registry.md. -RULES=( - "no_cpu_limits:mechanical" - "harbor_ecr_convention:mechanical" - "narration_comments:llm:3" - "temporary_migration_notes:llm:3" - "authoritative_voice:llm:3" - "brevity_dispatch:skill" -) - -# Per-rule scope filter — if no changed files match the rule's scope, skip dispatch entirely. -rule_applies() { - local rule="$1" - local changed_tags - changed_tags=$(jq -r '[.changed_files[].scope_tags[]] | unique[]' "${CONTEXT}") - case "${rule}" in - no_cpu_limits) grep -qx "yaml" <<< "${changed_tags}" ;; - harbor_ecr_convention) grep -qx "harbor" <<< "${changed_tags}" ;; - narration_comments) grep -qxE "go|py|ts" <<< "${changed_tags}" ;; - temporary_migration_notes) grep -qx "durable-doc" <<< "${changed_tags}" ;; - authoritative_voice) grep -qx "skill-md" <<< "${changed_tags}" ;; - brevity_dispatch) return 0 ;; # PR body always present - *) return 1 ;; - esac -} - -# Track cost; halt if over $1.00. (Implementation detail of the claude-code-action runtime; this script logs.) -COST_CAP_USD="1.00" - -# Background dispatch with parallelism cap of 5 -PIDS=() -for rule_spec in "${RULES[@]}"; do - IFS=':' read -r rule kind n <<< "${rule_spec}" - if ! rule_applies "${rule}"; then - log "rule ${rule}: scope does not apply; skipping" - echo '{"verdict": "no_violation", "skipped": "scope_filter"}' > "${JUDGES_DIR}/${rule}.json" - continue - fi - log "dispatching judge: ${rule} (${kind}${n:+, n=${n}})" - ( - case "${kind}" in - mechanical) bash "$(dirname "$0")/judge-mechanical.sh" "${rule}" ;; - llm) bash "$(dirname "$0")/judge-llm.sh" "${rule}" "${n}" ;; - skill) bash "$(dirname "$0")/judge-skill-dispatch.sh" "${rule}" ;; - esac - ) > "${JUDGES_DIR}/${rule}.json" 2>>"${STATE_DIR}/audit.log" & - PIDS+=($!) - # Cap parallelism at 5 - while [[ ${#PIDS[@]} -ge 5 ]]; do - wait -n - NEW_PIDS=() - for pid in "${PIDS[@]}"; do - if kill -0 "${pid}" 2>/dev/null; then NEW_PIDS+=("${pid}"); fi - done - PIDS=("${NEW_PIDS[@]}") - done -done -wait - -log "all judges complete: $(ls "${JUDGES_DIR}"/*.json 2>/dev/null | wc -l) outputs" diff --git a/.claude/skills/pr-quality/scripts/fetch-context.sh b/.claude/skills/pr-quality/scripts/fetch-context.sh deleted file mode 100755 index 25e3e182..00000000 --- a/.claude/skills/pr-quality/scripts/fetch-context.sh +++ /dev/null @@ -1,69 +0,0 @@ -#!/usr/bin/env bash -# fetch-context.sh — collect shared context once for all judges. -# -# Writes: state/run--/context.json with: -# { pr_number, head_sha, diff, body, commits, changed_files: [{path, scope_tags}], memory: {...} } - -set -euo pipefail - -PR_NUMBER="${PR_NUMBER:?PR_NUMBER required}" -HEAD_SHA="${HEAD_SHA:?HEAD_SHA required}" -STATE_DIR="${STATE_DIR:?STATE_DIR required}" -REPO_ROOT="${REPO_ROOT:-$(git rev-parse --show-toplevel)}" - -log() { printf '[%s] fetch-context: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } - -log "fetching PR diff, body, commits" -DIFF=$(gh pr diff "${PR_NUMBER}") -BODY=$(gh pr view "${PR_NUMBER}" --json body --jq .body) -COMMITS=$(gh pr view "${PR_NUMBER}" --json commits --jq '[.commits[] | {sha: .oid, message: .messageHeadline}]') -CHANGED_FILES=$(gh pr diff "${PR_NUMBER}" --name-only) - -# Tag each changed file with scope tags for judge filtering -TAGGED_FILES="[]" -while IFS= read -r file; do - [[ -z "${file}" ]] && continue - TAGS=() - case "${file}" in - *.yaml|*.yml) TAGS+=("yaml") ;; - esac - case "${file}" in - *.go) TAGS+=("go") ;; - *.py) TAGS+=("py") ;; - *.ts) TAGS+=("ts") ;; - esac - case "${file}" in - CLAUDE.md|AGENTS.md|README.md|docs/*) TAGS+=("durable-doc") ;; - esac - case "${file}" in - .claude/skills/*.md|.claude/skills/*/*.md) TAGS+=("skill-md") ;; - esac - case "${file}" in - clusters/harbor/*) TAGS+=("harbor") ;; - esac - TAGS_JSON=$(printf '%s\n' "${TAGS[@]}" | jq -R . | jq -s .) - TAGGED_FILES=$(echo "${TAGGED_FILES}" | jq --arg path "${file}" --argjson tags "${TAGS_JSON}" '. + [{path: $path, scope_tags: $tags}]') -done <<< "${CHANGED_FILES}" - -# Memory snapshot (read feedback_* entries for the 5 active rules) -MEMORY_DIR="${HOME}/.claude/projects/-Users-brandon-tide-workspace-Tide/memory" -MEMORY="{}" -for entry in feedback_no_cpu_limits feedback_harbor_ecr_convention feedback_narration_comments feedback_temporary_migration_notes feedback_authoritative_voice; do - if [[ -f "${MEMORY_DIR}/${entry}.md" ]]; then - CONTENT=$(cat "${MEMORY_DIR}/${entry}.md") - MEMORY=$(echo "${MEMORY}" | jq --arg k "${entry}" --arg v "${CONTENT}" '. + {($k): $v}') - fi -done - -jq -n \ - --arg pr_number "${PR_NUMBER}" \ - --arg head_sha "${HEAD_SHA}" \ - --arg diff "${DIFF}" \ - --arg body "${BODY}" \ - --argjson commits "${COMMITS}" \ - --argjson changed_files "${TAGGED_FILES}" \ - --argjson memory "${MEMORY}" \ - '{pr_number: $pr_number, head_sha: $head_sha, diff: $diff, body: $body, commits: $commits, changed_files: $changed_files, memory: $memory}' \ - > "${STATE_DIR}/context.json" - -log "context.json written ($(wc -c < "${STATE_DIR}/context.json") bytes)" diff --git a/.claude/skills/pr-quality/scripts/judge-llm.sh b/.claude/skills/pr-quality/scripts/judge-llm.sh deleted file mode 100755 index 5c2fc045..00000000 --- a/.claude/skills/pr-quality/scripts/judge-llm.sh +++ /dev/null @@ -1,73 +0,0 @@ -#!/usr/bin/env bash -# judge-llm.sh — runs an LLM-judged rule with n=3 self-consistency. -# Usage: judge-llm.sh -# -# Stdout: JSON only — { "verdict": "violation"|"no_violation", "findings": [...] } -# Stderr: human-readable logs -# -# This script is the contract surface. Actual LLM dispatch is done by the -# claude-code-action runtime, which loads .claude/skills/pr-quality/references/judges/.md -# as the system prompt + few-shot examples and samples n times at temp=0.3. -# -# Each sample returns a per-judge JSON; the runner aggregates 2/3 agreement and emits -# a single finding per (file, line) tuple if the sample voted "violation" at 2/3 or 3/3. -# -# Stdout JSON shape (final, after aggregation): -# { "verdict": "violation"|"no_violation", "findings": [{span, citation, confidence, severity, mechanism, explanation}] } - -set -euo pipefail - -RULE="${1:?rule_id required}" -N="${2:?n_samples required}" -STATE_DIR="${STATE_DIR:?STATE_DIR required}" -CONTEXT="${STATE_DIR}/context.json" -JUDGE_PROMPT="$(dirname "$0")/../references/judges/${RULE}.md" - -log() { printf '[%s] judge-%s: %s\n' "$(date -u +%FT%TZ)" "${RULE}" "$*" >&2; } - -if [[ ! -f "${JUDGE_PROMPT}" ]]; then - log "judge prompt missing: ${JUDGE_PROMPT}" - echo '{"verdict": "no_violation", "findings": [], "error": "missing_judge_prompt"}' - exit 1 -fi - -# The actual LLM invocation is handled by claude-code-action. In CI the runtime -# expects this script to print structured JSON synthesized from n=${N} samples. -# In v1 we delegate the sampling to the action's built-in dispatch using a marker -# protocol: stdout JSON includes a `_llm_dispatch` block that the runtime expands -# into n samples and replaces with aggregated findings. -# -# For local dev / dry-run, set PR_QUALITY_LOCAL=1 to get a no-op no_violation. - -if [[ "${PR_QUALITY_LOCAL:-0}" == "1" ]]; then - log "PR_QUALITY_LOCAL=1; returning no_violation (no LLM dispatch in local mode)" - echo '{"verdict": "no_violation", "findings": []}' - exit 0 -fi - -# Emit the dispatch marker that the runtime expands. -# Use --rawfile (streams from disk) rather than --arg (passed through argv) so -# large PR diffs don't blow ARG_MAX (~128KB on Linux; PRs >1MB are rare but real). -DIFF_FILE="${STATE_DIR}/diff.txt" -MEMORY_FILE="${STATE_DIR}/memory-${RULE}.txt" - -jq -r '.diff' "${CONTEXT}" > "${DIFF_FILE}" -jq -r --arg k "feedback_${RULE}" '.memory[$k] // ""' "${CONTEXT}" > "${MEMORY_FILE}" - -jq -n \ - --arg rule "${RULE}" \ - --argjson n "${N}" \ - --rawfile prompt "${JUDGE_PROMPT}" \ - --rawfile diff "${DIFF_FILE}" \ - --rawfile memory "${MEMORY_FILE}" \ - '{ - _llm_dispatch: { - rule: $rule, - n: $n, - temperature: 0.3, - consistency_threshold: 2, - prompt: $prompt, - memory: $memory, - diff: $diff - } - }' diff --git a/.claude/skills/pr-quality/scripts/judge-mechanical.sh b/.claude/skills/pr-quality/scripts/judge-mechanical.sh deleted file mode 100755 index d29165a3..00000000 --- a/.claude/skills/pr-quality/scripts/judge-mechanical.sh +++ /dev/null @@ -1,104 +0,0 @@ -#!/usr/bin/env bash -# judge-mechanical.sh — runs the per-rule mechanical predicate. -# Usage: judge-mechanical.sh -# -# Stdout: JSON only — { "verdict": "violation"|"no_violation", "findings": [...] } -# Stderr: human-readable logs - -set -euo pipefail - -RULE="${1:?rule_id required}" -STATE_DIR="${STATE_DIR:?STATE_DIR required}" -CONTEXT="${STATE_DIR}/context.json" - -log() { printf '[%s] judge-%s: %s\n' "$(date -u +%FT%TZ)" "${RULE}" "$*" >&2; } - -# Walk a YAML file for `cpu:` lines inside a `limits:` block at deeper indent. -# Stateful awk tracking indent + parent block. One deterministic code path. -# Prints every match (no early return); multi-container files surface all violations. -# Per PE cross-review: rejected a PyYAML-based path that mis-cited line numbers -# on multi-container files and silently dropped findings after the first hit. -scan_cpu_limits() { - awk ' - function indent(s) { match(s, /^ */); return RLENGTH } - { - line = $0 - # Strip trailing comment - sub(/[ \t]+#.*$/, "", line) - if (line ~ /^[ \t]*limits:[ \t]*$/) { - limits_indent = indent($0) - in_limits = 1 - next - } - if (in_limits && indent($0) <= limits_indent && length(line) > 0) { - in_limits = 0 - } - if (in_limits && match(line, /^[ \t]+cpu:[ \t]*[^[:space:]]/)) { - print FILENAME ":" NR - } - } - ' "$1" -} - -case "${RULE}" in - no_cpu_limits) - FINDINGS="[]" - CHANGED_YAML=$(jq -r '.changed_files[] | select(.scope_tags | contains(["yaml"])) | .path' "${CONTEXT}") - while IFS= read -r file; do - [[ -z "${file}" ]] && continue - [[ ! -f "${file}" ]] && continue - # Skip Kustomization files — they don't declare container resources - if grep -qx 'kind: Kustomization' "${file}" 2>/dev/null; then - continue - fi - MATCHES=$(scan_cpu_limits "${file}") - while IFS= read -r match; do - [[ -z "${match}" ]] && continue - FINDINGS=$(echo "${FINDINGS}" | jq --arg span "${match}" --arg cite "no_cpu_limits" \ - '. + [{verdict: "violation", span: $span, citation: $cite, confidence: "high", severity: "warn", mechanism: "mechanical", explanation: "CPU limit set; remove. Set requests only — throttling is an anti-pattern."}]') - done <<< "${MATCHES}" - done <<< "${CHANGED_YAML}" - COUNT=$(echo "${FINDINGS}" | jq 'length') - log "${COUNT} finding(s)" - if [[ "${COUNT}" -eq 0 ]]; then - echo '{"verdict": "no_violation", "findings": []}' - else - jq -n --argjson f "${FINDINGS}" '{verdict: "violation", findings: $f}' - fi - ;; - - harbor_ecr_convention) - FINDINGS="[]" - DIFF=$(jq -r '.diff' "${CONTEXT}") - # Grep added lines (+) under clusters/harbor/** for ghcr.io. - # State machine over diff: track current `+++ b/` then matched `+ ...ghcr.io...` lines. - HARBOR_MATCHES=$(echo "${DIFF}" | awk ' - /^\+\+\+ b\// { sub(/^\+\+\+ b\//, "", $0); current_file = $0; line_num = 0; next } - /^@@/ { match($0, /\+[0-9]+/); line_num = substr($0, RSTART+1, RLENGTH-1) + 0 - 1; next } - /^\+/ && !/^\+\+\+/ { - line_num++ - if (current_file ~ /^clusters\/harbor\// && /ghcr\.io/) print current_file ":" line_num - next - } - /^[- ]/ { line_num++ } - ') - while IFS= read -r match; do - [[ -z "${match}" ]] && continue - FINDINGS=$(echo "${FINDINGS}" | jq --arg span "${match}" --arg cite "harbor_ecr_convention" \ - '. + [{verdict: "violation", span: $span, citation: $cite, confidence: "high", severity: "warn", mechanism: "mechanical", explanation: "Harbor workload images go to AWS ECR; ghcr.io reference detected."}]') - done <<< "${HARBOR_MATCHES}" - COUNT=$(echo "${FINDINGS}" | jq 'length') - log "${COUNT} finding(s)" - if [[ "${COUNT}" -eq 0 ]]; then - echo '{"verdict": "no_violation", "findings": []}' - else - jq -n --argjson f "${FINDINGS}" '{verdict: "violation", findings: $f}' - fi - ;; - - *) - log "unknown mechanical rule: ${RULE}" - echo '{"verdict": "no_violation", "findings": [], "error": "unknown_rule"}' - exit 1 - ;; -esac diff --git a/.claude/skills/pr-quality/scripts/judge-skill-dispatch.sh b/.claude/skills/pr-quality/scripts/judge-skill-dispatch.sh deleted file mode 100755 index 52a375d6..00000000 --- a/.claude/skills/pr-quality/scripts/judge-skill-dispatch.sh +++ /dev/null @@ -1,60 +0,0 @@ -#!/usr/bin/env bash -# judge-skill-dispatch.sh — composes with another skill by dispatching a subagent. -# Usage: judge-skill-dispatch.sh -# -# v1 only target: brevity_dispatch (loads .claude/skills/brevity/SKILL.md, applies to PR body + comment additions). -# -# Stdout: JSON only. -# Stderr: human-readable logs. - -set -euo pipefail - -TARGET="${1:?dispatch_target required}" -STATE_DIR="${STATE_DIR:?STATE_DIR required}" -CONTEXT="${STATE_DIR}/context.json" - -log() { printf '[%s] judge-skill-%s: %s\n' "$(date -u +%FT%TZ)" "${TARGET}" "$*" >&2; } - -case "${TARGET}" in - brevity_dispatch) - SKILL_PATH=".claude/skills/brevity/SKILL.md" - if [[ ! -f "${SKILL_PATH}" ]]; then - log "brevity skill missing at ${SKILL_PATH}" - echo '{"verdict": "no_violation", "findings": [], "error": "missing_target_skill"}' - exit 1 - fi - BODY=$(jq -r '.body' "${CONTEXT}") - BODY_WORDS=$(echo "${BODY}" | wc -w | tr -d ' ') - # Quick heuristic: if PR body > 250 words, flag for brevity review. - # The LLM-dispatched subagent does the real judgment with /brevity loaded. - if [[ "${BODY_WORDS}" -lt 50 ]]; then - log "PR body ${BODY_WORDS} words; below brevity floor; no finding" - echo '{"verdict": "no_violation", "findings": []}' - exit 0 - fi - # Emit dispatch marker for the runtime to invoke the brevity skill on the body. - jq -n \ - --arg target "${TARGET}" \ - --arg skill_path "${SKILL_PATH}" \ - --arg body "${BODY}" \ - --argjson body_words "${BODY_WORDS}" \ - '{ - _skill_dispatch: { - target: $target, - skill_path: $skill_path, - input_kind: "pr_body", - input: $body, - input_word_count: $body_words, - rule_id_on_violation: "brevity_dispatch", - severity_on_violation: "nudge", - mechanism: "skill-dispatch" - } - }' - ;; - - *) - log "unknown dispatch target: ${TARGET}" - echo '{"verdict": "no_violation", "findings": [], "error": "unknown_target"}' - exit 1 - ;; -esac diff --git a/.claude/skills/pr-quality/scripts/post-or-update.sh b/.claude/skills/pr-quality/scripts/post-or-update.sh deleted file mode 100755 index e4cc8577..00000000 --- a/.claude/skills/pr-quality/scripts/post-or-update.sh +++ /dev/null @@ -1,56 +0,0 @@ -#!/usr/bin/env bash -# post-or-update.sh — idempotent comment posting. -# -# Logic: -# - Find existing bot comment by marker prefix -# - Extract previous findings-hash from marker -# - If empty comment + no prior comment → no-op -# - If empty comment + prior comment exists → DELETE prior (clean PR after fixes) -# - If new comment + no prior → CREATE -# - If new comment + prior + hash matches → no-op (no churn) -# - If new comment + prior + hash differs → PATCH in place - -set -euo pipefail - -PR_NUMBER="${PR_NUMBER:?PR_NUMBER required}" -STATE_DIR="${STATE_DIR:?STATE_DIR required}" -GH_REPO="${GH_REPO:?GH_REPO required (owner/repo)}" - -log() { printf '[%s] post-or-update: %s\n' "$(date -u +%FT%TZ)" "$*" | tee -a "${STATE_DIR}/audit.log"; } - -COMMENT_FILE="${STATE_DIR}/comment.md" -NEW_BODY=$([ -s "${COMMENT_FILE}" ] && cat "${COMMENT_FILE}" || echo "") -NEW_HASH=$(echo "${NEW_BODY}" | grep -oE 'findings-hash=[a-f0-9]+' | head -1 | cut -d= -f2 || echo "") - -# Find prior bot comment by marker prefix -PRIOR=$(gh api "repos/${GH_REPO}/issues/${PR_NUMBER}/comments" \ - --jq '.[] | select(.body | startswith("" - echo "### PR Quality — ${COUNT} finding(s)" - echo - jq -r '.findings[] | "- `\(.span)` — \(.explanation)\n Rule: [`\(.citation)`](.claude/memory/feedback_\(.citation).md) — see memory entry."' "${AGG}" - if [[ "${SUPPRESSED}" -gt 0 ]]; then - echo - echo "
+${SUPPRESSED} additional lower-severity findings suppressed (${SUPPRESSED_RULES})" - echo - echo "Full set in CI artifact \`pr-quality-artifacts\`." - echo "
" - fi - echo - echo "---" - echo - echo "Suggestive only; humans decide. Opt out via label \`skip-pr-quality\`." -} > "${STATE_DIR}/comment.md" - -log "comment.md rendered ($(wc -c < "${STATE_DIR}/comment.md") bytes, ${SUPPRESSED} suppressed)" diff --git a/.claude/skills/pr-quality/scripts/scan-harbor-ghcr.sh b/.claude/skills/pr-quality/scripts/scan-harbor-ghcr.sh new file mode 100755 index 00000000..ba46a7e1 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/scan-harbor-ghcr.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +# scan-harbor-ghcr.sh — emit `:` for ghcr.io occurrences in +# added/modified lines under clusters/harbor/**. +# +# Usage: scan-harbor-ghcr.sh +# OR pipe a unified diff to stdin: gh pr diff | scan-harbor-ghcr.sh +# Stdout: one line per finding. +# +# State machine walks the unified diff: tracks current `+++ b/` and +# `@@ ... +, @@` hunk headers, counts added lines (`+`) and +# context lines (` `) — match emits the path + post-hunk line number. + +set -euo pipefail + +DIFF_INPUT="${1:--}" +awk ' + /^\+\+\+ b\// { + current_file = substr($0, 7) + in_harbor = (current_file ~ /^clusters\/harbor\//) ? 1 : 0 + next + } + /^@@/ { + # Match the +, portion of @@ -a,b +c,d @@ + match($0, /\+[0-9]+/) + if (RSTART > 0) line_num = substr($0, RSTART+1, RLENGTH-1) + 0 - 1 + next + } + /^\+/ && !/^\+\+\+/ { + line_num++ + if (in_harbor && /ghcr\.io/) print current_file ":" line_num + next + } + /^[- ]/ && !/^---/ { line_num++ } +' "${DIFF_INPUT}" diff --git a/.claude/skills/pr-quality/scripts/scan-yaml-cpu.sh b/.claude/skills/pr-quality/scripts/scan-yaml-cpu.sh new file mode 100755 index 00000000..9c4d1877 --- /dev/null +++ b/.claude/skills/pr-quality/scripts/scan-yaml-cpu.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +# scan-yaml-cpu.sh — emit `:` for every cpu: set inside a limits: block. +# +# Usage: scan-yaml-cpu.sh [file2.yaml ...] +# Stdout: one line per finding. Empty stdout = no findings. +# Stderr: human-readable status; non-empty does not signal violation. +# +# Stateful awk: tracks indent + parent-block name. Prints every match (no +# early return) so multi-container files surface all violations. Skips +# Kustomization files (no container resources). + +set -euo pipefail + +for file in "$@"; do + [[ -z "${file}" ]] && continue + [[ ! -f "${file}" ]] && continue + if grep -qx 'kind: Kustomization' "${file}" 2>/dev/null; then + continue + fi + awk ' + function indent(s) { match(s, /^ */); return RLENGTH } + { + line = $0 + # Strip trailing comment + sub(/[ \t]+#.*$/, "", line) + if (line ~ /^[ \t]*limits:[ \t]*$/) { + limits_indent = indent($0) + in_limits = 1 + next + } + if (in_limits && indent($0) <= limits_indent && length(line) > 0) { + in_limits = 0 + } + if (in_limits && match(line, /^[ \t]+cpu:[ \t]*[^[:space:]]/)) { + print FILENAME ":" NR + } + } + ' "${file}" +done diff --git a/.github/workflows/pr-quality.yml b/.github/workflows/pr-quality.yml deleted file mode 100644 index e3fdbe98..00000000 --- a/.github/workflows/pr-quality.yml +++ /dev/null @@ -1,67 +0,0 @@ -name: pr-quality - -on: - pull_request: - types: [opened, synchronize, reopened] - -permissions: - contents: read - pull-requests: write - id-token: write - -concurrency: - group: pr-quality-${{ github.event.pull_request.number }} - cancel-in-progress: true - -jobs: - review: - # Skip on fork PRs — `pull_request` from a fork has a read-only token, so - # `pull-requests: write` is not honored. Posting a comment would silently - # fail. Switching to `pull_request_target` is a security footgun (the fork's - # workflow code would run with write tokens). v1 explicitly skips forks. - if: github.event.pull_request.head.repo.full_name == github.repository - runs-on: ubuntu-latest - timeout-minutes: 10 - steps: - - name: Verify required secrets - env: - ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} - run: | - if [[ -z "${ANTHROPIC_API_KEY:-}" ]]; then - echo "ANTHROPIC_API_KEY secret missing; skipping pr-quality run" - exit 0 - fi - - - name: Checkout - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - - name: Run pr-quality skill via claude-code-action - uses: anthropics/claude-code-action@v1 - with: - anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} - prompt: | - Apply the .claude/skills/pr-quality/ skill against this PR. - Follow the procedure in SKILL.md exactly. Do not invent rules - beyond the v1 registry. Do not post a comment when findings - count is zero. Do not future-proof. - claude_args: | - { - "model": "claude-opus-4-7", - "max_tokens": 8000 - } - env: - PR_NUMBER: ${{ github.event.pull_request.number }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - GH_REPO: ${{ github.repository }} - STATE_DIR: ${{ github.workspace }}/pr-quality-state/run-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} - - - name: Upload pr-quality artifacts - if: always() - uses: actions/upload-artifact@v4 - with: - name: pr-quality-${{ github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} - path: pr-quality-state/ - retention-days: 14 - if-no-files-found: ignore diff --git a/AGENTS.md b/AGENTS.md index 23cb4392..77ac1c5f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,6 +35,8 @@ All design work follows the constitution at `design/constitution/constitution.md **Output discipline.** Every agent in the roster authors PR descriptions and in-code comments. Before any agent ships a PR body or writes a WHY-style comment, it applies `/brevity` (`.claude/skills/brevity/`). The skill's 8 rules + 5-row rationalization table hold against the verbose-by-default biases an LLM agent produces under pressure. The skill self-determines when input is at floor — agents do not pre-skip. Doc/design, runbooks, memory writes, and mid-conversation chat are out of scope for the skill today; see its `references/guardrails.md` for un-defer triggers. +**Pre-PR review.** Before invoking `gh pr create`, every agent applies `/pr-quality` (`.claude/skills/pr-quality/`) to the staged diff + planned body. Findings surface inline for revision; the agent decides what to act on. Suggestive only — no merge gating. Post-PR, the user may invoke `/pr-quality ` to post a fresh comment with findings. v1 covers 2 dimensions (verbosity via `/brevity` dispatch + 5 convention rules); deferred dimensions enumerated in the skill's `references/rule-registry.md`. + ## Cross-Component Interfaces The most critical contracts between components. The **provider owns** the interface; consumers adapt. diff --git a/CLAUDE.md b/CLAUDE.md index 478263a7..ba078c19 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,6 +26,7 @@ The single source of truth for all cross-component interfaces is `tide/interface - **Exit codes:** Granular codes (10-52) from runtimes. Operator groups them for retry/fail decisions. - **Commit style:** Conventional commits (`feat:`, `fix:`, `docs:`, `refactor:`). Reference the component in scope. - **Brevity discipline:** Apply `/brevity` (`.claude/skills/brevity/`) before writing PR bodies or in-code comments. The skill self-determines floor; agents do not pre-skip. +- **PR-quality discipline:** Before invoking `gh pr create`, apply `/pr-quality` (`.claude/skills/pr-quality/`) to the staged diff + planned body. Findings surface inline for revision. Post-PR: invoke `/pr-quality ` to post a fresh comment with findings. (Brevity runs during authoring; pr-quality runs on the final diff — they don't chain.) ## Key File Locations - `design/constitution/constitution.md` — governing document for all design work