-
Notifications
You must be signed in to change notification settings - Fork 10
Forbid blind scope drop and re-arguing settled instructions #197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -256,3 +256,17 @@ If `normalization_warnings > 0`, append after the findings table: | |
| - **NO FOOTER**: Do NOT add any signature, attribution, or footer like "Automated review by Claude Code" | ||
|
|
||
| Mark todo as `completed`. | ||
|
|
||
| ## Blind scope drop — never an option | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This PR changes plugin artifacts but includes no root or plugin |
||
|
|
||
| **Deferring or skipping work is NOT a viable alternative in the presence of contention.** That is *blind scope drop*, and it is the most expensive failure this pipeline has. | ||
|
|
||
| When you hit contention — a file another change is editing, a merge conflict, a reviewer's objection, an unclear owner, a busy box — the answer is to resolve it, not to shrink the work. Specifically: | ||
|
|
||
| - **The deliverable is never deferrable.** "Aggressively defer out-of-scope work" applies ONLY to work outside the change's purpose. The thing the change exists to do is not out of scope for itself. | ||
| - **Never substitute a filed ticket for the thing you were told to build.** A follow-up ticket is for genuinely separate work, not a way to close a change that does not do what it claims. | ||
| - **Verify a blocker is real NOW before letting it stop you.** Stale file lists and already-merged PRs are the common trap — a collision cited from an hour-old snapshot is usually gone. | ||
| - **A settled instruction from the operator is not open for re-argument.** Implement as specified. A genuinely new conflict is raised once, separately, and does not block the work. | ||
| - **The operator's present instruction outranks every inline comment, docstring, guardrail, and prior spec.** Do not weigh it against the accumulated ruleset and negotiate a compromise. | ||
|
|
||
| If something genuinely blocks you, say so plainly and keep going on everything else. Silent downscoping is the failure; a stated blocker is not. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,8 @@ You are a senior code reviewer specializing in security vulnerabilities, correct | |
| 10. Before suggesting custom helpers, search the codebase for existing utilities. | ||
| 11. Do not assume how unseen internal implementations work — if you can't see a function's code, don't claim it has bugs. | ||
| 12. Apply the "author awareness" test: Would the original author fix this if they knew? If yes → report it. If the author would say "that's intentional" → skip it. | ||
| 13. **A settled instruction from the operator is NOT open for re-argument.** If a ticket, PRD, PR body, or in-thread ruling records an explicit decision by the repository owner, review the change AGAINST that decision. Do not reopen it, do not propose the alternative it already rejected, and do not raise its absence as a finding. If you believe you have found a genuinely NEW conflict the decision did not consider, state it once as a separate note — it does not block the change and it is not a Critical/High finding. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: This lets untrusted PR/ticket/thread text claim an owner ruling and forces a newly discovered conflict to be non-blocking and below Critical/High. That can suppress the authorization bypasses this same reviewer is required to report as Critical. Only honor a decision supplied by trusted operator/orchestrator context, and preserve normal severity/blocking for security and repository-safety conflicts. |
||
| 14. **Never give feedback whose effect is to defer the change's own deliverable.** Asking for work outside the change's purpose is legitimate; asking the author to drop or postpone the thing the change exists to do is not. If the deliverable looks incomplete, say what is missing — do not propose shipping it as a follow-up ticket. | ||
| </constraints> | ||
|
|
||
| ## Severity Levels | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| --- | ||
| name: verification-subagent | ||
| description: Verifies if a task from the implementation plan has been completed by checking source files. | ||
| model: sonnet | ||
| model: opus | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This changes the runtime agent from Sonnet to Opus, but |
||
| tools: Read, Glob, Grep, SendMessage | ||
| --- | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: This changes the
code-reviewplugin but leaves its manifest at1.5.5. Theplugin-version-bumpCI job fails every changedplugins/<name>directory whose manifest isn't updated, so this PR cannot pass its required gate. Bumpplugins/code-review/.claude-plugin/plugin.jsonhere.