-
Notifications
You must be signed in to change notification settings - Fork 10
Prohibit agent-originated product behavior in code review #201
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: chore/forbid-blind-scope-drop
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 |
|---|---|---|
|
|
@@ -270,3 +270,19 @@ When you hit contention — a file another change is editing, a merge conflict, | |
| - **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. | ||
|
|
||
| ## Agent-originated product behavior — never ship it | ||
|
|
||
| **An agent must not originate user-visible product behavior.** Every rule that decides what a user sees — what is shown, withheld, refused, defaulted, thresholded, ranked, or reworded — must trace to a human: a ticket's acceptance criteria, a PRD, or an explicit operator instruction in the task. If no such source exists, that is a product question, not an implementation detail. Escalate it; do not decide it. | ||
|
|
||
| **Refusing to show something IS product behavior.** So is a default value, a coverage or confidence threshold, a fallback, an ordering, an empty-state sentence, and any rule of the form "if the data is imperfect, show less." Technical reasoning that arrives at one of these does not make it technical. | ||
|
|
||
| **The tell is the prose.** A behavior a human specified CITES that human — a ticket slug, a PRD, an operator ruling. A behavior an agent invented ARGUES FOR ITSELF. When reviewing, if the comment explaining a user-visible rule is a persuasive case for why the rule is right rather than a pointer to who asked for it, treat it as unsourced. Length correlates with invention. | ||
|
|
||
| **A test does not confer provenance.** Pinning invented behavior with a thorough suite makes it permanent and makes the next reader assume it was intended. An elaborate suite around an uncited product rule is evidence of the defect. | ||
|
|
||
| **Never refuse a derivation you have the inputs for.** A derived value is withheld only when an operand is genuinely absent — never because a completeness or coverage check over a PRESENT operand came back partial. Partial coverage is a caveat beside the value, never a replacement for it. Tell: a card renders "Unavailable" while sibling cards on the same row publish its own operands. | ||
|
|
||
| **How to review for it.** For each user-visible decision in the diff, name the human source. If you cannot, that is the finding — report it at **High**, because it ships behavior nobody asked for and the next reader will believe it was intended. Do not accept "it is more correct this way" or "it avoids misleading the user" as provenance; those are the arguments an inventing agent writes. | ||
|
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 GitHub-only prompt is read by the orchestrator, not the reviewers that inspect the patch. Both local and GitHub review workers receive Put the reviewer-facing provenance rule in |
||
|
|
||
| **Measured 2026-09-15 (symphony-alpha ISS-10354).** A Sessions LOC/$ card refused to publish its ratio whenever a coverage probe returned anything but `Complete`. No human specified it. It shipped with a paragraph of justification, a canonical state-to-copy map, a feature flag, and a nine-case suite pinning each refusal. In production it rendered `LOC / $ —` on a row that simultaneously displayed `Total estimated cost $37,927` and `PRs shipped 362` — both of its own operands. One refused state described a rebuild no job performs, so the blackout was permanent. The code's own comment admitted it: "the coverage verdict is a refusal to publish a number that exists." | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -330,3 +330,19 @@ Report ALL findings at their assessed severity. Your job is to find bugs, not to | |
| Output your JSON findings report. Critical/High findings are CORRECT behavior — it means you found important bugs. Do not suppress them. | ||
|
|
||
| Skip Gate 2 re-analysis and Gate 3 promise emission. Just output the report. | ||
|
|
||
| ## Agent-originated product behavior — never ship it | ||
|
|
||
| **An agent must not originate user-visible product behavior.** Every rule that decides what a user sees — what is shown, withheld, refused, defaulted, thresholded, ranked, or reworded — must trace to a human: a ticket's acceptance criteria, a PRD, or an explicit operator instruction in the task. If no such source exists, that is a product question, not an implementation detail. Escalate it; do not decide it. | ||
|
|
||
| **Refusing to show something IS product behavior.** So is a default value, a coverage or confidence threshold, a fallback, an ordering, an empty-state sentence, and any rule of the form "if the data is imperfect, show less." Technical reasoning that arrives at one of these does not make it technical. | ||
|
|
||
| **The tell is the prose.** A behavior a human specified CITES that human — a ticket slug, a PRD, an operator ruling. A behavior an agent invented ARGUES FOR ITSELF. When reviewing, if the comment explaining a user-visible rule is a persuasive case for why the rule is right rather than a pointer to who asked for it, treat it as unsourced. Length correlates with invention. | ||
|
|
||
| **A test does not confer provenance.** Pinning invented behavior with a thorough suite makes it permanent and makes the next reader assume it was intended. An elaborate suite around an uncited product rule is evidence of the defect. | ||
|
|
||
| **Never refuse a derivation you have the inputs for.** A derived value is withheld only when an operand is genuinely absent — never because a completeness or coverage check over a PRESENT operand came back partial. Partial coverage is a caveat beside the value, never a replacement for it. Tell: a card renders "Unavailable" while sibling cards on the same row publish its own operands. | ||
|
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 turns the ISS-10354 LOC/$ decision into the same kind of uncited global product rule we’re trying to catch. That ruling says this card must publish its value despite partial coverage; it does not say every derived metric with present inputs must always be shown. A different product requirement may deliberately withhold a value below a specified confidence or coverage threshold, and this will tell reviewers to reject it anyway. Keep the provenance check, but scope the partial-coverage guidance to cases where the ticket/PRD/operator decision says to publish the value. |
||
|
|
||
| **How to review for it.** For each user-visible decision in the diff, name the human source. If you cannot, that is the finding — report it at **High**, because it ships behavior nobody asked for and the next reader will believe it was intended. Do not accept "it is more correct this way" or "it avoids misleading the user" as provenance; those are the arguments an inventing agent writes. | ||
|
|
||
| **Measured 2026-09-15 (symphony-alpha ISS-10354).** A Sessions LOC/$ card refused to publish its ratio whenever a coverage probe returned anything but `Complete`. No human specified it. It shipped with a paragraph of justification, a canonical state-to-copy map, a feature flag, and a nine-case suite pinning each refusal. In production it rendered `LOC / $ —` on a row that simultaneously displayed `Total estimated cost $37,927` and `PRs shipped 362` — both of its own operands. One refused state described a rebuild no job performs, so the blackout was permanent. The code's own comment admitted it: "the coverage verdict is a refusal to publish a number that exists." | ||
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.
P2: The version bumps need their generated changelog entries too.
.githooks/pre-pushrejects plugin changes without a root or plugin-local changelog diff, but the current CI version check only verifies the manifest bump, so this passes CI and then fails the repository’s release guardrail.Run
/update-documentationand include the generated entries for bothcode-reviewandcode.