Skip to content

feat: add evidence-backed false-positive rules to review-pr skill - #46

Draft
captainsafia wants to merge 1 commit into
mainfrom
update-review-pr-false-positive-rules
Draft

feat: add evidence-backed false-positive rules to review-pr skill#46
captainsafia wants to merge 1 commit into
mainfrom
update-review-pr-false-positive-rules

Conversation

@captainsafia

@captainsafia captainsafia commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Refines the shared review-pr skill using real reviewer feedback aggregated by the update-pr-review workflow over the recent warp-server PR window. The analysis surfaced recurring cases where humans overrode the review bot (oz-for-oss / warp-dev-github-integration) as false positives, while its security/correctness findings (shell injection, error leakage, lock races) were consistently validated and addressed — so this change targets precision, not weakening genuine checks.

Changes

Adds three evidence-backed bullets to the Review Scope section:

  1. Generated files — don't review *.gen.go / generated-header files as hand-maintained; renamed identifiers are intended codegen output. (Evidence: warp-server #13249, maintainer: @captainsafia — "This isn't actually a real issue. This is the types generator...")
  2. Reachability / global safeguards — verify a state-dependent bug is actually reachable given current control flow and existing global safeguards before flagging; downgrade hypothetical-future-caller concerns to optional defense-in-depth. (Evidence: #13201 and #13175, reviewer @johnturcoo.)
  3. Authorization / data-scoping — trace whether the identifier is already viewer/principal-scoped and account for domain invariants before asserting a security hole; raise unverified concerns as open questions instead of inline 🚨/⚠️ assertions. (Evidence: #13184 ×3, reviewer @yuanben-warp.)

Notes

  • All qualifying feedback came from code PRs (0 spec-only PRs), so review-spec is intentionally unchanged.
  • The review-pr skill lives here in common-skills (migrated out of warp-server); the update-pr-review workflow that produced this analysis still lives in warp-server.

cc @captainsafia


Conversation: https://staging.warp.dev/conversation/88890f9f-352d-4594-840c-a75b2db92c33
Run: https://oz.staging.warp.dev/runs/019f94a4-1135-79f6-b922-a465ed938d40

This PR was generated with Oz.

Derived from human overrides of the review bot on recent warp-server code
PRs (via the update-pr-review workflow). Adds three review-precision rules:
- Do not treat generated files (*.gen.go, generated headers) as
  hand-maintained; codegen renames are intended output, not defects.
- Verify a state-dependent bug is reachable given current control flow and
  existing global safeguards before flagging; downgrade hypothetical-future
  concerns to optional defense-in-depth suggestions.
- Trace the viewer/principal-scoped entry path and domain invariants before
  asserting an authorization/data-scoping security hole; raise unverified
  concerns as open questions instead of inline CRITICAL/IMPORTANT assertions.

These improve precision without weakening genuine security/correctness checks.

Co-Authored-By: Oz <oz-agent@warp.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant