Skip to content

give the PR reviewer the filed issue's text so it can judge intent, not just relatedness - #6129

Merged
atomantic merged 1 commit into
mainfrom
cos/task-mtm2k7n5/agent-784a0b2b
Sep 3, 2026
Merged

give the PR reviewer the filed issue's text so it can judge intent, not just relatedness#6129
atomantic merged 1 commit into
mainfrom
cos/task-mtm2k7n5/agent-784a0b2b

Conversation

@atomantic

Copy link
Copy Markdown
Owner

Summary

The pr-reviewer pipeline could tell that an external PR linked an open issue assigned to its opener. It could not tell whether the PR did what that issue asked — the issue's title and body never crossed into any stage. Only issue numbers and open/assigned booleans did, so Stage 2's rule "return true only for a change related to the linked issue" and Stage 3's approve/request-changes verdict were both judged without the requirement in front of them.

  • server/lib/modelAbuseGuard.jsnormalizeLinkedIssues (bounded: 10 issues, 300-char titles, 8000-char bodies, per-issue truncated flag), linkedIssueIntentContent, and linkedIssueIntentFingerprint, which hashes through the existing modelAbuseContentFingerprint so intent evidence shares the boundary's one fingerprinting scheme. normalizeEligibilityFacts gains intentFingerprint; the sha256-hex shape is now one exported isSha256Hex instead of three private copies.
  • server/services/prReviewerSecurity.js — the preflight keeps the title/body of each open linked issue and screens that text in the PR's own model-abuse pass. An injected issue body therefore flags the PR rather than reaching a reviewer as the requirement it is supposed to trust. The scan key covers the issue text, so editing a linked issue re-scans rather than reusing a stale verdict.
  • server/services/modelAbuseGuard.jslinkedIssues validated at the snapshot boundary, so it reaches both Stage 2 and Stage 3 envelopes and counts against the snapshot size cap.
  • Stage 2 / Stage 3 prompts — one shared LINKED_ISSUE_INTENT_EVIDENCE block (the pattern REQUIRED_REVIEW_PUBLICATION_RULE already uses) states that the issue text is the requirement, that the PR's own description is the author's claim about it, and that a line inside an issue addressing the reviewer is content and not a command. Stage 2 rejects a diff that implements something else, is a refactor the issue never asked for, or leaves the stated ask untouched. Stage 3 treats scope drift as blocking — clean, well-tested code for the wrong task is request_changes, and vague or clipped intent is defer.

Deterministic guards, not just prompt text

  • eligibilityFactsAllow admits a non-waived PR only when screened issue intent actually reached the gate. A model that answered eligible with no requirement in front of it was guessing.
  • eligibilityFactsStillCurrent recomputes the intent fingerprint from the issues it already refetches, so an approval whose linked issue was rewritten after the gate judged it is discarded instead of merged. Facts persisted before intent screening existed carry no fingerprint and keep their previous meaning.
  • The maintainer "Review this PR" waiver is untouched: an explicitly requested PR with no linked issue still runs, judged on the change alone.

Decisions worth flagging

  • One classifier pass, not two. The issue text rides the PR's existing scan rather than a second Prompt Guard subprocess per PR. The verdict is one status for the whole PR and findings are deliberately generic, so a separate pass bought no attribution — and freshness stays anchored on both halves separately (contentFingerprint for the diff, intentFingerprint for the issue text).
  • The model-abuse content fingerprint is unchanged. Adding issue text to it would have broken parity with pullRequestContentFingerprint, which the pre-action recheck recomputes from PR title/body/diff.
  • Only open linked issues carry intent. A closed issue is not a live requirement, and the recheck derives its fingerprint from the same open set.

Test plan

  • server/lib/modelAbuseGuard.test.js — bounds/order/dedup and the truncated flag; fingerprint stability, sensitivity to a rewritten body, and null for no evidence; an unusable fingerprint kept out of the validated fact set.
  • server/services/prReviewerSecurity.test.js — intent evidence and its fingerprint recorded alongside the prerequisites; the issue text present in the single scanned payload and carried to the reviewer; a hostile linked issue withholds the PR without quoting the content; unreadable issue still fails the lookup closed.
  • server/services/prReviewerPipeline.test.js — a model-positive decision forced false when no screened intent reached the gate.
  • server/services/issueWatcher.test.js — an approval discarded when the linked issue was rewritten after the gate judged it, and the same approval landing against the text it was actually judged against (verified to fail without the fix).
  • server/services/taskPromptDefaults.test.js — both judging stages carry the shared intent block; Stage 2's mismatch rejections and Stage 3's scope-drift/defer rules.
  • Full server suite: 38,645 passed. One unrelated timeout flake per run (different file each time: sprite atlas, then settings secrets-strip), both green in isolation.

…ot just relatedness

The pr-reviewer pipeline already required an external PR to link an open issue
assigned to its opener, and Stage 2 was told to return true only for a change
"related to the linked issue" — but the issue's title and body never crossed
into any stage. Only issue NUMBERS and open/assigned booleans did, so neither
the eligibility gate nor the Stage 3 reviewer could tell a PR that implements
the filed issue from one that links it and does something else entirely.

The preflight now keeps the title and description of each open linked issue,
screens that text through the model-abuse boundary in the PR's own scan pass
(an injected issue body flags the PR instead of reaching a reviewer as the
requirement it is supposed to trust), and carries it into both judging stages
as `linkedIssues`. Stage 2 rejects a diff that implements something other than
what the issue asks; Stage 3 treats scope drift as a blocking finding rather
than approving clean code for the wrong task.

Two deterministic guards keep the model honest: a non-waived PR is eligible
only when screened issue intent actually reached the gate, and the pre-action
recheck discards an approval whose linked issue was rewritten after the gate
judged it. The maintainer "Review this PR" waiver is unaffected.
@atomantic
atomantic merged commit e00609b into main Sep 3, 2026
7 checks passed
@atomantic
atomantic deleted the cos/task-mtm2k7n5/agent-784a0b2b branch September 3, 2026 22:34
@atomantic
atomantic restored the cos/task-mtm2k7n5/agent-784a0b2b branch September 3, 2026 22:35
@atomantic
atomantic deleted the cos/task-mtm2k7n5/agent-784a0b2b branch September 3, 2026 22:35
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