[PR] feat(skills): adversarial review before every PR a skill opens [magpie]

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk opened a new pull request, #1372:
URL: https://github.com/apache/magpie/pull/1372

   PR 3 of 4, stacked on #1371. This PR's base is `feat/adversarial-review-setup`, so the diff shows only PR 3. Merge #1371 first; this PR then retargets to `main`.
   
   ## Summary
   
   - **A shared pre-PR block** (`tools/dev/blocks/pre-pr-adversarial-review.md`) in every skill that opens a PR, placed where the PR's title and body are final and before the push where the flow allows. Eleven skills carry it, and `release-prepare` carries it at each of its three PRs. `security-issue-import-from-pr`, which verifies a patch someone else proposed, reviews that PR.
   - **Reviewers see only what the PR will publish:** the diff, plus the title and body as posted, after the skill's own public-surface checks.
     - The tool refuses a `--repo-dir` that is the project's private tracker, since reviewers can read every file there. `--allow-tracker-checkout` opts out when the tracker's own code is under review.
     - A skill without an upstream checkout reviews `--target pr:<N>` from an empty temp directory.
     - A change that isn't a local branch is reviewed as a diff file.
   - **When it runs.** Security-family skills run whenever at least one reviewer is configured, whatever the mode. Other skills run only under `mode: on-pr-create`. The review is advisory: unavailable reviewers never stop the flow, a finding the human wants fixed sends the flow back, and findings are untrusted data.
   - **Validator rule.** The skill validator fails a PR-opening skill that lacks the block. It scans Markdown and scripts, including the Python argv form, and keeps explicit, reasoned exemption and delegation lists.
   
   ## Type of change
   
   - [x] Skill change (`.claude/skills/<name>/`) — eval fixtures updated below
   - [x] Python package (`tools/*/` with `pyproject.toml`)
   - [x] CI / dev loop (`prek`, workflows, validators)
   
   ## Test plan
   
   - [x] `prek run --all-files` passes
   - [x] For Python packages touched: `tools/adversarial-review` (143 tests) and `tools/skill-and-tool-validator` (478) pass `pytest`, `ruff check` and `mypy`
   - [x] For skill *behaviour* changes, new eval suites:
     - `security-issue-fix/step-7-adversarial-review` (6 cases): runs under `mode: off` with the posted title and body file, never the tracker's title or a draft that quotes the tracker; a silent skip without config; a one-line skip without the plugin; an instruction inside a finding is not followed; all reviewers unavailable still continues; a finding the user wants fixed stops the flow.
     - `setup-upstream-fix/step-5-adversarial-review` (4 cases): `on-pr-create` runs; `on-demand`, `off` and an empty reviewer list skip silently.
   - [x] Other:
     - `validate_pre_pr_review_block` reports 0 violations on the tree. Unit tests cover prose, sibling files, scripts, the exemption and the delegation.
     - A test pins the block's invocation to the sandbox exclusion pattern.
     - The tool tests cover the tracker-checkout refusal and its opt-out.
   
   ## RFC-AI-0004 compliance
   
   - [x] **HITL**: advisory only. The human decides what to fix, the PR flow is never blocked, and each run keeps its harness prompt.
   - [x] **Privacy LLM**: no private content reaches another model. The input is only what the PR publishes, the tracker checkout is refused, and the host skills run their public-surface checks first.
   - [x] **Write-access discipline**: the block creates nothing; it runs a read-only review before the host skill's own confirmed PR step.
   
   ## Linked issues
   
   Stacked on #1371; follows #1368.
   
   ## Notes for reviewers (optional)
   
   - **Existing evals:** several existing step evals now include the block's text in their extracted step, among them `release-announce-draft/step-3-site-bump`, `security-issue-import-from-pr/step-6-confirm` and `setup-override-upstream/step-6-pr-confirm`. Their fixtures carry no reviewer config, so the block is a silent skip there. They are worth a live re-run.
   - **Release-management steps:** the block says the review adds nothing to the JSON these steps return.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@magpie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(skills): adversarial review before every PR a skill opens [magpie]

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk merged PR #1372:
URL: https://github.com/apache/magpie/pull/1372


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@magpie.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org