feat(review-pr): require evidence for security and unwired-UI findings - #47
Draft
captainsafia wants to merge 1 commit into
Draft
feat(review-pr): require evidence for security and unwired-UI findings#47captainsafia wants to merge 1 commit into
captainsafia wants to merge 1 commit into
Conversation
Derived from a week of human replies to agent review comments on
warpdotdev/warp-server.
Reviewers rejected speculative sensitive-data and input-shape findings
("this error string could contain secrets", "this identifier could
contain a path separator") when the producing code made the hazardous
input impossible, and repeatedly answered "intentional" to IMPORTANT
findings about buttons and menu items that had no handler yet.
Co-Authored-By: Oz <oz-agent@warp.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ran the
update-pr-reviewskill over the last 7 days ofwarpdotdev/warp-serverpull requests (100 PRs, 82 review threads, 13 agent review comments that drew human replies) and folded the repeated reviewer signals back into the sharedreview-prskill.All PRs in the window were code PRs (
review_type: "code"), soreview-specis unchanged.Signals
1. Speculative security findings get rejected (3 threads, 3 reviewers).
⚠️ [IMPORTANT] [SECURITY]claimed logging raw errors could leak secrets.@seemeroland: "I think we need to be able to log errs. If it turns out something sensitive is actually included we should remove it from the err".@seemeroland: "I think this is fine".⚠️ [IMPORTANT]asked forencodeURIComponenton an agent UID.@johnturcoo: "agent IDs are uuid and uniform".In each case the bot asserted a hazard from a value's hypothetical shape without tracing how the value is actually produced. The new rule asks for that trace before filing the finding, and routes unprovable concerns to top-level
bodyinstead of an asserted[SECURITY]inline comment.This does not weaken genuine findings: the evidence-backed security comments in the same window were accepted (#13583 cross-team suite ownership — "Should be fixed now"; #13646 empty-owner authorization — "I think this is kind of valid. Will push a change to address").
2. Unwired UI elements are deliberate placeholders (3 threads, 2 PRs, 1 reviewer).
FactoryAgentEditor.tsx— Reconnect button with noonPress.@johnturcoo: "intentional".index.tsx— New button with noonPress. "Intentional".AgentActions.tsx— Copy-as-JSON menu item with no handler. "Intentional".Three
[IMPORTANT]/[SUGGESTION]comments, three identical dismissals. This extends the existing V0/initial-implementation rule to UI scaffolding.Changes
Two bullets added to
## Review Scopein.agents/skills/review-pr/SKILL.md. No change to the output schema, severity labels, safety rules, or diff-annotation contract.cc @captainsafia
Conversation: https://staging.warp.dev/conversation/dc738e5e-a8e5-4566-b2d4-29ee96789b37
Run: https://oz.staging.warp.dev/runs/019fb8b0-9544-7b70-9887-85d1d17593cc
This PR was generated with Oz.