From ccef88355362086d784796224eeceb386c3de018 Mon Sep 17 00:00:00 2001 From: GiggleLiu Date: Sun, 15 Mar 2026 15:05:53 +0800 Subject: [PATCH 01/29] refactor(pipeline): script board and PR helpers --- .claude/skills/fix-pr/SKILL.md | 96 +-- .claude/skills/review-pipeline/SKILL.md | 128 ++-- Makefile | 73 +- ...03-15-pipeline-automation-refactor-plan.md | 390 +++++++++++ ...03-15-review-pipeline-closed-pr-cleanup.md | 57 ++ scripts/make_helpers.sh | 33 +- scripts/pipeline_board.py | 639 ++++++++++++++++++ scripts/pipeline_pr.py | 524 ++++++++++++++ scripts/project_board_poll.py | 199 ++---- scripts/project_board_recover.py | 271 +------- scripts/test_make_helpers.py | 128 ++++ scripts/test_pipeline_board.py | 235 +++++++ scripts/test_pipeline_pr.py | 228 +++++++ scripts/test_project_board_poll.py | 78 +++ 14 files changed, 2550 insertions(+), 529 deletions(-) create mode 100644 docs/plans/2026-03-15-pipeline-automation-refactor-plan.md create mode 100644 docs/plans/2026-03-15-review-pipeline-closed-pr-cleanup.md create mode 100644 scripts/pipeline_board.py create mode 100644 scripts/pipeline_pr.py create mode 100644 scripts/test_pipeline_board.py create mode 100644 scripts/test_pipeline_pr.py diff --git a/.claude/skills/fix-pr/SKILL.md b/.claude/skills/fix-pr/SKILL.md index 61149c887..f57d3cb5f 100644 --- a/.claude/skills/fix-pr/SKILL.md +++ b/.claude/skills/fix-pr/SKILL.md @@ -9,9 +9,7 @@ Resolve PR review comments, fix CI failures, and address codecov coverage gaps f ## Step 1: Gather PR State -**IMPORTANT:** Do NOT use `gh api --jq` for extracting data — it uses a built-in jq that -chokes on response bodies containing backslashes (common in Copilot code suggestions). -Always pipe to `python3 -c` instead. (`gh pr view --jq` is fine — only `gh api --jq` is affected.) +Use the shared scripted helpers for deterministic PR metadata, comment, CI, and Codecov collection. Do not rebuild this logic inline with `gh api | python3 -c` unless you are debugging the helper itself. ```bash # Get repo identifiers @@ -20,69 +18,44 @@ REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner) # e.g., "owner/re # Get PR number PR=$(gh pr view --json number --jq .number) -# Get PR head SHA (on remote) -HEAD_SHA=$(gh api repos/$REPO/pulls/$PR | python3 -c "import sys,json; print(json.load(sys.stdin)['head']['sha'])") +# Get structured PR state +SNAPSHOT=$(python3 scripts/pipeline_pr.py snapshot --repo "$REPO" --pr "$PR" --format json) +COMMENTS=$(python3 scripts/pipeline_pr.py comments --repo "$REPO" --pr "$PR" --format json) +CI=$(python3 scripts/pipeline_pr.py ci --repo "$REPO" --pr "$PR" --format json) +CODECOV=$(python3 scripts/pipeline_pr.py codecov --repo "$REPO" --pr "$PR" --format json) + +# Extract the current head SHA when needed +HEAD_SHA=$(printf '%s\n' "$SNAPSHOT" | python3 -c "import sys,json; print(json.load(sys.stdin)['head_sha'])") ``` ### 1a. Fetch Review Comments **Check ALL four sources.** User inline comments are the most commonly missed — do not skip any. -```bash -# 1. Inline review comments on code lines (from ALL reviewers: users AND Copilot) -gh api repos/$REPO/pulls/$PR/comments | python3 -c " -import sys,json -comments = json.load(sys.stdin) -print(f'=== Inline comments: {len(comments)} ===') -for c in comments: - line = c.get('line') or c.get('original_line') or '?' - print(f'[{c[\"user\"][\"login\"]}] {c[\"path\"]}:{line} — {c[\"body\"][:200]}') -" - -# 2. Review-level comments (top-level review body from formal reviews) -gh api repos/$REPO/pulls/$PR/reviews | python3 -c " -import sys,json -reviews = json.load(sys.stdin) -print(f'=== Reviews: {len(reviews)} ===') -for r in reviews: - if r.get('body'): - print(f'[{r[\"user\"][\"login\"]}] {r[\"state\"]}: {r[\"body\"][:200]}') -" - -# 3. Issue-level comments (general discussion) -gh api repos/$REPO/issues/$PR/comments | python3 -c " -import sys,json -comments = [c for c in json.load(sys.stdin) if 'codecov' not in c['user']['login']] -print(f'=== Issue comments: {len(comments)} ===') -for c in comments: - print(f'[{c[\"user\"][\"login\"]}] {c[\"body\"][:200]}') -" -``` +Read the `COMMENTS` JSON and inspect these arrays: +- `inline_comments` — all inline review comments +- `reviews` — top-level review bodies +- `human_issue_comments` — PR conversation comments excluding bots and Codecov +- `human_linked_issue_comments` — linked issue comments excluding bots +- `codecov_comments` — PR conversation comments from Codecov only -**Verify counts:** If any source returns 0, confirm it's genuinely empty — don't assume no feedback exists. +Use `COMMENTS["counts"]` to verify whether any source is genuinely empty before assuming there is no feedback. ### 1b. Check CI Status -```bash -# All check runs on the PR head -gh api repos/$REPO/commits/$HEAD_SHA/check-runs | python3 -c " -import sys,json -for cr in json.load(sys.stdin)['check_runs']: - print(f'{cr[\"name\"]}: {cr.get(\"conclusion\") or cr[\"status\"]}') -" -``` +Read the `CI` JSON. It includes: +- `state` — `pending`, `failure`, or `success` +- `runs` — normalized check-run details +- `pending` / `failing` / `succeeding` counts ### 1c. Check Codecov Report -```bash -# Codecov bot comment with coverage diff -gh api repos/$REPO/issues/$PR/comments | python3 -c " -import sys,json -for c in json.load(sys.stdin): - if c['user']['login'] == 'codecov[bot]': - print(c['body']) -" -``` +Read the `CODECOV` JSON. It includes: +- `found` — whether a Codecov comment is present +- `patch_coverage` +- `project_coverage` +- `filepaths` — deduplicated paths referenced by Codecov links +- `body` — the raw latest Codecov comment body ## Step 2: Triage and Prioritize @@ -130,23 +103,10 @@ Copilot suggestions with `suggestion` blocks contain exact code. Evaluate each: ### 5a. Identify Uncovered Lines -From the codecov bot comment (fetched in Step 1c), extract: +From the `CODECOV` JSON (fetched in Step 1c), extract: - Files with missing coverage - Patch coverage percentage -- Specific uncovered lines (linked in the report) - -For detailed line-by-line coverage, use the Codecov API: - -```bash -# Get file-level coverage for the PR -gh api repos/$REPO/issues/$PR/comments | python3 -c " -import sys,json,re -for c in json.load(sys.stdin): - if c['user']['login'] == 'codecov[bot]': - for m in re.findall(r'filepath=([^&\"]+)', c['body']): - print(m) -" -``` +- Specific uncovered files referenced in `filepaths` Then read the source files and identify which new/changed lines lack test coverage. diff --git a/.claude/skills/review-pipeline/SKILL.md b/.claude/skills/review-pipeline/SKILL.md index 791fa3ce7..b288bf0df 100644 --- a/.claude/skills/review-pipeline/SKILL.md +++ b/.claude/skills/review-pipeline/SKILL.md @@ -34,7 +34,7 @@ GitHub Project board IDs (for `gh project item-edit`): ## Autonomous Mode -This skill runs **fully autonomously** -- no confirmation prompts, no user questions. +This skill runs **fully autonomously** except for one case: if a Review pool card links multiple PRs in `CodingThrust/problem-reductions` and the user did not specify which PR to process, STOP and ask the user which PR is the intended target. ## Steps @@ -46,16 +46,32 @@ gh project item-list 8 --owner CodingThrust --format json --limit 500 Filter items where `status == "Review pool"`. Each item should have an associated PR. Extract the PR number from the item title or linked issue. +Before treating an item as a candidate: + +- Only consider PRs in `CodingThrust/problem-reductions` whose GitHub state is `OPEN`. +- If a Review pool **PR card** points to a PR that is `CLOSED` or `MERGED`, mark it stale and skip it. +- If a Review pool **issue card** has multiple linked repo PRs, treat the card as **ambiguous**. Do not guess which PR to process. +- If a Review pool issue card has exactly one linked repo PR, require that PR to be `OPEN`. +- If a Review pool issue card has no linked repo PRs, resolve the current open PR from the issue number as before. + +Stale links must be cleaned up at the PR level (for example, remove outdated `Closes #...` references from superseded PRs) rather than by moving the board card. + #### 0a. Check Copilot Review Status -For each candidate PR, check whether Copilot has already submitted a review: +For each candidate PR that passed the open/ambiguity checks above, check whether Copilot has already submitted a review: ```bash REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner) gh api repos/$REPO/pulls/$PR/reviews --jq '[.[] | select(.user.login == "copilot-pull-request-reviewer[bot]")] | length' ``` -A PR is **eligible** only if the count is ≥ 1 (Copilot has submitted at least one review). PRs without a Copilot review yet are marked `[waiting for Copilot]` and skipped. +A PR is **eligible** only if: + +1. The PR state is `OPEN` +2. The Review pool card is unambiguous, or the user explicitly chose the PR from an ambiguous card +3. The Copilot review count is ≥ 1 + +PRs without a Copilot review yet are marked `[waiting for Copilot]` and skipped. Closed/merged PRs are marked `[stale closed PR]` and skipped. Mixed-status or multi-PR issue cards are marked `[ambiguous linked PRs]` and require user confirmation before proceeding. #### 0b. Print the List @@ -65,14 +81,28 @@ Print all Review pool items with their Copilot status: Review pool PRs: #570 Fix #117: [Model] GraphPartitioning [copilot reviewed] #571 Fix #97: [Rule] BinPacking to ILP [waiting for Copilot] + #170 Fix #108: [Model] LongestCommonSubsequence [stale closed PR] + issue #108 [Model] LongestCommonSubsequence [ambiguous linked PRs] ``` -**If a specific PR number was provided:** verify it is in the Review pool column. If it is waiting for Copilot, STOP with a message: `PR #N is waiting for Copilot review. Re-run after Copilot has reviewed.` +**If a specific PR number was provided:** verify it is in the Review pool column and is `OPEN`. If it is waiting for Copilot, STOP with a message: `PR #N is waiting for Copilot review. Re-run after Copilot has reviewed.` If it is closed, STOP with: `PR #N is closed and not eligible for review-pipeline.` A specific PR number counts as the user's disambiguation choice for a multi-PR card. -**If `--all`:** process only eligible (Copilot-reviewed) items in order (lowest PR number first). Skip waiting items. +**If `--all`:** process only eligible (open, unambiguous, Copilot-reviewed) items in order (lowest PR number first). Skip waiting, stale, and ambiguous items. **Otherwise:** pick the first eligible item. If no items are eligible, STOP with: `No Review pool PRs have been reviewed by Copilot yet.` +**If the first candidate card is ambiguous:** STOP and ask the user which PR to process. Show short options and recommend the most likely correct one: + +```text +Review pool card [Model] LongestCommonSubsequence links multiple repo PRs: +1. PR #170 — CLOSED — Superseded: Add LongestCommonSubsequence model +2. PR #173 — OPEN — Fix #109: Add LCS to MaximumIndependentSet reduction (Recommended) + +Recommendation rule: +- Prefer the only OPEN repo PR when all others are closed or merged. +- If multiple repo PRs are OPEN, prefer the one whose title/body most directly matches the card's current work, and say that the card still needs stale-link cleanup. +``` + ### 0g. Claim: Move to "Under review" **Immediately** move the chosen PR to the `Under review` column to signal that an agent is actively working on it. This prevents other agents from picking the same PR: @@ -143,10 +173,10 @@ git merge origin/main --no-edit Copilot review is guaranteed to exist (verified in Step 0). Fetch the comments: ```bash -COMMENTS=$(gh api repos/$REPO/pulls/$PR/comments --jq '[.[] | select(.user.login == "copilot-pull-request-reviewer[bot]")]') +COMMENTS=$(python3 scripts/pipeline_pr.py comments --repo "$REPO" --pr "$PR" --format json) ``` -If there are actionable comments: invoke `/fix-pr` to address them, then push: +Inspect the `copilot_inline_comments` array in `COMMENTS`. If there are actionable comments: invoke `/fix-pr` to address them, then push: ```bash git push @@ -156,55 +186,12 @@ If Copilot approved with no actionable comments: skip to next step. ### 2a. Check Issue Comments and Human PR Reviews -Extract the linked issue number from the PR title (pattern: `Fix #N:`): - -```bash -ISSUE=$(gh pr view $PR --json title --jq .title | grep -oP '(?<=Fix #)\d+') -``` - -Fetch all comment sources: - -```bash -# 1. Linked issue comments (from contributors, excluding bots) -if [ -n "$ISSUE" ]; then - gh api repos/$REPO/issues/$ISSUE/comments | python3 -c " -import sys,json -comments = [c for c in json.load(sys.stdin) if not c['user']['login'].endswith('[bot]')] -print(f'=== Issue #{sys.argv[1]} comments: {len(comments)} ===') -for c in comments: - print(f'[{c[\"user\"][\"login\"]}] {c[\"body\"][:300]}') - print('---') -" "$ISSUE" -fi - -# 2. Human PR review comments (inline, excluding Copilot) -gh api repos/$REPO/pulls/$PR/comments | python3 -c " -import sys,json -comments = [c for c in json.load(sys.stdin) if not c['user']['login'].endswith('[bot]')] -print(f'=== Human PR inline comments: {len(comments)} ===') -for c in comments: - line = c.get('line') or c.get('original_line') or '?' - print(f'[{c[\"user\"][\"login\"]}] {c[\"path\"]}:{line} — {c[\"body\"][:300]}') -" - -# 3. Human PR conversation comments (general discussion, excluding bots) -gh api repos/$REPO/issues/$PR/comments | python3 -c " -import sys,json -comments = [c for c in json.load(sys.stdin) if not c['user']['login'].endswith('[bot]')] -print(f'=== Human PR conversation comments: {len(comments)} ===') -for c in comments: - print(f'[{c[\"user\"][\"login\"]}] {c[\"body\"][:300]}') -" - -# 4. Human review-level comments (top-level review body) -gh api repos/$REPO/pulls/$PR/reviews | python3 -c " -import sys,json -reviews = [r for r in json.load(sys.stdin) if not r['user']['login'].endswith('[bot]') and r.get('body')] -print(f'=== Human reviews: {len(reviews)} ===') -for r in reviews: - print(f'[{r[\"user\"][\"login\"]}] {r[\"state\"]}: {r[\"body\"][:300]}') -" -``` +Reuse the `COMMENTS` JSON from Step 2. It already includes: +- `linked_issue_number` +- `human_linked_issue_comments` +- `human_inline_comments` +- `human_issue_comments` +- `human_reviews` For each actionable comment found: @@ -255,26 +242,13 @@ For each retry: 1. **Wait for CI to complete** (poll every 30s, up to 15 minutes): ```bash - for i in $(seq 1 30); do - sleep 30 - HEAD_SHA=$(gh api repos/$REPO/pulls/$PR | python3 -c "import sys,json; print(json.load(sys.stdin)['head']['sha'])") - STATUS=$(gh api repos/$REPO/commits/$HEAD_SHA/check-runs | python3 -c " - import sys,json - runs = json.load(sys.stdin)['check_runs'] - if not runs: - print('PENDING') - else: - failed = [r['name'] for r in runs if r.get('conclusion') not in ('success', 'skipped', None)] - pending = [r['name'] for r in runs if r.get('conclusion') is None and r['status'] != 'completed'] - if pending: - print('PENDING') - elif failed: - print('FAILED') - else: - print('GREEN') - ") - if [ "$STATUS" != "PENDING" ]; then break; fi - done + CI=$(python3 scripts/pipeline_pr.py wait-ci --repo "$REPO" --pr "$PR" --timeout 900 --interval 30 --format json) + STATUS=$(printf '%s\n' "$CI" | python3 -c " +import sys,json +state = json.load(sys.stdin)['state'] +mapping = {'success': 'GREEN', 'failure': 'FAILED', 'timeout': 'FAILED', 'pending': 'PENDING'} +print(mapping.get(state, 'FAILED')) +") ``` - If `GREEN` on the **first** iteration (before any fix-pr): skip the fix loop, done. @@ -354,6 +328,8 @@ Completed: 2/2 | All moved to Final review | Mistake | Fix | |---------|-----| | PR not in Review pool column | Verify status before processing; STOP if not Review pool | +| Processing a closed PR from a stale issue card | Require PR state `OPEN`; skip stale closed PRs | +| Guessing on an issue card with multiple linked repo PRs | Stop, show options to the user, and recommend the most likely correct OPEN PR | | Picking a PR before Copilot has reviewed | Check `pulls/$PR/reviews` for copilot-pull-request-reviewer[bot]; skip if absent | | Missing project scopes | Run `gh auth refresh -s read:project,project` | | Skipping review-implementation | Always run structural completeness check in Step 2b — it catches gaps Copilot misses (paper entries, CLI registration, trait_consistency) | diff --git a/Makefile b/Makefile index d7011b9ea..cc844ada2 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # Makefile for problemreductions -.PHONY: help build test mcp-test fmt clippy doc mdbook paper examples clean coverage rust-export compare qubo-testdata export-schemas release run-plan run-issue run-pipeline run-pipeline-forever run-review run-review-forever diagrams jl-testdata cli cli-demo copilot-review +.PHONY: help build test mcp-test fmt clippy doc mdbook paper examples clean coverage rust-export compare qubo-testdata export-schemas release run-plan run-issue run-pipeline run-pipeline-forever run-review run-review-forever board-next board-ack board-move pr-context pr-wait-ci diagrams jl-testdata cli cli-demo copilot-review RUNNER ?= codex CLAUDE_MODEL ?= opus @@ -37,6 +37,11 @@ help: @echo " run-pipeline-forever - Loop: poll Ready column for new issues, run-pipeline when new ones appear" @echo " run-review [N=] - Pick PR from Review pool, fix comments/CI, run agentic tests" @echo " run-review-forever - Loop: poll Review pool for Copilot-reviewed PRs, run-review when new ones appear" + @echo " board-next MODE= - Get the next eligible queued project item" + @echo " board-ack MODE= ITEM= - Acknowledge a queued project item" + @echo " board-move ITEM= STATUS= - Move a project item to a named status" + @echo " pr-context PR= [REPO=] - Fetch structured PR snapshot JSON" + @echo " pr-wait-ci PR= [REPO=] - Poll CI until terminal state and print JSON" @echo " copilot-review - Request Copilot code review on current PR" @echo "" @echo " Set RUNNER=claude to use Claude instead of Codex (default: codex)" @@ -387,6 +392,72 @@ run-pipeline-forever: @. scripts/make_helpers.sh; \ MAKE=$(MAKE) watch_and_dispatch ready run-pipeline "Ready issues" +# Get the next eligible board item from the scripted queue logic +# Usage: make board-next MODE=ready +# make board-next MODE=review REPO=CodingThrust/problem-reductions +# STATE_FILE=/tmp/custom.json make board-next MODE=ready +board-next: + @if [ -z "$(MODE)" ]; then \ + echo "MODE=ready|review is required"; \ + exit 2; \ + fi + @. scripts/make_helpers.sh; \ + state_file=$${STATE_FILE:-/tmp/problemreductions-$(MODE)-state.json}; \ + if [ "$(MODE)" = "review" ]; then \ + repo=$${REPO:-$$(gh repo view --json nameWithOwner --jq .nameWithOwner)}; \ + poll_project_items "$(MODE)" "$$state_file" "$$repo"; \ + else \ + poll_project_items "$(MODE)" "$$state_file"; \ + fi + +# Advance a scripted board queue after an item is processed +# Usage: make board-ack MODE=ready ITEM=PVTI_xxx +# STATE_FILE=/tmp/custom.json make board-ack MODE=review ITEM=PVTI_xxx +board-ack: + @if [ -z "$(MODE)" ] || [ -z "$(ITEM)" ]; then \ + echo "MODE=ready|review and ITEM= are required"; \ + exit 2; \ + fi + @. scripts/make_helpers.sh; \ + state_file=$${STATE_FILE:-/tmp/problemreductions-$(MODE)-state.json}; \ + ack_polled_item "$$state_file" "$(ITEM)" + +# Move a project board item to a named status through the shared board script +# Usage: make board-move ITEM=PVTI_xxx STATUS=final-review +board-move: + @if [ -z "$(ITEM)" ] || [ -z "$(STATUS)" ]; then \ + echo "ITEM= and STATUS= are required"; \ + exit 2; \ + fi + @. scripts/make_helpers.sh; \ + move_board_item "$(ITEM)" "$(STATUS)" + +# Fetch structured PR snapshot JSON from the shared helper +# Usage: make pr-context PR=570 +# make pr-context PR=570 REPO=CodingThrust/problem-reductions +pr-context: + @if [ -z "$(PR)" ]; then \ + echo "PR= is required"; \ + exit 2; \ + fi + @. scripts/make_helpers.sh; \ + repo=$${REPO:-$$(gh repo view --json nameWithOwner --jq .nameWithOwner)}; \ + pr_snapshot "$$repo" "$(PR)" + +# Poll CI for a PR until it reaches a terminal state +# Usage: make pr-wait-ci PR=570 +# make pr-wait-ci PR=570 TIMEOUT=1200 INTERVAL=15 +pr-wait-ci: + @if [ -z "$(PR)" ]; then \ + echo "PR= is required"; \ + exit 2; \ + fi + @. scripts/make_helpers.sh; \ + repo=$${REPO:-$$(gh repo view --json nameWithOwner --jq .nameWithOwner)}; \ + timeout=$${TIMEOUT:-900}; \ + interval=$${INTERVAL:-30}; \ + pr_wait_ci "$$repo" "$(PR)" "$$timeout" "$$interval" + # Usage: make run-review (picks next Review pool PR automatically) # make run-review N=570 (processes specific PR) # RUNNER=claude make run-review (use Claude instead of Codex) diff --git a/docs/plans/2026-03-15-pipeline-automation-refactor-plan.md b/docs/plans/2026-03-15-pipeline-automation-refactor-plan.md new file mode 100644 index 000000000..2dafe738a --- /dev/null +++ b/docs/plans/2026-03-15-pipeline-automation-refactor-plan.md @@ -0,0 +1,390 @@ +# Pipeline Automation Refactor Plan + +> **For agentic workers:** REQUIRED: Use superpowers:subagent-driven-development (if subagents available) or superpowers:executing-plans to implement this plan. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Extract deterministic pipeline orchestration out of repo-local skills into tested Python helpers and thin `Makefile` entry points, while preserving agent skills for judgment-heavy decisions. + +**Architecture:** Introduce a small `scripts/pipeline_*.py` layer for board selection, PR context, worktree lifecycle, and review checklists. `Makefile` stays the human-facing interface. Skills shrink to the parts that require reasoning: prioritization, research, code changes, semantic review, and maintainer judgment. + +**Tech Stack:** Python 3, `gh` CLI, `unittest`, `Makefile`, repo-local `SKILL.md` docs + +--- + +## Recommended Approach + +### Option A: Keep skills as-is, add a few shell helpers + +- Lowest implementation cost +- Keeps most logic duplicated across `SKILL.md` +- Hard to test and easy to drift + +### Option B: Hybrid scripted orchestration + skill judgment (**Recommended**) + +- Scripts own deterministic logic +- Skills call scripts, interpret results, and make decisions +- Best trade-off between reliability and flexibility + +### Option C: Full scripted pipeline with minimal skills + +- Maximum determinism +- Too rigid for research-heavy issue planning and nuanced final review +- Higher risk of encoding unstable heuristics into scripts + +The rest of this plan assumes **Option B**. + +--- + +## File Structure + +### New scripts + +- Create: `scripts/pipeline_board.py` +- Create: `scripts/pipeline_pr.py` +- Create: `scripts/pipeline_worktree.py` +- Create: `scripts/pipeline_checks.py` + +### Shared helpers + +- Create: `scripts/pipeline_common.py` + +### Tests + +- Create: `scripts/test_pipeline_board.py` +- Create: `scripts/test_pipeline_pr.py` +- Create: `scripts/test_pipeline_worktree.py` +- Create: `scripts/test_pipeline_checks.py` + +### Existing files to shrink + +- Modify: `scripts/project_board_poll.py` +- Modify: `scripts/project_board_recover.py` +- Modify: `scripts/make_helpers.sh` +- Modify: `Makefile` +- Modify: `.claude/skills/project-pipeline/SKILL.md` +- Modify: `.claude/skills/review-pipeline/SKILL.md` +- Modify: `.claude/skills/final-review/SKILL.md` +- Modify: `.claude/skills/issue-to-pr/SKILL.md` +- Modify: `.claude/skills/fix-pr/SKILL.md` +- Modify: `.claude/skills/review-implementation/SKILL.md` + +--- + +## Script APIs + +### `scripts/pipeline_board.py` + +Purpose: all project-board reads, selection, and status transitions. + +Subcommands: + +- `next-ready --repo [--issue ] [--format json]` +- `next-review --repo [--pr ] [--allow-ambiguous skip|report] [--format json]` +- `next-final-review --repo [--pr ] [--format json]` +- `move --item-id --status ` +- `claim-review --item-id ` +- `recover --repo [--apply]` + +Output contract: + +- Always emit stable JSON in machine mode. +- Include `item_id`, `status`, `title`, `issue_number`, `pr_number`, `reason`, `ambiguity`, and `recommendation` when relevant. + +Notes: + +- Fold the useful logic from `project_board_poll.py` and `project_board_recover.py` into this file. +- Keep `project_board_poll.py` as a thin compatibility wrapper or delete it after migration. + +### `scripts/pipeline_pr.py` + +Purpose: one-shot PR snapshots and repeated PR-related queries. + +Subcommands: + +- `snapshot --repo --pr --format json` +- `comments --repo --pr --format json` +- `ci --repo --pr --format json` +- `wait-ci --repo --pr [--timeout 900] [--interval 30] --format json` +- `codecov --repo --pr --format json` +- `linked-issue --repo --pr --format json` +- `edit-body --repo --pr --body-file ` +- `comment --repo --pr --body-file ` +- `close --repo --pr --comment-file ` + +Output contract: + +- `snapshot` should include title, body, state, mergeability, linked issue, changed files, review counts, CI summary, and Codecov summary. + +### `scripts/pipeline_worktree.py` + +Purpose: worktree creation, PR checkout, merge-from-main, and cleanup. + +Subcommands: + +- `create-issue --issue --slug --base origin/main --format json` +- `checkout-pr --repo --pr --format json` +- `merge-main --worktree --format json` +- `cleanup --worktree ` + +Output contract: + +- Return `worktree_dir`, `branch`, `base_sha`, `head_sha`. +- `merge-main` returns `status=clean|conflicted|aborted`, plus `conflicts` and a `likely_complex` boolean. + +### `scripts/pipeline_checks.py` + +Purpose: deterministic review and completeness checks that skills currently describe in prose. + +Subcommands: + +- `detect-scope --base --head --format json` +- `file-whitelist --kind model|rule --files-file --format json` +- `completeness --kind model|rule --name [--source --target ] --format json` +- `issue-guards --repo --issue --format json` + +Output contract: + +- `detect-scope` returns review type, problem/rule identifiers, and changed file lists. +- `file-whitelist` returns out-of-policy files with reasons. +- `completeness` returns pass/fail checks for registrations, tests, paper entries, and trait consistency. + +### `scripts/pipeline_common.py` + +Purpose: shared `gh` runner, JSON decoding, project constants, and common parsing. + +Keep this small: + +- `run_gh_json(...)` +- status option IDs +- PR/issue/title parsing helpers +- common error formatting + +--- + +## Makefile Changes + +Keep `Makefile` as the user entrypoint, but stop encoding workflow logic in shell prompts. + +### New targets + +- `make board-next MODE=ready` +- `make board-next MODE=review` +- `make board-move ITEM= STATUS=` +- `make pr-context PR=` +- `make pr-wait-ci PR=` +- `make worktree-pr PR=` + +### Existing targets to keep + +- `make run-pipeline` +- `make run-review` +- `make run-review-forever` + +### Change in responsibility + +- `run-pipeline` and `run-review` should call scripts first, then pass structured context into the agent prompt. +- `run-review-forever` should continue using queue state, but the eligibility logic should live in `pipeline_board.py`, not in shell. + +--- + +## Skill Shrink Map + +### `project-pipeline` + +Shrink these sections: + +- Replace discovery and raw filtering from `.claude/skills/project-pipeline/SKILL.md:39` +- Replace board move snippets from `.claude/skills/project-pipeline/SKILL.md:123` +- Replace worktree boilerplate from `.claude/skills/project-pipeline/SKILL.md:107` + +Keep: + +- C1/C2 scoring judgment from `.claude/skills/project-pipeline/SKILL.md:65` +- decision to proceed with a ranked issue +- delegation to `issue-to-pr --execute` + +New skill shape: + +- Call `pipeline_board.py next-ready` +- Read JSON candidates +- Apply judgment-only ranking +- Call `pipeline_worktree.py create-issue` +- Call `pipeline_board.py move` +- Continue with `issue-to-pr` + +### `review-pipeline` + +Shrink these sections: + +- Replace candidate discovery and stale/ambiguity filtering from `.claude/skills/review-pipeline/SKILL.md:41` +- Replace claim/move snippets from `.claude/skills/review-pipeline/SKILL.md:106` +- Replace worktree checkout and merge boilerplate from `.claude/skills/review-pipeline/SKILL.md:120` +- Replace comment harvesting shell blocks from `.claude/skills/review-pipeline/SKILL.md:187` +- Replace CI polling loop later in the file with `pipeline_pr.py wait-ci` + +Keep: + +- how to interpret Copilot and human comments +- whether a suggested fix is technically correct +- invoking `fix-pr`, `review-implementation`, and `agentic-tests:test-feature` + +New skill shape: + +- Call `pipeline_board.py next-review` +- If `ambiguity.kind != none`, ask the user and present the script's recommendation +- Call `pipeline_worktree.py checkout-pr` +- Call `pipeline_worktree.py merge-main` +- Call `pipeline_pr.py comments` +- Make judgment calls and edit code + +### `final-review` + +Shrink these sections: + +- Replace final-review PR discovery from `.claude/skills/final-review/SKILL.md:31` +- Replace PR metadata gathering from `.claude/skills/final-review/SKILL.md:43` +- Replace file whitelist checking from `.claude/skills/final-review/SKILL.md:105` +- Replace board move snippets from `.claude/skills/final-review/SKILL.md:232` + +Keep: + +- usefulness assessment +- safety interpretation +- completeness judgment +- quality percentile and merge/on-hold recommendation + +New skill shape: + +- Call `pipeline_board.py next-final-review` +- Call `pipeline_pr.py snapshot` +- Call `pipeline_checks.py file-whitelist` +- Present structured context to the maintainer + +### `issue-to-pr` + +Shrink these sections: + +- Replace input normalization and issue guards from `.claude/skills/issue-to-pr/SKILL.md:30` +- Replace model-existence guard from `.claude/skills/issue-to-pr/SKILL.md:59` +- Replace PR resume/create mechanics from `.claude/skills/issue-to-pr/SKILL.md:107` +- Replace plan-file cleanup mechanics from `.claude/skills/issue-to-pr/SKILL.md:180` + +Keep: + +- reference research +- implementation-plan authoring +- deciding how to batch plan steps +- subagent-driven execution + +New skill shape: + +- Call `pipeline_checks.py issue-guards` +- Call `pipeline_worktree.py create-issue` +- Call `pipeline_pr.py snapshot` when resuming an existing PR +- Keep the agent focused on research and plan quality + +### `fix-pr` + +Shrink these sections: + +- Replace PR snapshot gathering from `.claude/skills/fix-pr/SKILL.md:10` +- Replace comments aggregation from `.claude/skills/fix-pr/SKILL.md:27` +- Replace CI and Codecov extraction from `.claude/skills/fix-pr/SKILL.md:64` + +Keep: + +- triage order +- whether a review comment is correct +- what code/test changes to make + +### `review-implementation` + +Shrink these sections: + +- Replace change-scope detection from `.claude/skills/review-implementation/SKILL.md:19` +- Replace linked-issue context fetch from `.claude/skills/review-implementation/SKILL.md:53` + +Keep: + +- dispatching subagents +- interpreting structural and semantic findings +- deciding which findings to auto-fix + +--- + +## Migration Sequence + +### Task 1: Consolidate board logic + +- [x] Move reusable logic from `project_board_poll.py` and `project_board_recover.py` into `pipeline_board.py` +- [x] Keep old scripts as wrappers during migration +- [x] Add tests for ready/review/final-review selection and status moves + +### Task 2: Add PR snapshot script + +- [x] Implement `pipeline_pr.py snapshot/comments/ci/wait-ci/codecov` +- [x] Add fixtures for PR metadata and comment parsing +- [x] Update `fix-pr` and `review-pipeline` docs to call the script + +### Task 3: Add worktree script + +- [ ] Implement PR checkout from forks using pull refs +- [ ] Implement merge-main with structured conflict output +- [ ] Add tests around path/branch naming and result formatting + +### Task 4: Add deterministic review checks + +- [ ] Implement `detect-scope` +- [ ] Implement `file-whitelist` +- [ ] Implement `completeness` +- [ ] Point `final-review` and `review-implementation` at the new checks + +### Task 5: Thin the skills + +- [ ] Rewrite `project-pipeline` to call scripts for board/worktree actions +- [ ] Rewrite `review-pipeline` to call scripts for board selection, PR context, CI wait, and cleanup +- [ ] Rewrite `final-review`, `issue-to-pr`, and `fix-pr` similarly +- [ ] Remove long shell snippets that duplicate script behavior + +### Task 6: Makefile cleanup + +- [ ] Add board/pr/worktree helper targets +- [ ] Keep existing public targets stable +- [ ] Make prompt generation consume script output instead of duplicating selection logic in prose + +--- + +## Testing Strategy + +- `scripts/test_pipeline_board.py` + Covers ready/review/final selection, stale PRs, ambiguous cards, status transitions. +- `scripts/test_pipeline_pr.py` + Covers comment parsing, linked-issue extraction, CI summary, Codecov parsing. +- `scripts/test_pipeline_worktree.py` + Covers branch naming, pull-ref checkout planning, merge-main result formatting. +- `scripts/test_pipeline_checks.py` + Covers scope detection, whitelist results, completeness checklist generation. + +Verification commands: + +- `python3 -m unittest scripts/test_pipeline_board.py` +- `python3 -m unittest scripts/test_pipeline_pr.py` +- `python3 -m unittest scripts/test_pipeline_worktree.py` +- `python3 -m unittest scripts/test_pipeline_checks.py` + +--- + +## Risks and Constraints + +- Do not encode subjective ranking or usefulness heuristics too aggressively in scripts. +- Keep script output JSON stable so skills and `Makefile` targets do not drift. +- Preserve current public commands (`make run-pipeline`, `make run-review`) so maintainers do not have to relearn the interface. +- Maintain a strict boundary: scripts gather facts, skills make judgments. + +--- + +## Expected End State + +- Skills are shorter, easier to read, and focused on reasoning. +- Python scripts are testable and reusable from both `Makefile` and skills. +- `Makefile` remains the single ergonomic entrypoint for humans. +- Board selection and PR context gathering stop being duplicated across multiple `SKILL.md` files. diff --git a/docs/plans/2026-03-15-review-pipeline-closed-pr-cleanup.md b/docs/plans/2026-03-15-review-pipeline-closed-pr-cleanup.md new file mode 100644 index 000000000..83e2a87af --- /dev/null +++ b/docs/plans/2026-03-15-review-pipeline-closed-pr-cleanup.md @@ -0,0 +1,57 @@ +# Review Pipeline Closed-PR Cleanup Implementation Plan + +> **For agentic workers:** REQUIRED: Use superpowers:subagent-driven-development (if subagents available) or superpowers:executing-plans to implement this plan. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Remove stale closed-PR links that confuse board selection, make review selection skip closed or mixed-status cards, and clean up the leftover local review worktree. + +**Architecture:** Keep the operational cleanup separate from repository behavior changes. The durable fix lives in the review poller and the repo-local `review-pipeline` skill so both the forever poller and manual review runs reject stale or ambiguous Review pool cards. + +**Tech Stack:** GitHub CLI, Python, unittest, repo-local skill docs + +--- + +### Task 1: Add failing review-poller tests + +**Files:** +- Modify: `scripts/test_project_board_poll.py` + +- [ ] Add a test showing a Review pool PR card with a closed PR is skipped. +- [ ] Add a test showing a Review pool issue card with mixed linked PR states is skipped. +- [ ] Run the targeted test file and confirm the new assertions fail before implementation. + +### Task 2: Implement minimal poller fix + +**Files:** +- Modify: `scripts/project_board_poll.py` + +- [ ] Add minimal PR-state inspection helpers needed by review mode. +- [ ] Reject closed PR cards in review mode. +- [ ] Reject issue cards whose linked repo PRs are mixed-status or otherwise ambiguous. +- [ ] Re-run the targeted poller tests and confirm they pass. + +### Task 3: Update manual review-pipeline instructions + +**Files:** +- Modify: `.claude/skills/review-pipeline/SKILL.md` + +- [ ] Update candidate-discovery guidance to require open PRs only. +- [ ] Add an explicit rule to skip mixed-status cards and cards with multiple linked repo PRs unless the selection is unambiguous. +- [ ] Document the stale-link cleanup expectation so manual review runs match the automated poller. + +### Task 4: Perform operational cleanup + +**Files:** +- None (GitHub/project operations) + +- [ ] Remove stale issue-closing references from closed PRs that should no longer be attached to active board cards. +- [ ] Remove the leftover local worktree from the aborted `review-pipeline` run. +- [ ] Refresh the project board state and confirm the affected card no longer presents the same stale closed-PR selection problem. + +### Task 5: Verify + +**Files:** +- None + +- [ ] Run the targeted poller test file. +- [ ] Re-check the affected PR/card state on GitHub. +- [ ] Summarize what changed and any remaining board ambiguities. diff --git a/scripts/make_helpers.sh b/scripts/make_helpers.sh index ef81e6a9b..37b2b7827 100644 --- a/scripts/make_helpers.sh +++ b/scripts/make_helpers.sh @@ -43,29 +43,46 @@ run_agent() { # --- Project board --- -project_items_json() { - gh project item-list 8 --owner CodingThrust --format json --limit 500 -} - # Detect the next eligible item and preserve retryable state in a queue. # poll_project_items [repo] poll_project_items() { mode=$1 state_file=$2 repo=${3-} - board_json=$(project_items_json) || return $? if [ -n "$repo" ]; then - printf '%s\n' "$board_json" | python3 scripts/project_board_poll.py poll "$mode" "$state_file" --repo "$repo" + python3 scripts/pipeline_board.py next "$mode" "$state_file" --repo "$repo" else - printf '%s\n' "$board_json" | python3 scripts/project_board_poll.py poll "$mode" "$state_file" + python3 scripts/pipeline_board.py next "$mode" "$state_file" fi } ack_polled_item() { state_file=$1 item_id=$2 - python3 scripts/project_board_poll.py ack "$state_file" "$item_id" + python3 scripts/pipeline_board.py ack "$state_file" "$item_id" +} + +move_board_item() { + item_id=$1 + status=$2 + python3 scripts/pipeline_board.py move "$item_id" "$status" +} + +# --- PR helpers --- + +pr_snapshot() { + repo=$1 + pr=$2 + python3 scripts/pipeline_pr.py snapshot --repo "$repo" --pr "$pr" --format json +} + +pr_wait_ci() { + repo=$1 + pr=$2 + timeout=${3:-900} + interval=${4:-30} + python3 scripts/pipeline_pr.py wait-ci --repo "$repo" --pr "$pr" --timeout "$timeout" --interval "$interval" --format json } # Poll a board column and dispatch a make target when new items appear. diff --git a/scripts/pipeline_board.py b/scripts/pipeline_board.py new file mode 100644 index 000000000..0d05bab27 --- /dev/null +++ b/scripts/pipeline_board.py @@ -0,0 +1,639 @@ +#!/usr/bin/env python3 +"""Shared project-board logic for polling, recovery, and board CLI helpers.""" + +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +from collections import Counter +from datetime import datetime, timezone +from pathlib import Path +from typing import Callable + +PROJECT_ID = "PVT_kwDOBrtarc4BRNVy" +STATUS_FIELD_ID = "PVTSSF_lADOBrtarc4BRNVyzg_GmQc" + +COPILOT_REVIEWER = "copilot-pull-request-reviewer[bot]" +COPILOT_REVIEWERS = { + "copilot-pull-request-reviewer", + COPILOT_REVIEWER, +} + +STATUS_BACKLOG = "Backlog" +STATUS_READY = "Ready" +STATUS_REVIEW_POOL = "Review pool" +STATUS_FINAL_REVIEW = "Final review" +STATUS_DONE = "Done" + +STATUS_OPTION_IDS = { + STATUS_BACKLOG: "ab337660", + STATUS_READY: "f37d0d80", + STATUS_REVIEW_POOL: "7082ed60", + STATUS_FINAL_REVIEW: "51a3d8bb", + STATUS_DONE: "6aca54fa", +} + +STATUS_ALIASES = { + "backlog": STATUS_BACKLOG, + "ready": STATUS_READY, + "review-pool": STATUS_REVIEW_POOL, + "review_pool": STATUS_REVIEW_POOL, + "review pool": STATUS_REVIEW_POOL, + "final-review": STATUS_FINAL_REVIEW, + "final_review": STATUS_FINAL_REVIEW, + "final review": STATUS_FINAL_REVIEW, + "done": STATUS_DONE, +} + +FAILURE_LABELS = {"PoorWritten", "Wrong", "Trivial", "Useless"} + + +def run_gh(*args: str) -> str: + return subprocess.check_output(["gh", *args], text=True) + + +def fetch_board_items(owner: str, project_number: int, limit: int) -> dict: + return json.loads( + run_gh( + "project", + "item-list", + str(project_number), + "--owner", + owner, + "--format", + "json", + "--limit", + str(limit), + ) + ) + + +def fetch_pr_reviews(repo: str, pr_number: int) -> list[dict]: + data = json.loads(run_gh("api", f"repos/{repo}/pulls/{pr_number}/reviews")) + if not isinstance(data, list): + raise ValueError(f"Unexpected PR review payload for #{pr_number}: {data!r}") + return data + + +def fetch_pr_state(repo: str, pr_number: int) -> str: + return run_gh( + "pr", + "view", + str(pr_number), + "--repo", + repo, + "--json", + "state", + "--jq", + ".state", + ).strip() + + +def resolve_issue_pr(repo: str, issue_number: int) -> int | None: + data = json.loads( + run_gh( + "pr", + "list", + "-R", + repo, + "--search", + f"Fix #{issue_number} in:title state:open", + "--json", + "number", + "--limit", + "1", + ) + ) + if not data: + return None + return int(data[0]["number"]) + + +def item_identity(item: dict) -> str: + item_id = item.get("id") + if item_id is not None: + return str(item_id) + + content = item.get("content") or {} + number = content.get("number") + item_type = content.get("type", "item") + if number is not None: + return f"{item_type}:{number}" + + title = item.get("title") + if title: + return str(title) + + raise ValueError(f"Board item has no stable identity: {item!r}") + + +def load_state(state_file: Path) -> dict: + if not state_file.exists(): + return {"visible": {}, "pending": []} + + raw = state_file.read_text().strip() + if not raw: + return {"visible": {}, "pending": []} + + data = json.loads(raw) + if not isinstance(data, dict): + raise ValueError(f"State file must contain a JSON object: {state_file}") + + visible = data.get("visible", {}) + pending = data.get("pending", []) + if not isinstance(visible, dict) or not isinstance(pending, list): + raise ValueError(f"Invalid poll state format: {state_file}") + + return {"visible": visible, "pending": [str(item_id) for item_id in pending]} + + +def save_state(state_file: Path, state: dict) -> None: + state_file.parent.mkdir(parents=True, exist_ok=True) + state_file.write_text(json.dumps(state, indent=2, sort_keys=True) + "\n") + + +def has_copilot_review(reviews: list[dict]) -> bool: + return any( + (review.get("author") or review.get("user") or {}).get("login") + in COPILOT_REVIEWERS + for review in reviews + ) + + +def linked_pr_numbers(item: dict, repo: str | None = None) -> list[int]: + urls = item.get("linked pull requests") or [] + numbers: list[int] = [] + + if repo is not None: + prefix = f"https://github.com/{repo}/pull/" + for url in urls: + if not isinstance(url, str) or not url.startswith(prefix): + continue + suffix = url.removeprefix(prefix) + if suffix.isdigit(): + numbers.append(int(suffix)) + return numbers + + for url in urls: + if not isinstance(url, str): + continue + try: + numbers.append(int(url.rstrip("/").split("/")[-1])) + except ValueError: + continue + return numbers + + +def linked_repo_pr_numbers(item: dict, repo: str) -> list[int]: + return linked_pr_numbers(item, repo) + + +def ready_entries(board_data: dict) -> dict[str, dict]: + entries = {} + for item in board_data.get("items", []): + if item.get("status") != STATUS_READY: + continue + + content = item.get("content") or {} + number = content.get("number") + if number is None: + continue + + entries[item_identity(item)] = {"number": int(number)} + return entries + + +def review_entries( + board_data: dict, + repo: str, + review_fetcher: Callable[[str, int], list[dict]], + pr_resolver: Callable[[str, int], int | None] | None, + pr_state_fetcher: Callable[[str, int], str], +) -> dict[str, dict]: + entries = {} + for item in board_data.get("items", []): + if item.get("status") != STATUS_REVIEW_POOL: + continue + + content = item.get("content") or {} + item_type = content.get("type") + number = content.get("number") + if number is None: + continue + + pr_number: int | None + if item_type == "PullRequest": + pr_number = int(number) + if pr_state_fetcher(repo, pr_number) != "OPEN": + continue + elif item_type == "Issue": + linked_numbers = linked_pr_numbers(item, repo) + if len(linked_numbers) > 1: + continue + if len(linked_numbers) == 1: + pr_number = linked_numbers[0] + if pr_state_fetcher(repo, pr_number) != "OPEN": + continue + else: + if pr_resolver is None: + raise ValueError("review mode requires pr_resolver for issue cards without linked PRs") + pr_number = pr_resolver(repo, int(number)) + if pr_number is None: + continue + if pr_state_fetcher(repo, pr_number) != "OPEN": + continue + else: + pr_number = None + + if pr_number is None: + continue + + reviews = review_fetcher(repo, pr_number) + if has_copilot_review(reviews): + entries[item_identity(item)] = {"number": pr_number} + return entries + + +def current_entries( + mode: str, + board_data: dict, + repo: str | None = None, + review_fetcher: Callable[[str, int], list[dict]] | None = None, + pr_resolver: Callable[[str, int], int | None] | None = None, + pr_state_fetcher: Callable[[str, int], str] | None = None, +) -> dict[str, dict]: + if mode == "ready": + return ready_entries(board_data) + if mode == "review": + if repo is None: + raise ValueError("repo is required in review mode") + if review_fetcher is None or pr_state_fetcher is None: + raise ValueError("review mode requires review_fetcher and pr_state_fetcher") + return review_entries( + board_data, + repo, + review_fetcher, + pr_resolver, + pr_state_fetcher, + ) + raise ValueError(f"Unsupported mode: {mode}") + + +def process_snapshot( + mode: str, + board_data: dict, + state_file: Path, + repo: str | None = None, + review_fetcher: Callable[[str, int], list[dict]] | None = None, + pr_resolver: Callable[[str, int], int | None] | None = None, + pr_state_fetcher: Callable[[str, int], str] | None = None, +) -> tuple[str, int] | None: + state = load_state(state_file) + previous_visible = state["visible"] + current_visible = current_entries( + mode, + board_data, + repo, + review_fetcher, + pr_resolver, + pr_state_fetcher, + ) + + pending = [item_id for item_id in state["pending"] if item_id in current_visible] + entered = sorted( + (item_id for item_id in current_visible if item_id not in previous_visible), + key=lambda item_id: (current_visible[item_id]["number"], item_id), + ) + for item_id in entered: + if item_id not in pending: + pending.append(item_id) + + state["visible"] = current_visible + state["pending"] = pending + save_state(state_file, state) + + if not pending: + return None + + item_id = pending[0] + return item_id, int(current_visible[item_id]["number"]) + + +def ack_item(state_file: Path, item_id: str) -> None: + state = load_state(state_file) + state["pending"] = [ + pending_id for pending_id in state["pending"] if pending_id != item_id + ] + save_state(state_file, state) + + +def label_names(issue: dict) -> set[str]: + return {label["name"] for label in issue.get("labels", [])} + + +def is_tracked_issue_title(title: str | None) -> bool: + if not title: + return False + return title.startswith("[Model]") or title.startswith("[Rule]") + + +def all_checks_green(pr: dict) -> bool: + statuses = pr.get("statusCheckRollup") or [] + if not statuses: + return False + + for status in statuses: + typename = status.get("__typename") + if typename == "CheckRun": + if status.get("status") != "COMPLETED": + return False + if status.get("conclusion") not in {"SUCCESS", "SKIPPED", "NEUTRAL"}: + return False + elif typename == "StatusContext": + if status.get("state") != "SUCCESS": + return False + return True + + +def infer_issue_status( + issue: dict, + linked_prs: list[dict], + pr_reviews: dict[int, list[dict]], +) -> tuple[str, str]: + labels = label_names(issue) + merged_prs = [pr for pr in linked_prs if pr.get("mergedAt")] + open_prs = [pr for pr in linked_prs if pr.get("state") == "OPEN"] + + if merged_prs: + pr_numbers = ", ".join(f"#{pr['number']}" for pr in merged_prs) + return STATUS_DONE, f"linked merged PR {pr_numbers}" + + if issue.get("state") == "CLOSED": + return STATUS_DONE, "issue itself is closed" + + if open_prs: + waiting_for_copilot = [ + pr + for pr in open_prs + if not has_copilot_review(pr_reviews.get(int(pr["number"]), [])) + ] + if waiting_for_copilot: + pr_numbers = ", ".join(f"#{pr['number']}" for pr in waiting_for_copilot) + return STATUS_REVIEW_POOL, f"open PR {pr_numbers} waiting for Copilot review" + + green_prs = [pr for pr in open_prs if all_checks_green(pr)] + if len(green_prs) == len(open_prs): + pr_numbers = ", ".join(f"#{pr['number']}" for pr in open_prs) + return STATUS_FINAL_REVIEW, f"Copilot reviewed green open PR {pr_numbers}" + + pr_numbers = ", ".join(f"#{pr['number']}" for pr in open_prs) + return STATUS_REVIEW_POOL, f"open PR {pr_numbers} still implementing or fixing review" + + if "Good" in labels: + return STATUS_READY, 'label "Good" present and no linked PR' + + if labels & FAILURE_LABELS: + bad = ", ".join(sorted(labels & FAILURE_LABELS)) + return STATUS_BACKLOG, f"failure labels present: {bad}" + + return STATUS_BACKLOG, "default backlog: no linked PR and no Ready signal" + + +def build_recovery_plan( + board_data: dict, + issues: list[dict], + prs: list[dict], + pr_reviews: dict[int, list[dict]], +) -> list[dict]: + issues_by_number = {issue["number"]: issue for issue in issues} + prs_by_number = {pr["number"]: pr for pr in prs} + + plan = [] + for item in board_data.get("items", []): + content = item.get("content") or {} + issue_number = content.get("number") + if issue_number is None: + continue + + issue = issues_by_number.get(issue_number) + if issue is None: + continue + + title = content.get("title") or issue.get("title") + if not is_tracked_issue_title(title): + continue + + linked_prs = [ + prs_by_number[pr_number] + for pr_number in linked_pr_numbers(item) + if pr_number in prs_by_number + ] + status_name, reason = infer_issue_status(issue, linked_prs, pr_reviews) + plan.append( + { + "item_id": item["id"], + "issue_number": issue_number, + "title": title, + "current_status": item.get("status"), + "proposed_status": status_name, + "option_id": STATUS_OPTION_IDS[status_name], + "reason": reason, + } + ) + + return sorted(plan, key=lambda entry: entry["issue_number"]) + + +def save_backup( + backup_file: Path, + *, + board_data: dict, + issues: list[dict], + prs: list[dict], + pr_reviews: dict[int, list[dict]], + plan: list[dict], +) -> None: + backup_file.parent.mkdir(parents=True, exist_ok=True) + payload = { + "generated_at": datetime.now(timezone.utc).isoformat(), + "board_data": board_data, + "issues": issues, + "prs": prs, + "pr_reviews": pr_reviews, + "plan": plan, + } + backup_file.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n") + + +def default_backup_path(project_number: int) -> Path: + stamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%SZ") + return Path("/tmp") / f"project-{project_number}-status-recovery-{stamp}.json" + + +def print_summary(plan: list[dict]) -> None: + counts = Counter(entry["proposed_status"] for entry in plan) + print("Proposed status counts:") + for status_name in [ + STATUS_BACKLOG, + STATUS_READY, + STATUS_REVIEW_POOL, + STATUS_FINAL_REVIEW, + STATUS_DONE, + ]: + print(f" {status_name}: {counts.get(status_name, 0)}") + + +def print_examples(plan: list[dict], limit: int = 20) -> None: + print("") + print(f"First {min(limit, len(plan))} assignments:") + for entry in plan[:limit]: + print( + f" #{entry['issue_number']:<4} {entry['proposed_status']:<13} " + f"{entry['reason']} | {entry['title']}" + ) + + +def normalize_status_name(status: str) -> str: + normalized = status.strip() + if normalized in STATUS_OPTION_IDS: + return normalized + + alias = STATUS_ALIASES.get(normalized.lower()) + if alias is None: + choices = ", ".join(sorted(STATUS_OPTION_IDS)) + raise ValueError(f"Unsupported status {status!r}. Expected one of: {choices}") + return alias + + +def move_item( + item_id: str, + status: str, + *, + project_id: str = PROJECT_ID, + field_id: str = STATUS_FIELD_ID, +) -> None: + status_name = normalize_status_name(status) + subprocess.check_call( + [ + "gh", + "project", + "item-edit", + "--project-id", + project_id, + "--id", + item_id, + "--field-id", + field_id, + "--single-select-option-id", + STATUS_OPTION_IDS[status_name], + ] + ) + + +def apply_plan( + plan: list[dict], + *, + project_id: str = PROJECT_ID, + field_id: str = STATUS_FIELD_ID, +) -> int: + changed = 0 + for entry in plan: + if entry["current_status"] == entry["proposed_status"]: + continue + move_item( + entry["item_id"], + entry["proposed_status"], + project_id=project_id, + field_id=field_id, + ) + changed += 1 + return changed + + +def print_next_item( + next_item: tuple[str, int] | None, + *, + mode: str, + fmt: str = "text", +) -> int: + if next_item is None: + return 1 + + item_id, number = next_item + if fmt == "json": + print( + json.dumps( + { + "mode": mode, + "item_id": item_id, + "number": number, + } + ) + ) + else: + print(f"{item_id}\t{number}") + return 0 + + +def parse_args(argv: list[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser(description="Project board automation helpers.") + subparsers = parser.add_subparsers(dest="command", required=True) + + next_parser = subparsers.add_parser("next") + next_parser.add_argument("mode", choices=["ready", "review"]) + next_parser.add_argument("state_file", type=Path) + next_parser.add_argument("--repo") + next_parser.add_argument("--owner", default="CodingThrust") + next_parser.add_argument("--project-number", type=int, default=8) + next_parser.add_argument("--limit", type=int, default=500) + next_parser.add_argument("--format", choices=["text", "json"], default="text") + + ack_parser = subparsers.add_parser("ack") + ack_parser.add_argument("state_file", type=Path) + ack_parser.add_argument("item_id") + + move_parser = subparsers.add_parser("move") + move_parser.add_argument("item_id") + move_parser.add_argument("status") + move_parser.add_argument("--project-id", default=PROJECT_ID) + move_parser.add_argument("--field-id", default=STATUS_FIELD_ID) + + return parser.parse_args(argv) + + +def main(argv: list[str] | None = None) -> int: + args = parse_args(argv or sys.argv[1:]) + + if args.command == "ack": + ack_item(args.state_file, args.item_id) + return 0 + + if args.command == "move": + move_item( + args.item_id, + args.status, + project_id=args.project_id, + field_id=args.field_id, + ) + return 0 + + if args.mode == "review" and not args.repo: + raise SystemExit("--repo is required in review mode") + + board_data = fetch_board_items(args.owner, args.project_number, args.limit) + next_item = process_snapshot( + args.mode, + board_data, + args.state_file, + repo=args.repo, + review_fetcher=fetch_pr_reviews, + pr_resolver=resolve_issue_pr, + pr_state_fetcher=fetch_pr_state, + ) + return print_next_item(next_item, mode=args.mode, fmt=args.format) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/pipeline_pr.py b/scripts/pipeline_pr.py new file mode 100644 index 000000000..b25a8a0d6 --- /dev/null +++ b/scripts/pipeline_pr.py @@ -0,0 +1,524 @@ +#!/usr/bin/env python3 +"""Shared PR metadata, comments, CI, and codecov helpers.""" + +from __future__ import annotations + +import argparse +import json +import re +import subprocess +import sys +import time +from typing import Callable +from urllib.parse import unquote + +COPILOT_REVIEWERS = { + "copilot-pull-request-reviewer", + "copilot-pull-request-reviewer[bot]", +} +CODECOV_REVIEWER = "codecov[bot]" + +_CLOSING_ISSUE_RE = re.compile( + r"(?i)\b(?:fix(?:e[sd])?|close[sd]?|resolve[sd]?)\s+" + r"(?:(?:[-.\w]+/[-.\w]+)#)?(\d+)\b" +) +_GENERIC_ISSUE_RE = re.compile(r"(? str: + return subprocess.check_output(["gh", *args], text=True) + + +def run_gh_json(*args: str): + return json.loads(run_gh(*args)) + + +def login_for(entry: dict) -> str: + return (entry.get("user") or entry.get("author") or {}).get("login", "") + + +def is_bot_login(login: str) -> bool: + return login.endswith("[bot]") or login in COPILOT_REVIEWERS + + +def extract_linked_issue_number(title: str | None, body: str | None) -> int | None: + for text in [body or "", title or ""]: + match = _CLOSING_ISSUE_RE.search(text) + if match: + return int(match.group(1)) + + for text in [body or "", title or ""]: + match = _GENERIC_ISSUE_RE.search(text) + if match: + return int(match.group(1)) + + return None + + +def summarize_comments( + inline_comments: list[dict], + reviews: list[dict], + issue_comments: list[dict], + linked_issue_comments: list[dict] | None = None, +) -> dict: + linked_issue_comments = linked_issue_comments or [] + + normalized_inline = [] + for comment in inline_comments: + login = login_for(comment) + normalized_inline.append( + { + "user": login, + "body": comment.get("body", ""), + "path": comment.get("path"), + "line": comment.get("line") or comment.get("original_line"), + "is_bot": is_bot_login(login), + "is_copilot": login in COPILOT_REVIEWERS, + } + ) + + normalized_reviews = [] + for review in reviews: + login = login_for(review) + normalized_reviews.append( + { + "user": login, + "body": review.get("body", ""), + "state": review.get("state"), + "is_bot": is_bot_login(login), + "is_copilot": login in COPILOT_REVIEWERS, + } + ) + + normalized_issue_comments = [] + for comment in issue_comments: + login = login_for(comment) + normalized_issue_comments.append( + { + "user": login, + "body": comment.get("body", ""), + "is_bot": is_bot_login(login), + "is_codecov": login == CODECOV_REVIEWER, + } + ) + + normalized_linked_issue_comments = [] + for comment in linked_issue_comments: + login = login_for(comment) + normalized_linked_issue_comments.append( + { + "user": login, + "body": comment.get("body", ""), + "is_bot": is_bot_login(login), + } + ) + + human_reviews = [ + review + for review in normalized_reviews + if not review["is_bot"] and review["body"].strip() + ] + codecov_comments = [ + comment for comment in normalized_issue_comments if comment["is_codecov"] + ] + + return { + "inline_comments": normalized_inline, + "reviews": normalized_reviews, + "issue_comments": normalized_issue_comments, + "linked_issue_comments": normalized_linked_issue_comments, + "human_inline_comments": [ + comment for comment in normalized_inline if not comment["is_bot"] + ], + "copilot_inline_comments": [ + comment for comment in normalized_inline if comment["is_copilot"] + ], + "human_reviews": human_reviews, + "human_issue_comments": [ + comment + for comment in normalized_issue_comments + if not comment["is_bot"] and not comment["is_codecov"] + ], + "human_linked_issue_comments": [ + comment + for comment in normalized_linked_issue_comments + if not comment["is_bot"] + ], + "codecov_comments": codecov_comments, + "counts": { + "inline_comments": len(normalized_inline), + "copilot_inline_comments": sum( + 1 for comment in normalized_inline if comment["is_copilot"] + ), + "human_inline_comments": sum( + 1 for comment in normalized_inline if not comment["is_bot"] + ), + "reviews": len(normalized_reviews), + "human_reviews": len(human_reviews), + "issue_comments": len(normalized_issue_comments), + "human_issue_comments": sum( + 1 + for comment in normalized_issue_comments + if not comment["is_bot"] and not comment["is_codecov"] + ), + "linked_issue_comments": len(normalized_linked_issue_comments), + "human_linked_issue_comments": sum( + 1 for comment in normalized_linked_issue_comments if not comment["is_bot"] + ), + "codecov_comments": len(codecov_comments), + }, + } + + +def extract_codecov_summary(issue_comments: list[dict]) -> dict: + codecov_comments = [ + comment for comment in issue_comments if login_for(comment) == CODECOV_REVIEWER + ] + if not codecov_comments: + return { + "found": False, + "body": None, + "patch_coverage": None, + "project_coverage": None, + "filepaths": [], + } + + body = codecov_comments[-1].get("body", "") + patch_match = _PATCH_COVERAGE_RE.search(body) + project_match = _PROJECT_COVERAGE_RE.search(body) + + filepaths: list[str] = [] + seen: set[str] = set() + for encoded in _FILEPATH_RE.findall(body): + path = unquote(encoded) + if path not in seen: + seen.add(path) + filepaths.append(path) + + return { + "found": True, + "body": body, + "patch_coverage": float(patch_match.group(1)) if patch_match else None, + "project_coverage": float(project_match.group(1)) if project_match else None, + "filepaths": filepaths, + } + + +def summarize_check_runs(payload: dict) -> dict: + runs = payload.get("check_runs") or [] + normalized_runs = [] + pending = 0 + failing = 0 + succeeding = 0 + + for run in runs: + status = (run.get("status") or "").lower() + conclusion = run.get("conclusion") + normalized_conclusion = conclusion.lower() if isinstance(conclusion, str) else None + normalized_runs.append( + { + "name": run.get("name"), + "status": status, + "conclusion": normalized_conclusion, + "details_url": run.get("details_url"), + } + ) + + if status != "completed" or normalized_conclusion is None: + pending += 1 + elif normalized_conclusion in {"success", "skipped", "neutral"}: + succeeding += 1 + else: + failing += 1 + + if failing: + state = "failure" + elif pending or not normalized_runs: + state = "pending" + else: + state = "success" + + return { + "state": state, + "total": len(normalized_runs), + "pending": pending, + "failing": failing, + "succeeding": succeeding, + "runs": normalized_runs, + } + + +def build_snapshot( + pr_data: dict, + *, + linked_issue_number: int | None = None, + linked_issue: dict | None = None, + ci_summary: dict | None = None, + codecov_summary: dict | None = None, +) -> dict: + if linked_issue_number is None: + linked_issue_number = extract_linked_issue_number( + pr_data.get("title"), + pr_data.get("body"), + ) + + labels = [label.get("name") for label in pr_data.get("labels", []) if label.get("name")] + files = [ + file_info.get("path") or file_info.get("filename") + for file_info in pr_data.get("files", []) + if file_info.get("path") or file_info.get("filename") + ] + commits = [ + commit.get("oid") or commit.get("commit", {}).get("oid") + for commit in pr_data.get("commits", []) + ] + + return { + "number": pr_data.get("number"), + "title": pr_data.get("title"), + "body": pr_data.get("body"), + "state": pr_data.get("state"), + "url": pr_data.get("url"), + "mergeable": pr_data.get("mergeable"), + "head_ref_name": pr_data.get("headRefName"), + "base_ref_name": pr_data.get("baseRefName"), + "head_sha": pr_data.get("headRefOid"), + "linked_issue_number": linked_issue_number, + "linked_issue": linked_issue, + "labels": labels, + "files": files, + "commits": commits, + "additions": pr_data.get("additions", 0), + "deletions": pr_data.get("deletions", 0), + "ci": ci_summary, + "codecov": codecov_summary, + "counts": { + "labels": len(labels), + "files": len(files), + "commits": len(commits), + }, + } + + +def wait_for_ci( + fetcher: Callable[[], dict], + *, + timeout_seconds: float, + interval_seconds: float, + monotonic_fn: Callable[[], float] = time.monotonic, + sleep_fn: Callable[[float], None] = time.sleep, +) -> dict: + start = monotonic_fn() + attempts = 0 + + while True: + attempts += 1 + summary = dict(fetcher()) + summary.setdefault("state", "pending") + summary["attempts"] = attempts + summary["elapsed_seconds"] = round(monotonic_fn() - start, 3) + + if summary["state"] != "pending": + summary["timed_out"] = False + return summary + + if monotonic_fn() + interval_seconds > start + timeout_seconds: + summary["state"] = "timeout" + summary["timed_out"] = True + return summary + + sleep_fn(interval_seconds) + + +def fetch_pr_data(repo: str, pr_number: int) -> dict: + return run_gh_json( + "pr", + "view", + str(pr_number), + "--repo", + repo, + "--json", + ( + "number,title,body,labels,files,additions,deletions,commits," + "headRefName,baseRefName,headRefOid,url,state,mergeable" + ), + ) + + +def fetch_issue_data(repo: str, issue_number: int) -> dict: + return run_gh_json( + "issue", + "view", + str(issue_number), + "--repo", + repo, + "--json", + "number,title,body,labels,state,url", + ) + + +def fetch_issue_comments(repo: str, issue_number: int) -> list[dict]: + data = run_gh_json("api", f"repos/{repo}/issues/{issue_number}/comments") + if not isinstance(data, list): + raise ValueError(f"Unexpected issue comments payload for #{issue_number}: {data!r}") + return data + + +def fetch_inline_comments(repo: str, pr_number: int) -> list[dict]: + data = run_gh_json("api", f"repos/{repo}/pulls/{pr_number}/comments") + if not isinstance(data, list): + raise ValueError(f"Unexpected inline comments payload for PR #{pr_number}: {data!r}") + return data + + +def fetch_reviews(repo: str, pr_number: int) -> list[dict]: + data = run_gh_json("api", f"repos/{repo}/pulls/{pr_number}/reviews") + if not isinstance(data, list): + raise ValueError(f"Unexpected reviews payload for PR #{pr_number}: {data!r}") + return data + + +def fetch_check_runs(repo: str, head_sha: str) -> dict: + return run_gh_json("api", f"repos/{repo}/commits/{head_sha}/check-runs") + + +def fetch_linked_issue_bundle(repo: str, pr_data: dict) -> tuple[int | None, dict | None]: + issue_number = extract_linked_issue_number(pr_data.get("title"), pr_data.get("body")) + if issue_number is None: + return None, None + return issue_number, fetch_issue_data(repo, issue_number) + + +def fetch_ci_summary(repo: str, pr_number: int, pr_data: dict | None = None) -> dict: + pr_data = pr_data or fetch_pr_data(repo, pr_number) + head_sha = pr_data.get("headRefOid") + if not head_sha: + raise ValueError(f"PR #{pr_number} is missing headRefOid") + + summary = summarize_check_runs(fetch_check_runs(repo, head_sha)) + summary["pr_number"] = pr_number + summary["head_sha"] = head_sha + return summary + + +def build_comments_summary(repo: str, pr_number: int, pr_data: dict | None = None) -> dict: + pr_data = pr_data or fetch_pr_data(repo, pr_number) + linked_issue_number = extract_linked_issue_number( + pr_data.get("title"), + pr_data.get("body"), + ) + + summary = summarize_comments( + inline_comments=fetch_inline_comments(repo, pr_number), + reviews=fetch_reviews(repo, pr_number), + issue_comments=fetch_issue_comments(repo, pr_number), + linked_issue_comments=( + fetch_issue_comments(repo, linked_issue_number) + if linked_issue_number is not None + else [] + ), + ) + summary["pr_number"] = pr_number + summary["linked_issue_number"] = linked_issue_number + return summary + + +def build_codecov_summary(repo: str, pr_number: int) -> dict: + summary = extract_codecov_summary(fetch_issue_comments(repo, pr_number)) + summary["pr_number"] = pr_number + return summary + + +def build_pr_snapshot(repo: str, pr_number: int) -> dict: + pr_data = fetch_pr_data(repo, pr_number) + linked_issue_number, linked_issue = fetch_linked_issue_bundle(repo, pr_data) + return build_snapshot( + pr_data, + linked_issue_number=linked_issue_number, + linked_issue=linked_issue, + ci_summary=fetch_ci_summary(repo, pr_number, pr_data), + codecov_summary=build_codecov_summary(repo, pr_number), + ) + + +def emit_result(result: dict, fmt: str) -> None: + if fmt == "json": + print(json.dumps(result, indent=2, sort_keys=True)) + return + + if "number" in result: + print(f"PR #{result['number']}: {result.get('title', '')}") + return + + print(json.dumps(result, indent=2, sort_keys=True)) + + +def parse_args(argv: list[str]) -> argparse.Namespace: + parser = argparse.ArgumentParser(description="PR automation helpers.") + subparsers = parser.add_subparsers(dest="command", required=True) + + for name in ["snapshot", "comments", "ci", "wait-ci", "codecov", "linked-issue"]: + command = subparsers.add_parser(name) + command.add_argument("--repo", required=True) + command.add_argument("--pr", required=True, type=int) + command.add_argument("--format", choices=["json", "text"], default="json") + if name == "wait-ci": + command.add_argument("--timeout", type=float, default=900) + command.add_argument("--interval", type=float, default=30) + + return parser.parse_args(argv) + + +def main(argv: list[str] | None = None) -> int: + args = parse_args(argv or sys.argv[1:]) + + if args.command == "snapshot": + emit_result(build_pr_snapshot(args.repo, args.pr), args.format) + return 0 + + if args.command == "comments": + emit_result(build_comments_summary(args.repo, args.pr), args.format) + return 0 + + if args.command == "ci": + emit_result(fetch_ci_summary(args.repo, args.pr), args.format) + return 0 + + if args.command == "wait-ci": + result = wait_for_ci( + lambda: fetch_ci_summary(args.repo, args.pr), + timeout_seconds=args.timeout, + interval_seconds=args.interval, + ) + emit_result(result, args.format) + return 0 + + if args.command == "codecov": + emit_result(build_codecov_summary(args.repo, args.pr), args.format) + return 0 + + if args.command == "linked-issue": + pr_data = fetch_pr_data(args.repo, args.pr) + issue_number, issue = fetch_linked_issue_bundle(args.repo, pr_data) + emit_result( + { + "pr_number": args.pr, + "linked_issue_number": issue_number, + "linked_issue": issue, + }, + args.format, + ) + return 0 + + raise AssertionError(f"Unhandled command: {args.command}") + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/project_board_poll.py b/scripts/project_board_poll.py index 6df5ebac4..92c628334 100644 --- a/scripts/project_board_poll.py +++ b/scripts/project_board_poll.py @@ -1,59 +1,23 @@ #!/usr/bin/env python3 -"""Track eligible project-board items and expose a retryable pending queue.""" +"""Compatibility wrapper for the board poller CLI.""" from __future__ import annotations import argparse -import json import subprocess import sys from pathlib import Path -from typing import Callable -COPILOT_REVIEWER = "copilot-pull-request-reviewer[bot]" - - -def item_identity(item: dict) -> str: - item_id = item.get("id") - if item_id is not None: - return str(item_id) - - content = item.get("content") or {} - number = content.get("number") - item_type = content.get("type", "item") - if number is not None: - return f"{item_type}:{number}" - - title = item.get("title") - if title: - return str(title) - - raise ValueError(f"Board item has no stable identity: {item!r}") - - -def load_state(state_file: Path) -> dict: - if not state_file.exists(): - return {"visible": {}, "pending": []} - - raw = state_file.read_text().strip() - if not raw: - return {"visible": {}, "pending": []} - - data = json.loads(raw) - if not isinstance(data, dict): - raise ValueError(f"State file must contain a JSON object: {state_file}") +import pipeline_board - visible = data.get("visible", {}) - pending = data.get("pending", []) - if not isinstance(visible, dict) or not isinstance(pending, list): - raise ValueError(f"Invalid poll state format: {state_file}") +item_identity = pipeline_board.item_identity +load_state = pipeline_board.load_state +save_state = pipeline_board.save_state +has_copilot_review = pipeline_board.has_copilot_review +ready_entries = pipeline_board.ready_entries +ack_item = pipeline_board.ack_item - return {"visible": visible, "pending": [str(item_id) for item_id in pending]} - - -def save_state(state_file: Path, state: dict) -> None: - state_file.parent.mkdir(parents=True, exist_ok=True) - state_file.write_text(json.dumps(state, indent=2, sort_keys=True) + "\n") +COPILOT_REVIEWER = "copilot-pull-request-reviewer[bot]" def fetch_pr_reviews(repo: str, pr_number: int) -> list[dict]: @@ -61,12 +25,30 @@ def fetch_pr_reviews(repo: str, pr_number: int) -> list[dict]: ["gh", "api", f"repos/{repo}/pulls/{pr_number}/reviews"], text=True, ) - data = json.loads(output) + data = pipeline_board.json.loads(output) if not isinstance(data, list): raise ValueError(f"Unexpected PR review payload for #{pr_number}: {data!r}") return data +def fetch_pr_state(repo: str, pr_number: int) -> str: + return subprocess.check_output( + [ + "gh", + "pr", + "view", + str(pr_number), + "--repo", + repo, + "--json", + "state", + "--jq", + ".state", + ], + text=True, + ).strip() + + def resolve_issue_pr(repo: str, issue_number: int) -> int | None: output = subprocess.check_output( [ @@ -84,81 +66,48 @@ def resolve_issue_pr(repo: str, issue_number: int) -> int | None: ], text=True, ) - data = json.loads(output) + data = pipeline_board.json.loads(output) if not data: return None return int(data[0]["number"]) -def has_copilot_review(reviews: list[dict]) -> bool: - return any( - review.get("user", {}).get("login") == COPILOT_REVIEWER for review in reviews - ) - - -def ready_entries(board_data: dict) -> dict[str, dict]: - entries = {} - for item in board_data.get("items", []): - if item.get("status") != "Ready": - continue - - content = item.get("content") or {} - number = content.get("number") - if number is None: - continue - - entries[item_identity(item)] = {"number": int(number)} - return entries +def linked_repo_pr_numbers(item: dict, repo: str) -> list[int]: + return pipeline_board.linked_repo_pr_numbers(item, repo) def review_entries( board_data: dict, repo: str, - review_fetcher: Callable[[str, int], list[dict]] = fetch_pr_reviews, - pr_resolver: Callable[[str, int], int | None] = resolve_issue_pr, + review_fetcher=fetch_pr_reviews, + pr_resolver=resolve_issue_pr, + pr_state_fetcher=fetch_pr_state, ) -> dict[str, dict]: - entries = {} - for item in board_data.get("items", []): - if item.get("status") != "Review pool": - continue - - content = item.get("content") or {} - item_type = content.get("type") - number = content.get("number") - if number is None: - continue - - pr_number: int | None - if item_type == "PullRequest": - pr_number = int(number) - elif item_type == "Issue": - pr_number = pr_resolver(repo, int(number)) - else: - pr_number = None - - if pr_number is None: - continue - - reviews = review_fetcher(repo, pr_number) - if has_copilot_review(reviews): - entries[item_identity(item)] = {"number": pr_number} - return entries + return pipeline_board.review_entries( + board_data, + repo, + review_fetcher, + pr_resolver, + pr_state_fetcher, + ) def current_entries( mode: str, board_data: dict, repo: str | None = None, - review_fetcher: Callable[[str, int], list[dict]] = fetch_pr_reviews, - pr_resolver: Callable[[str, int], int | None] = resolve_issue_pr, + review_fetcher=fetch_pr_reviews, + pr_resolver=resolve_issue_pr, + pr_state_fetcher=fetch_pr_state, ) -> dict[str, dict]: - if mode == "ready": - return ready_entries(board_data) - if mode == "review": - if repo is None: - raise ValueError("repo is required in review mode") - return review_entries(board_data, repo, review_fetcher, pr_resolver) - raise ValueError(f"Unsupported mode: {mode}") + return pipeline_board.current_entries( + mode, + board_data, + repo, + review_fetcher, + pr_resolver, + pr_state_fetcher, + ) def process_snapshot( @@ -166,39 +115,19 @@ def process_snapshot( board_data: dict, state_file: Path, repo: str | None = None, - review_fetcher: Callable[[str, int], list[dict]] = fetch_pr_reviews, - pr_resolver: Callable[[str, int], int | None] = resolve_issue_pr, + review_fetcher=fetch_pr_reviews, + pr_resolver=resolve_issue_pr, + pr_state_fetcher=fetch_pr_state, ) -> tuple[str, int] | None: - state = load_state(state_file) - previous_visible = state["visible"] - current_visible = current_entries(mode, board_data, repo, review_fetcher, pr_resolver) - - pending = [item_id for item_id in state["pending"] if item_id in current_visible] - entered = sorted( - (item_id for item_id in current_visible if item_id not in previous_visible), - key=lambda item_id: (current_visible[item_id]["number"], item_id), + return pipeline_board.process_snapshot( + mode, + board_data, + state_file, + repo, + review_fetcher, + pr_resolver, + pr_state_fetcher, ) - for item_id in entered: - if item_id not in pending: - pending.append(item_id) - - state["visible"] = current_visible - state["pending"] = pending - save_state(state_file, state) - - if not pending: - return None - - item_id = pending[0] - return item_id, int(current_visible[item_id]["number"]) - - -def ack_item(state_file: Path, item_id: str) -> None: - state = load_state(state_file) - state["pending"] = [ - pending_id for pending_id in state["pending"] if pending_id != item_id - ] - save_state(state_file, state) def parse_args(argv: list[str]) -> argparse.Namespace: @@ -229,7 +158,7 @@ def main(argv: list[str] | None = None) -> int: if args.mode == "review" and not args.repo: raise SystemExit("--repo is required in review mode") - board_data = json.load(sys.stdin) + board_data = pipeline_board.json.load(sys.stdin) next_item = process_snapshot( args.mode, board_data, diff --git a/scripts/project_board_recover.py b/scripts/project_board_recover.py index c77b27692..c0eb74ae6 100644 --- a/scripts/project_board_recover.py +++ b/scripts/project_board_recover.py @@ -1,38 +1,37 @@ #!/usr/bin/env python3 -"""Recover GitHub Project board statuses after the Status field was recreated.""" +"""Compatibility wrapper for project-board status recovery.""" from __future__ import annotations import argparse -import json import subprocess import sys -from collections import Counter -from datetime import datetime, timezone from pathlib import Path +import pipeline_board + PROJECT_ID = "PVT_kwDOBrtarc4BRNVy" STATUS_FIELD_ID = "PVTSSF_lADOBrtarc4BRNVyzg_GmQc" -STATUS_BACKLOG = "Backlog" -STATUS_READY = "Ready" -STATUS_REVIEW_POOL = "Review pool" -STATUS_FINAL_REVIEW = "Final review" -STATUS_DONE = "Done" - -STATUS_OPTION_IDS = { - STATUS_BACKLOG: "ab337660", - STATUS_READY: "f37d0d80", - STATUS_REVIEW_POOL: "7082ed60", - STATUS_FINAL_REVIEW: "51a3d8bb", - STATUS_DONE: "6aca54fa", -} - -FAILURE_LABELS = {"PoorWritten", "Wrong", "Trivial", "Useless"} -COPILOT_REVIEWERS = { - "copilot-pull-request-reviewer", - "copilot-pull-request-reviewer[bot]", -} +STATUS_BACKLOG = pipeline_board.STATUS_BACKLOG +STATUS_READY = pipeline_board.STATUS_READY +STATUS_REVIEW_POOL = pipeline_board.STATUS_REVIEW_POOL +STATUS_FINAL_REVIEW = pipeline_board.STATUS_FINAL_REVIEW +STATUS_DONE = pipeline_board.STATUS_DONE +STATUS_OPTION_IDS = pipeline_board.STATUS_OPTION_IDS +FAILURE_LABELS = pipeline_board.FAILURE_LABELS +COPILOT_REVIEWERS = pipeline_board.COPILOT_REVIEWERS +label_names = pipeline_board.label_names +linked_pr_numbers = pipeline_board.linked_pr_numbers +is_tracked_issue_title = pipeline_board.is_tracked_issue_title +has_copilot_review = pipeline_board.has_copilot_review +all_checks_green = pipeline_board.all_checks_green +infer_issue_status = pipeline_board.infer_issue_status +build_recovery_plan = pipeline_board.build_recovery_plan +apply_plan = pipeline_board.apply_plan +save_backup = pipeline_board.save_backup +print_summary = pipeline_board.print_summary +print_examples = pipeline_board.print_examples def run_gh(*args: str) -> str: @@ -40,7 +39,7 @@ def run_gh(*args: str) -> str: def fetch_board_items(owner: str, project_number: int, limit: int) -> dict: - return json.loads( + return pipeline_board.json.loads( run_gh( "project", "item-list", @@ -56,7 +55,7 @@ def fetch_board_items(owner: str, project_number: int, limit: int) -> dict: def fetch_issues(repo: str, limit: int) -> list[dict]: - return json.loads( + return pipeline_board.json.loads( run_gh( "issue", "list", @@ -73,7 +72,7 @@ def fetch_issues(repo: str, limit: int) -> list[dict]: def fetch_prs(repo: str, limit: int) -> list[dict]: - return json.loads( + return pipeline_board.json.loads( run_gh( "pr", "list", @@ -90,224 +89,12 @@ def fetch_prs(repo: str, limit: int) -> list[dict]: def fetch_pr_reviews(repo: str, pr_number: int) -> list[dict]: - data = json.loads( + data = pipeline_board.json.loads( run_gh("pr", "view", str(pr_number), "-R", repo, "--json", "reviews") ) return data.get("reviews", []) - - -def label_names(issue: dict) -> set[str]: - return {label["name"] for label in issue.get("labels", [])} - - -def linked_pr_numbers(item: dict) -> list[int]: - urls = item.get("linked pull requests") or [] - numbers = [] - for url in urls: - try: - numbers.append(int(url.rstrip("/").split("/")[-1])) - except ValueError: - continue - return numbers - - -def is_tracked_issue_title(title: str | None) -> bool: - if not title: - return False - return title.startswith("[Model]") or title.startswith("[Rule]") - - -def has_copilot_review(reviews: list[dict]) -> bool: - for review in reviews: - author = review.get("author") or review.get("user") or {} - if author.get("login") in COPILOT_REVIEWERS: - return True - return False - - -def all_checks_green(pr: dict) -> bool: - statuses = pr.get("statusCheckRollup") or [] - if not statuses: - return False - - for status in statuses: - typename = status.get("__typename") - if typename == "CheckRun": - if status.get("status") != "COMPLETED": - return False - if status.get("conclusion") not in {"SUCCESS", "SKIPPED", "NEUTRAL"}: - return False - elif typename == "StatusContext": - if status.get("state") != "SUCCESS": - return False - return True - - -def infer_issue_status( - issue: dict, - linked_prs: list[dict], - pr_reviews: dict[int, list[dict]], -) -> tuple[str, str]: - labels = label_names(issue) - merged_prs = [pr for pr in linked_prs if pr.get("mergedAt")] - open_prs = [pr for pr in linked_prs if pr.get("state") == "OPEN"] - - if merged_prs: - pr_numbers = ", ".join(f"#{pr['number']}" for pr in merged_prs) - return STATUS_DONE, f"linked merged PR {pr_numbers}" - - if issue.get("state") == "CLOSED": - return STATUS_DONE, "issue itself is closed" - - if open_prs: - waiting_for_copilot = [ - pr - for pr in open_prs - if not has_copilot_review(pr_reviews.get(int(pr["number"]), [])) - ] - if waiting_for_copilot: - pr_numbers = ", ".join(f"#{pr['number']}" for pr in waiting_for_copilot) - return STATUS_REVIEW_POOL, f"open PR {pr_numbers} waiting for Copilot review" - - green_prs = [pr for pr in open_prs if all_checks_green(pr)] - if len(green_prs) == len(open_prs): - pr_numbers = ", ".join(f"#{pr['number']}" for pr in open_prs) - return STATUS_FINAL_REVIEW, f"Copilot reviewed green open PR {pr_numbers}" - - pr_numbers = ", ".join(f"#{pr['number']}" for pr in open_prs) - return STATUS_REVIEW_POOL, f"open PR {pr_numbers} still implementing or fixing review" - - if "Good" in labels: - return STATUS_READY, 'label "Good" present and no linked PR' - - if labels & FAILURE_LABELS: - bad = ", ".join(sorted(labels & FAILURE_LABELS)) - return STATUS_BACKLOG, f"failure labels present: {bad}" - - return STATUS_BACKLOG, "default backlog: no linked PR and no Ready signal" - - -def build_recovery_plan( - board_data: dict, - issues: list[dict], - prs: list[dict], - pr_reviews: dict[int, list[dict]], -) -> list[dict]: - issues_by_number = {issue["number"]: issue for issue in issues} - prs_by_number = {pr["number"]: pr for pr in prs} - - plan = [] - for item in board_data.get("items", []): - content = item.get("content") or {} - issue_number = content.get("number") - if issue_number is None: - continue - - issue = issues_by_number.get(issue_number) - if issue is None: - continue - - title = content.get("title") or issue.get("title") - if not is_tracked_issue_title(title): - continue - - linked_prs = [ - prs_by_number[pr_number] - for pr_number in linked_pr_numbers(item) - if pr_number in prs_by_number - ] - status_name, reason = infer_issue_status(issue, linked_prs, pr_reviews) - plan.append( - { - "item_id": item["id"], - "issue_number": issue_number, - "title": title, - "current_status": item.get("status"), - "proposed_status": status_name, - "option_id": STATUS_OPTION_IDS[status_name], - "reason": reason, - } - ) - - return sorted(plan, key=lambda entry: entry["issue_number"]) - - -def save_backup( - backup_file: Path, - *, - board_data: dict, - issues: list[dict], - prs: list[dict], - pr_reviews: dict[int, list[dict]], - plan: list[dict], -) -> None: - backup_file.parent.mkdir(parents=True, exist_ok=True) - payload = { - "generated_at": datetime.now(timezone.utc).isoformat(), - "board_data": board_data, - "issues": issues, - "prs": prs, - "pr_reviews": pr_reviews, - "plan": plan, - } - backup_file.write_text(json.dumps(payload, indent=2, sort_keys=True) + "\n") - - -def apply_plan( - plan: list[dict], - *, - project_id: str, - field_id: str, -) -> int: - changed = 0 - for entry in plan: - if entry["current_status"] == entry["proposed_status"]: - continue - subprocess.check_call( - [ - "gh", - "project", - "item-edit", - "--project-id", - project_id, - "--id", - entry["item_id"], - "--field-id", - field_id, - "--single-select-option-id", - entry["option_id"], - ] - ) - changed += 1 - return changed - - def default_backup_path(project_number: int) -> Path: - stamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%SZ") - return Path("/tmp") / f"project-{project_number}-status-recovery-{stamp}.json" - - -def print_summary(plan: list[dict]) -> None: - counts = Counter(entry["proposed_status"] for entry in plan) - print("Proposed status counts:") - for status_name in [ - STATUS_BACKLOG, - STATUS_READY, - STATUS_REVIEW_POOL, - STATUS_FINAL_REVIEW, - STATUS_DONE, - ]: - print(f" {status_name}: {counts.get(status_name, 0)}") - - -def print_examples(plan: list[dict], limit: int = 20) -> None: - print("") - print(f"First {min(limit, len(plan))} assignments:") - for entry in plan[:limit]: - print( - f" #{entry['issue_number']:<4} {entry['proposed_status']:<13} " - f"{entry['reason']} | {entry['title']}" - ) + return pipeline_board.default_backup_path(project_number) def parse_args(argv: list[str]) -> argparse.Namespace: @@ -351,7 +138,9 @@ def main(argv: list[str] | None = None) -> int: plan = build_recovery_plan(board_data, issues, prs, pr_reviews) if args.plan_file is not None: args.plan_file.parent.mkdir(parents=True, exist_ok=True) - args.plan_file.write_text(json.dumps(plan, indent=2, sort_keys=True) + "\n") + args.plan_file.write_text( + pipeline_board.json.dumps(plan, indent=2, sort_keys=True) + "\n" + ) print_summary(plan) if not args.no_examples: diff --git a/scripts/test_make_helpers.py b/scripts/test_make_helpers.py index 5b3f7a224..c6c74640c 100644 --- a/scripts/test_make_helpers.py +++ b/scripts/test_make_helpers.py @@ -55,6 +55,134 @@ def test_run_agent_enables_multi_agent_for_codex(self) -> None: ], ) + def test_poll_project_items_uses_pipeline_board_cli(self) -> None: + if shutil.which("dash") is None: + self.skipTest("dash is not installed") + + proc = subprocess.run( + [ + "dash", + "-c", + ( + ". scripts/make_helpers.sh; " + "python3() { printf '%s\\n' \"$@\"; }; " + "poll_project_items ready /tmp/state.json" + ), + ], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual( + proc.stdout.splitlines(), + [ + "scripts/pipeline_board.py", + "next", + "ready", + "/tmp/state.json", + ], + ) + + def test_move_board_item_uses_pipeline_board_cli(self) -> None: + if shutil.which("dash") is None: + self.skipTest("dash is not installed") + + proc = subprocess.run( + [ + "dash", + "-c", + ( + ". scripts/make_helpers.sh; " + "python3() { printf '%s\\n' \"$@\"; }; " + "move_board_item PVTI_demo final-review" + ), + ], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual( + proc.stdout.splitlines(), + [ + "scripts/pipeline_board.py", + "move", + "PVTI_demo", + "final-review", + ], + ) + + def test_pr_snapshot_uses_pipeline_pr_cli(self) -> None: + if shutil.which("dash") is None: + self.skipTest("dash is not installed") + + proc = subprocess.run( + [ + "dash", + "-c", + ( + ". scripts/make_helpers.sh; " + "python3() { printf '%s\\n' \"$@\"; }; " + "pr_snapshot CodingThrust/problem-reductions 570" + ), + ], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual( + proc.stdout.splitlines(), + [ + "scripts/pipeline_pr.py", + "snapshot", + "--repo", + "CodingThrust/problem-reductions", + "--pr", + "570", + "--format", + "json", + ], + ) + + def test_pr_wait_ci_uses_pipeline_pr_cli(self) -> None: + if shutil.which("dash") is None: + self.skipTest("dash is not installed") + + proc = subprocess.run( + [ + "dash", + "-c", + ( + ". scripts/make_helpers.sh; " + "python3() { printf '%s\\n' \"$@\"; }; " + "pr_wait_ci CodingThrust/problem-reductions 570 1200 15" + ), + ], + cwd=REPO_ROOT, + capture_output=True, + text=True, + ) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual( + proc.stdout.splitlines(), + [ + "scripts/pipeline_pr.py", + "wait-ci", + "--repo", + "CodingThrust/problem-reductions", + "--pr", + "570", + "--timeout", + "1200", + "--interval", + "15", + "--format", + "json", + ], + ) + if __name__ == "__main__": unittest.main() diff --git a/scripts/test_pipeline_board.py b/scripts/test_pipeline_board.py new file mode 100644 index 000000000..dc75a62f4 --- /dev/null +++ b/scripts/test_pipeline_board.py @@ -0,0 +1,235 @@ +#!/usr/bin/env python3 +import tempfile +import unittest +from pathlib import Path + +from pipeline_board import ( + STATUS_DONE, + STATUS_FINAL_REVIEW, + STATUS_READY, + STATUS_REVIEW_POOL, + ack_item, + build_recovery_plan, + process_snapshot, +) + + +def make_issue_item( + item_id: str, + number: int, + *, + status: str = "Ready", + title: str | None = None, + linked_prs: list[int] | None = None, +) -> dict: + item = { + "id": item_id, + "status": status, + "content": { + "type": "Issue", + "number": number, + "title": title or f"[Model] Issue {number}", + }, + "title": title or f"[Model] Issue {number}", + } + if linked_prs is not None: + item["linked pull requests"] = [ + f"https://github.com/CodingThrust/problem-reductions/pull/{pr_number}" + for pr_number in linked_prs + ] + return item + + +def make_pr_item(item_id: str, number: int, status: str = "Review pool") -> dict: + return { + "id": item_id, + "status": status, + "content": {"type": "PullRequest", "number": number}, + } + + +def make_issue(number: int, *, state: str = "OPEN", labels: list[str] | None = None) -> dict: + return { + "number": number, + "state": state, + "title": f"[Model] Issue {number}", + "labels": [{"name": label} for label in (labels or [])], + } + + +def make_pr( + number: int, + *, + state: str = "OPEN", + merged: bool = False, + checks: list[dict] | None = None, +) -> dict: + return { + "number": number, + "state": state, + "mergedAt": "2026-03-15T00:00:00Z" if merged else None, + "statusCheckRollup": checks or [], + } + + +def success_check(name: str = "ci") -> dict: + return { + "__typename": "CheckRun", + "name": name, + "status": "COMPLETED", + "conclusion": "SUCCESS", + } + + +class PipelineBoardPollTests(unittest.TestCase): + def test_ready_queue_retries_same_item_until_ack(self) -> None: + with tempfile.TemporaryDirectory() as tmpdir: + state_file = Path(tmpdir) / "ready-state.json" + snapshot = { + "items": [ + make_issue_item("PVTI_1", 101), + make_issue_item("PVTI_2", 102), + ] + } + + item_id, number = process_snapshot("ready", snapshot, state_file) + self.assertEqual((item_id, number), ("PVTI_1", 101)) + + item_id, number = process_snapshot("ready", snapshot, state_file) + self.assertEqual((item_id, number), ("PVTI_1", 101)) + + ack_item(state_file, "PVTI_1") + item_id, number = process_snapshot("ready", snapshot, state_file) + self.assertEqual((item_id, number), ("PVTI_2", 102)) + + def test_review_queue_resolves_issue_cards_to_prs(self) -> None: + def fake_pr_resolver(repo: str, issue_number: int) -> int | None: + self.assertEqual(repo, "CodingThrust/problem-reductions") + return 570 if issue_number == 117 else None + + def fake_review_fetcher(repo: str, pr_number: int) -> list[dict]: + self.assertEqual(repo, "CodingThrust/problem-reductions") + if pr_number == 570: + return [{"user": {"login": "copilot-pull-request-reviewer[bot]"}}] + return [] + + def fake_pr_state_fetcher(repo: str, pr_number: int) -> str: + self.assertEqual(repo, "CodingThrust/problem-reductions") + self.assertEqual(pr_number, 570) + return "OPEN" + + with tempfile.TemporaryDirectory() as tmpdir: + state_file = Path(tmpdir) / "review-state.json" + item_id, number = process_snapshot( + "review", + {"items": [make_issue_item("PVTI_10", 117, status="Review pool")]}, + state_file, + repo="CodingThrust/problem-reductions", + review_fetcher=fake_review_fetcher, + pr_resolver=fake_pr_resolver, + pr_state_fetcher=fake_pr_state_fetcher, + ) + self.assertEqual((item_id, number), ("PVTI_10", 570)) + + def test_review_queue_skips_closed_pr_cards(self) -> None: + def fake_review_fetcher(repo: str, pr_number: int) -> list[dict]: + return [{"user": {"login": "copilot-pull-request-reviewer[bot]"}}] + + def fake_pr_state_fetcher(repo: str, pr_number: int) -> str: + self.assertEqual(repo, "CodingThrust/problem-reductions") + self.assertEqual(pr_number, 570) + return "CLOSED" + + with tempfile.TemporaryDirectory() as tmpdir: + state_file = Path(tmpdir) / "review-state.json" + no_item = process_snapshot( + "review", + {"items": [make_pr_item("PVTI_10", 570)]}, + state_file, + repo="CodingThrust/problem-reductions", + review_fetcher=fake_review_fetcher, + pr_state_fetcher=fake_pr_state_fetcher, + ) + self.assertIsNone(no_item) + + +class PipelineBoardRecoveryTests(unittest.TestCase): + def test_recovery_plan_marks_merged_pr_items_done(self) -> None: + board_data = { + "items": [ + make_issue_item( + "PVTI_1", + 101, + status="Review pool", + title="[Model] MinimumFeedbackVertexSet", + linked_prs=[615], + ) + ] + } + issues = [make_issue(101, labels=["Good"])] + prs = [make_pr(615, state="MERGED", merged=True)] + + plan = build_recovery_plan(board_data, issues, prs, pr_reviews={}) + + self.assertEqual(len(plan), 1) + self.assertEqual(plan[0]["proposed_status"], STATUS_DONE) + + def test_recovery_plan_marks_green_copilot_reviewed_prs_final_review(self) -> None: + board_data = { + "items": [ + make_issue_item( + "PVTI_1", + 101, + status="Review pool", + title="[Model] HamiltonianPath", + linked_prs=[621], + ) + ] + } + issues = [make_issue(101, labels=["Good"])] + prs = [make_pr(621, checks=[success_check()])] + pr_reviews = {621: [{"user": {"login": "copilot-pull-request-reviewer[bot]"}}]} + + plan = build_recovery_plan(board_data, issues, prs, pr_reviews=pr_reviews) + + self.assertEqual(plan[0]["proposed_status"], STATUS_FINAL_REVIEW) + + def test_recovery_plan_marks_open_pr_without_copilot_review_review_pool(self) -> None: + board_data = { + "items": [ + make_issue_item( + "PVTI_1", + 101, + status="In progress", + title="[Model] SteinerTree", + linked_prs=[192], + ) + ] + } + issues = [make_issue(101, labels=["Good"])] + prs = [make_pr(192, checks=[success_check()])] + + plan = build_recovery_plan(board_data, issues, prs, pr_reviews={192: []}) + + self.assertEqual(plan[0]["proposed_status"], STATUS_REVIEW_POOL) + + def test_recovery_plan_marks_good_issue_without_pr_ready(self) -> None: + board_data = { + "items": [ + make_issue_item( + "PVTI_1", + 101, + status="Backlog", + title="[Model] ExactCoverBy3Sets", + ) + ] + } + issues = [make_issue(101, labels=["Good"])] + + plan = build_recovery_plan(board_data, issues, prs=[], pr_reviews={}) + + self.assertEqual(plan[0]["proposed_status"], STATUS_READY) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/test_pipeline_pr.py b/scripts/test_pipeline_pr.py new file mode 100644 index 000000000..30394ce69 --- /dev/null +++ b/scripts/test_pipeline_pr.py @@ -0,0 +1,228 @@ +#!/usr/bin/env python3 +import unittest + +from pipeline_pr import ( + build_snapshot, + extract_codecov_summary, + extract_linked_issue_number, + summarize_check_runs, + summarize_comments, + wait_for_ci, +) + + +def make_inline_comment( + login: str, + *, + body: str = "comment", + path: str = "src/lib.rs", + line: int = 7, +) -> dict: + return { + "user": {"login": login}, + "body": body, + "path": path, + "line": line, + "original_line": line, + } + + +def make_review( + login: str, + *, + body: str = "review body", + state: str = "COMMENTED", +) -> dict: + return { + "user": {"login": login}, + "body": body, + "state": state, + } + + +def make_issue_comment(login: str, *, body: str = "discussion") -> dict: + return { + "user": {"login": login}, + "body": body, + } + + +def make_check_run( + name: str, + *, + status: str = "completed", + conclusion: str | None = "success", +) -> dict: + return { + "name": name, + "status": status, + "conclusion": conclusion, + } + + +class PipelinePrHelpersTests(unittest.TestCase): + def test_extract_linked_issue_number_prefers_body_over_title(self) -> None: + linked_issue = extract_linked_issue_number( + "Fix #117: Add GraphPartitioning model", + "This supersedes older work and closes #42.", + ) + self.assertEqual(linked_issue, 42) + + def test_extract_linked_issue_number_falls_back_to_title(self) -> None: + linked_issue = extract_linked_issue_number( + "Fix #117: Add GraphPartitioning model", + "No explicit closing keyword here.", + ) + self.assertEqual(linked_issue, 117) + + def test_summarize_comments_splits_human_copilot_and_codecov_sources(self) -> None: + summary = summarize_comments( + inline_comments=[ + make_inline_comment("copilot-pull-request-reviewer[bot]"), + make_inline_comment("alice"), + ], + reviews=[ + make_review("bob", body="Please add tests"), + make_review("copilot-pull-request-reviewer[bot]", body="bot review"), + make_review("carol", body=""), + ], + issue_comments=[ + make_issue_comment("dave", body="Please update docs"), + make_issue_comment("codecov[bot]", body="## [Codecov] Patch coverage is 82%"), + ], + linked_issue_comments=[ + make_issue_comment("erin", body="The literature citation is important"), + make_issue_comment("deploy-bot[bot]", body="automated deployment note"), + ], + ) + + self.assertEqual(summary["counts"]["inline_comments"], 2) + self.assertEqual(summary["counts"]["copilot_inline_comments"], 1) + self.assertEqual(summary["counts"]["human_inline_comments"], 1) + self.assertEqual(summary["counts"]["human_reviews"], 1) + self.assertEqual(summary["counts"]["human_issue_comments"], 1) + self.assertEqual(summary["counts"]["human_linked_issue_comments"], 1) + self.assertEqual(summary["counts"]["codecov_comments"], 1) + + def test_extract_codecov_summary_parses_latest_comment_and_filepaths(self) -> None: + summary = extract_codecov_summary( + [ + make_issue_comment("alice", body="looks fine"), + make_issue_comment( + "codecov[bot]", + body=( + "## [Codecov]\n" + "Patch coverage: `84.21%`\n" + "Project coverage is `91.30%`\n" + "https://codecov.io/gh/CodingThrust/problem-reductions?" + "filepath=src%2Fmodels%2Fgraph%2Ffoo.rs&line=17\n" + "https://codecov.io/gh/CodingThrust/problem-reductions?" + "filepath=src%2Fmodels%2Fgraph%2Ffoo.rs&line=21\n" + "https://codecov.io/gh/CodingThrust/problem-reductions?" + "filepath=src%2Frules%2Ffoo_bar.rs&line=8\n" + ), + ), + ] + ) + + self.assertTrue(summary["found"]) + self.assertEqual(summary["patch_coverage"], 84.21) + self.assertEqual(summary["project_coverage"], 91.30) + self.assertEqual( + summary["filepaths"], + [ + "src/models/graph/foo.rs", + "src/rules/foo_bar.rs", + ], + ) + + def test_summarize_check_runs_reports_overall_state(self) -> None: + self.assertEqual( + summarize_check_runs({"check_runs": [make_check_run("test", status="queued", conclusion=None)]})["state"], + "pending", + ) + self.assertEqual( + summarize_check_runs({"check_runs": [make_check_run("test", conclusion="failure")]})["state"], + "failure", + ) + self.assertEqual( + summarize_check_runs( + { + "check_runs": [ + make_check_run("fmt"), + make_check_run("coverage", conclusion="neutral"), + ] + } + )["state"], + "success", + ) + + def test_build_snapshot_includes_linked_issue_ci_and_codecov(self) -> None: + snapshot = build_snapshot( + { + "number": 570, + "title": "Fix #117: Add GraphPartitioning model", + "body": "Closes #117", + "state": "OPEN", + "url": "https://github.com/CodingThrust/problem-reductions/pull/570", + "headRefName": "feature/graph-partitioning", + "baseRefName": "main", + "mergeable": "MERGEABLE", + "headRefOid": "abc123", + "labels": [{"name": "model"}], + "files": [{"path": "src/models/graph/graph_partitioning.rs"}], + "commits": [{"oid": "abc123"}, {"oid": "def456"}], + "additions": 120, + "deletions": 7, + }, + linked_issue_number=117, + linked_issue={ + "number": 117, + "title": "[Model] GraphPartitioning", + "state": "OPEN", + }, + ci_summary={"state": "success", "total": 3, "failing": 0, "pending": 0}, + codecov_summary={"found": True, "patch_coverage": 84.21, "filepaths": ["src/models/graph/graph_partitioning.rs"]}, + ) + + self.assertEqual(snapshot["number"], 570) + self.assertEqual(snapshot["linked_issue_number"], 117) + self.assertEqual(snapshot["linked_issue"]["title"], "[Model] GraphPartitioning") + self.assertEqual(snapshot["ci"]["state"], "success") + self.assertEqual(snapshot["codecov"]["patch_coverage"], 84.21) + self.assertEqual(snapshot["counts"]["files"], 1) + self.assertEqual(snapshot["counts"]["commits"], 2) + + def test_wait_for_ci_polls_until_terminal_state(self) -> None: + summaries = [ + {"state": "pending", "total": 2, "pending": 1, "failing": 0}, + {"state": "pending", "total": 2, "pending": 1, "failing": 0}, + {"state": "success", "total": 2, "pending": 0, "failing": 0}, + ] + sleeps: list[float] = [] + now = [0.0] + + def fake_fetcher() -> dict: + return summaries.pop(0) + + def fake_monotonic() -> float: + return now[0] + + def fake_sleep(seconds: float) -> None: + sleeps.append(seconds) + now[0] += seconds + + result = wait_for_ci( + fake_fetcher, + timeout_seconds=30, + interval_seconds=5, + monotonic_fn=fake_monotonic, + sleep_fn=fake_sleep, + ) + + self.assertEqual(result["state"], "success") + self.assertEqual(sleeps, [5, 5]) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/test_project_board_poll.py b/scripts/test_project_board_poll.py index 10451a61d..44b925e1c 100644 --- a/scripts/test_project_board_poll.py +++ b/scripts/test_project_board_poll.py @@ -26,6 +26,15 @@ def make_pr_item(item_id: str, number: int, status: str = "Review pool") -> dict } +def with_linked_prs(item: dict, *pr_numbers: int) -> dict: + updated = dict(item) + updated["linked pull requests"] = [ + f"https://github.com/CodingThrust/problem-reductions/pull/{number}" + for number in pr_numbers + ] + return updated + + class ProjectBoardPollTests(unittest.TestCase): def test_ready_queue_retries_same_item_until_ack(self) -> None: with tempfile.TemporaryDirectory() as tmpdir: @@ -92,6 +101,11 @@ def fake_review_fetcher(repo: str, pr_number: int) -> list[dict]: return [{"user": {"login": "copilot-pull-request-reviewer[bot]"}}] return [] + def fake_pr_state_fetcher(repo: str, pr_number: int) -> str: + self.assertEqual(repo, "CodingThrust/problem-reductions") + self.assertEqual(pr_number, 570) + return "OPEN" + with tempfile.TemporaryDirectory() as tmpdir: state_file = Path(tmpdir) / "review-state.json" item_id, number = process_snapshot( @@ -101,6 +115,7 @@ def fake_review_fetcher(repo: str, pr_number: int) -> list[dict]: repo="CodingThrust/problem-reductions", review_fetcher=fake_review_fetcher, pr_resolver=fake_pr_resolver, + pr_state_fetcher=fake_pr_state_fetcher, ) self.assertEqual((item_id, number), ("PVTI_10", 570)) @@ -108,6 +123,11 @@ def test_review_fetch_errors_are_not_suppressed(self) -> None: def fake_review_fetcher(repo: str, pr_number: int) -> list[dict]: raise subprocess.CalledProcessError(42, ["gh", "api"]) + def fake_pr_state_fetcher(repo: str, pr_number: int) -> str: + self.assertEqual(repo, "CodingThrust/problem-reductions") + self.assertEqual(pr_number, 570) + return "OPEN" + with tempfile.TemporaryDirectory() as tmpdir: state_file = Path(tmpdir) / "review-state.json" with self.assertRaises(subprocess.CalledProcessError): @@ -117,8 +137,66 @@ def fake_review_fetcher(repo: str, pr_number: int) -> list[dict]: state_file, repo="CodingThrust/problem-reductions", review_fetcher=fake_review_fetcher, + pr_state_fetcher=fake_pr_state_fetcher, ) + def test_review_queue_skips_closed_pr_cards(self) -> None: + def fake_review_fetcher(repo: str, pr_number: int) -> list[dict]: + return [{"user": {"login": "copilot-pull-request-reviewer[bot]"}}] + + def fake_pr_state_fetcher(repo: str, pr_number: int) -> str: + self.assertEqual(repo, "CodingThrust/problem-reductions") + self.assertEqual(pr_number, 570) + return "CLOSED" + + with tempfile.TemporaryDirectory() as tmpdir: + state_file = Path(tmpdir) / "review-state.json" + no_item = process_snapshot( + "review", + {"items": [make_pr_item("PVTI_10", 570)]}, + state_file, + repo="CodingThrust/problem-reductions", + review_fetcher=fake_review_fetcher, + pr_state_fetcher=fake_pr_state_fetcher, + ) + self.assertIsNone(no_item) + + def test_review_queue_skips_issue_cards_with_mixed_linked_pr_states(self) -> None: + def fake_pr_resolver(repo: str, issue_number: int) -> int | None: + self.assertEqual(repo, "CodingThrust/problem-reductions") + self.assertEqual(issue_number, 108) + return 173 + + def fake_review_fetcher(repo: str, pr_number: int) -> list[dict]: + self.assertEqual(repo, "CodingThrust/problem-reductions") + self.assertEqual(pr_number, 173) + return [{"user": {"login": "copilot-pull-request-reviewer[bot]"}}] + + def fake_pr_state_fetcher(repo: str, pr_number: int) -> str: + self.assertEqual(repo, "CodingThrust/problem-reductions") + return {170: "CLOSED", 173: "OPEN"}[pr_number] + + with tempfile.TemporaryDirectory() as tmpdir: + state_file = Path(tmpdir) / "review-state.json" + no_item = process_snapshot( + "review", + { + "items": [ + with_linked_prs( + make_issue_item("PVTI_10", 108, status="Review pool"), + 170, + 173, + ) + ] + }, + state_file, + repo="CodingThrust/problem-reductions", + review_fetcher=fake_review_fetcher, + pr_resolver=fake_pr_resolver, + pr_state_fetcher=fake_pr_state_fetcher, + ) + self.assertIsNone(no_item) + if __name__ == "__main__": unittest.main() From 415eff5011bec6635e74119353befb0904465758 Mon Sep 17 00:00:00 2001 From: GiggleLiu Date: Sun, 15 Mar 2026 15:13:22 +0800 Subject: [PATCH 02/29] refactor(pipeline): add worktree and review checks --- .claude/skills/final-review/SKILL.md | 25 +- .claude/skills/project-pipeline/SKILL.md | 9 +- .claude/skills/review-implementation/SKILL.md | 25 +- .claude/skills/review-pipeline/SKILL.md | 20 +- Makefile | 27 +- ...03-15-pipeline-automation-refactor-plan.md | 12 +- scripts/make_helpers.sh | 25 ++ scripts/pipeline_checks.py | 206 +++++++++++++ scripts/pipeline_worktree.py | 278 ++++++++++++++++++ scripts/test_make_helpers.py | 68 +++++ scripts/test_pipeline_checks.py | 91 ++++++ scripts/test_pipeline_worktree.py | 91 ++++++ 12 files changed, 834 insertions(+), 43 deletions(-) create mode 100644 scripts/pipeline_checks.py create mode 100644 scripts/pipeline_worktree.py create mode 100644 scripts/test_pipeline_checks.py create mode 100644 scripts/test_pipeline_worktree.py diff --git a/.claude/skills/final-review/SKILL.md b/.claude/skills/final-review/SKILL.md index b9340b2a2..1130db01e 100644 --- a/.claude/skills/final-review/SKILL.md +++ b/.claude/skills/final-review/SKILL.md @@ -44,11 +44,16 @@ If a specific PR number was given, use it directly. Otherwise: Collect all information needed for the review: -1a. **PR metadata**: `gh pr view --json title,body,labels,files,additions,deletions,commits,headRefName,baseRefName,url,state` +1a. **PR metadata**: prefer the scripted snapshot: + ```bash + REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner) + SNAPSHOT=$(python3 scripts/pipeline_pr.py snapshot --repo "$REPO" --pr --format json) + ``` + This includes title, body, labels, files, additions, deletions, commits, branch names, mergeability, linked issue, CI summary, and Codecov summary in one JSON payload. 1b. **PR diff**: `gh pr diff ` — read the full diff to understand all changes. -1c. **Linked issue**: Extract the linked issue number from PR body (look for `Fixes #N`, `Closes #N`, or `#N` references). Fetch issue body: `gh issue view --json title,body,labels` +1c. **Linked issue**: read it from `SNAPSHOT["linked_issue_number"]` / `SNAPSHOT["linked_issue"]` instead of reparsing the PR body manually. 1d. **Determine PR type**: From labels and title, classify as `[Model]` or `[Rule]`. - For `[Model]`: identify the problem name being added @@ -126,7 +131,21 @@ Check that the PR only touches files expected for its type. Any file outside the - `docs/src/reductions/reduction_graph.json` — graph export - `docs/src/reductions/problem_schemas.json` — only if updating field descriptions -If any file falls outside these whitelists, flag it: +Run the deterministic whitelist check against the PR file list from `SNAPSHOT`: + +```bash +printf '%s\n' "$SNAPSHOT" | python3 -c " +import sys, json +snapshot = json.load(sys.stdin) +for path in snapshot['files']: + print(path) +" > /tmp/final-review-files.txt + +python3 scripts/pipeline_checks.py file-whitelist --kind model --files-file /tmp/final-review-files.txt --format json +# or --kind rule for [Rule] PRs +``` + +If any file falls outside the whitelist result, flag it: > **File Whitelist Check** > diff --git a/.claude/skills/project-pipeline/SKILL.md b/.claude/skills/project-pipeline/SKILL.md index f02504652..547717a8d 100644 --- a/.claude/skills/project-pipeline/SKILL.md +++ b/.claude/skills/project-pipeline/SKILL.md @@ -109,12 +109,9 @@ Ready issues (ranked): Create an isolated git worktree for this issue so the main working directory stays clean: ```bash -REPO_ROOT=$(git rev-parse --show-toplevel) -git fetch origin main -BRANCH="issue--" -WORKTREE_DIR=".worktrees/$BRANCH" -mkdir -p .worktrees -git worktree add "$WORKTREE_DIR" -b "$BRANCH" origin/main +WORKTREE=$(python3 scripts/pipeline_worktree.py create-issue --issue --slug --base origin/main --format json) +BRANCH=$(printf '%s\n' "$WORKTREE" | python3 -c "import sys,json; print(json.load(sys.stdin)['branch'])") +WORKTREE_DIR=$(printf '%s\n' "$WORKTREE" | python3 -c "import sys,json; print(json.load(sys.stdin)['worktree_dir'])") cd "$WORKTREE_DIR" ``` diff --git a/.claude/skills/review-implementation/SKILL.md b/.claude/skills/review-implementation/SKILL.md index 3e61fbd96..d515ab2ae 100644 --- a/.claude/skills/review-implementation/SKILL.md +++ b/.claude/skills/review-implementation/SKILL.md @@ -21,27 +21,22 @@ Dispatches two parallel review subagents with fresh context (no implementation h Determine whether new model/rule files were added: ```bash -# Check for NEW files across the entire branch -git diff --name-only --diff-filter=A main..HEAD +BASE_SHA=$(git merge-base main HEAD) +HEAD_SHA=$(git rev-parse HEAD) +SCOPE=$(python3 scripts/pipeline_checks.py detect-scope --base "$BASE_SHA" --head "$HEAD_SHA" --format json) ``` -Detection rules: -- New file in `src/models/` (not `mod.rs`) -> **model review** (structural + quality) -- New file in `src/rules/` (not `mod.rs`, `traits.rs`, `cost.rs`, `graph.rs`, `registry.rs`) -> **rule review** (structural + quality) -- Only modified files (no new model/rule) -> **quality review only** -- Both new model and rule files -> dispatch structural for both + quality -- Explicit argument overrides auto-detection +Read `SCOPE` to determine: +- `review_type` -> `model`, `rule`, `model+rule`, or `generic` +- `models` -> new model files with category, file stem, and problem name +- `rules` -> new rule files with rule stem +- `added_files` / `changed_files` -> normalized file lists -Extract the problem name(s) and rule source/target from the file paths. +Explicit arguments still override auto-detection. ## Step 2: Prepare Subagent Context -Get the git SHAs for the review range: - -```bash -BASE_SHA=$(git merge-base main HEAD) # or HEAD~N for batch reviews -HEAD_SHA=$(git rev-parse HEAD) -``` +Reuse `BASE_SHA` and `HEAD_SHA` from Step 1. For batch reviews you may still choose a narrower manual base SHA if needed. Get the diff summary and changed file list: diff --git a/.claude/skills/review-pipeline/SKILL.md b/.claude/skills/review-pipeline/SKILL.md index b288bf0df..88617b27b 100644 --- a/.claude/skills/review-pipeline/SKILL.md +++ b/.claude/skills/review-pipeline/SKILL.md @@ -122,13 +122,9 @@ In `--all` mode, claim each PR right before processing it (not all at once). Create an isolated git worktree so the main working directory stays clean: ```bash -REPO_ROOT=$(git rev-parse --show-toplevel) REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner) -BRANCH=$(gh pr view $PR --json headRefName --jq .headRefName) -WORKTREE_DIR=".worktrees/review-$BRANCH" -mkdir -p .worktrees -git fetch origin $BRANCH -git worktree add "$WORKTREE_DIR" $BRANCH +WORKTREE=$(python3 scripts/pipeline_worktree.py checkout-pr --repo "$REPO" --pr "$PR" --format json) +WORKTREE_DIR=$(printf '%s\n' "$WORKTREE" | python3 -c "import sys,json; print(json.load(sys.stdin)['worktree_dir'])") cd "$WORKTREE_DIR" ``` @@ -145,18 +141,18 @@ All subsequent steps run inside the worktree. Check if the branch has merge conflicts with main: ```bash -git fetch origin main -git merge origin/main --no-edit +MERGE=$(python3 scripts/pipeline_worktree.py merge-main --worktree "$WORKTREE_DIR" --format json) +MERGE_STATUS=$(printf '%s\n' "$MERGE" | python3 -c "import sys,json; print(json.load(sys.stdin)['status'])") ``` -- If the merge succeeds cleanly: push the merge commit and continue. +- If `MERGE_STATUS == clean`: push the merge commit and continue. - If there are conflicts: - 1. Inspect the conflicting files with `git diff --name-only --diff-filter=U`. + 1. Inspect the conflicting files from the `conflicts` array in `MERGE`. 2. Compare the current skill versions on main vs the PR branch to understand which patterns are current. 3. Resolve conflicts (prefer main's patterns for skill-generated code, the PR branch for problem-specific logic, main for regenerated artifacts like JSON). 4. Stage resolved files, commit, and push. -- If conflicts are too complex to resolve automatically (e.g., overlapping logic changes in the same function): - 1. Abort the merge: `git merge --abort` +- If `MERGE_STATUS == conflicted` and `likely_complex == true` (or the overlap is otherwise too complex to resolve automatically): + 1. Abort the merge: `git merge --abort` if a merge is still in progress 2. Move the project item back to `Review pool`: ```bash gh project item-edit \ diff --git a/Makefile b/Makefile index cc844ada2..0ab7701a2 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # Makefile for problemreductions -.PHONY: help build test mcp-test fmt clippy doc mdbook paper examples clean coverage rust-export compare qubo-testdata export-schemas release run-plan run-issue run-pipeline run-pipeline-forever run-review run-review-forever board-next board-ack board-move pr-context pr-wait-ci diagrams jl-testdata cli cli-demo copilot-review +.PHONY: help build test mcp-test fmt clippy doc mdbook paper examples clean coverage rust-export compare qubo-testdata export-schemas release run-plan run-issue run-pipeline run-pipeline-forever run-review run-review-forever board-next board-ack board-move pr-context pr-wait-ci worktree-issue worktree-pr diagrams jl-testdata cli cli-demo copilot-review RUNNER ?= codex CLAUDE_MODEL ?= opus @@ -42,6 +42,8 @@ help: @echo " board-move ITEM= STATUS= - Move a project item to a named status" @echo " pr-context PR= [REPO=] - Fetch structured PR snapshot JSON" @echo " pr-wait-ci PR= [REPO=] - Poll CI until terminal state and print JSON" + @echo " worktree-issue ISSUE= SLUG= - Create an issue worktree from origin/main" + @echo " worktree-pr PR= [REPO=] - Checkout a PR into an isolated worktree" @echo " copilot-review - Request Copilot code review on current PR" @echo "" @echo " Set RUNNER=claude to use Claude instead of Codex (default: codex)" @@ -458,6 +460,29 @@ pr-wait-ci: interval=$${INTERVAL:-30}; \ pr_wait_ci "$$repo" "$(PR)" "$$timeout" "$$interval" +# Create an issue worktree from origin/main +# Usage: make worktree-issue ISSUE=117 SLUG=graph-partitioning +worktree-issue: + @if [ -z "$(ISSUE)" ] || [ -z "$(SLUG)" ]; then \ + echo "ISSUE= and SLUG= are required"; \ + exit 2; \ + fi + @. scripts/make_helpers.sh; \ + base=$${BASE:-origin/main}; \ + create_issue_worktree "$(ISSUE)" "$(SLUG)" "$$base" + +# Checkout a PR into an isolated worktree +# Usage: make worktree-pr PR=570 +# make worktree-pr PR=570 REPO=CodingThrust/problem-reductions +worktree-pr: + @if [ -z "$(PR)" ]; then \ + echo "PR= is required"; \ + exit 2; \ + fi + @. scripts/make_helpers.sh; \ + repo=$${REPO:-$$(gh repo view --json nameWithOwner --jq .nameWithOwner)}; \ + checkout_pr_worktree "$$repo" "$(PR)" + # Usage: make run-review (picks next Review pool PR automatically) # make run-review N=570 (processes specific PR) # RUNNER=claude make run-review (use Claude instead of Codex) diff --git a/docs/plans/2026-03-15-pipeline-automation-refactor-plan.md b/docs/plans/2026-03-15-pipeline-automation-refactor-plan.md index 2dafe738a..cfc96a5c8 100644 --- a/docs/plans/2026-03-15-pipeline-automation-refactor-plan.md +++ b/docs/plans/2026-03-15-pipeline-automation-refactor-plan.md @@ -327,16 +327,16 @@ Keep: ### Task 3: Add worktree script -- [ ] Implement PR checkout from forks using pull refs -- [ ] Implement merge-main with structured conflict output -- [ ] Add tests around path/branch naming and result formatting +- [x] Implement PR checkout from forks using pull refs +- [x] Implement merge-main with structured conflict output +- [x] Add tests around path/branch naming and result formatting ### Task 4: Add deterministic review checks -- [ ] Implement `detect-scope` -- [ ] Implement `file-whitelist` +- [x] Implement `detect-scope` +- [x] Implement `file-whitelist` - [ ] Implement `completeness` -- [ ] Point `final-review` and `review-implementation` at the new checks +- [x] Point `final-review` and `review-implementation` at the new checks ### Task 5: Thin the skills diff --git a/scripts/make_helpers.sh b/scripts/make_helpers.sh index 37b2b7827..3049d47bb 100644 --- a/scripts/make_helpers.sh +++ b/scripts/make_helpers.sh @@ -85,6 +85,31 @@ pr_wait_ci() { python3 scripts/pipeline_pr.py wait-ci --repo "$repo" --pr "$pr" --timeout "$timeout" --interval "$interval" --format json } +# --- Worktree helpers --- + +create_issue_worktree() { + issue=$1 + slug=$2 + base=${3:-origin/main} + python3 scripts/pipeline_worktree.py create-issue --issue "$issue" --slug "$slug" --base "$base" --format json +} + +checkout_pr_worktree() { + repo=$1 + pr=$2 + python3 scripts/pipeline_worktree.py checkout-pr --repo "$repo" --pr "$pr" --format json +} + +merge_main_worktree() { + worktree=$1 + python3 scripts/pipeline_worktree.py merge-main --worktree "$worktree" --format json +} + +cleanup_pipeline_worktree() { + worktree=$1 + python3 scripts/pipeline_worktree.py cleanup --worktree "$worktree" --format json +} + # Poll a board column and dispatch a make target when new items appear. # watch_and_dispatch