Skip to content

Commit 0770147

Browse files
docs(agents): never commit review artifacts to a contributor's fork (#286)
The artifact rule said "PR review -> on the PR branch" without qualifying who owns it, so reviewing a community PR meant pushing to the contributor's branch. That happened on #274 and #277 this week; both were reverted. maintainerCanModify makes the push possible; it is not permission. It also backfires mechanically: this repo dismisses stale reviews on push, so the review-doc commit dismisses the approval it documents and staleness-flags the approval label against the new head. Fork PRs now have no committed artifact — the formal review body is the artifact. Closes #286
1 parent 5a9dac0 commit 0770147

1 file changed

Lines changed: 22 additions & 1 deletion

File tree

AGENTS.md

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,33 @@ cache-fix-specific context.
1919
This repo uses **`docs/code-reviews/`** (not `docs/reviews/` — older
2020
convention retained for continuity). Commit review documents there:
2121

22-
- **PR review** → on the PR branch as
22+
- **PR review, PR branch owned by this org** → on the PR branch as
2323
`docs/code-reviews/pr-<N>-round-<R>-codex.md`
24+
- **PR review, PR from a contributor's fork****do not push anything to
25+
the fork.** See below.
2426
- **Issue / directive review** → on a review branch (or the directive's
2527
branch if one exists) as
2628
`docs/code-reviews/issue-<N>-round-<R>-codex.md`
2729

30+
### Never write to a contributor's fork
31+
32+
When the PR head is a fork you do not own, the review artifact does **not**
33+
get committed anywhere. Post the review body in the formal `gh pr review`
34+
and stop there — that is the artifact.
35+
36+
`maintainerCanModify: true` makes pushing mechanically possible on most
37+
community PRs. It is not permission. A contributor's branch is theirs; a
38+
push you make to it rewrites history they may have local work on, and it
39+
puts our internal review notes into their PR's file diff where they do not
40+
belong.
41+
42+
There is also a mechanical cost: this repo dismisses stale reviews on push,
43+
so a review-doc commit **dismisses the very approval it documents**, and
44+
any `approved-by-*` label instantly goes stale against the new head.
45+
46+
Check before writing: if `gh pr view <N> --json headRepositoryOwner` is not
47+
this org, no commit.
48+
2849
## What cache-fix Is
2950

3051
The proxy that replaces the Node.js `--import` preload interceptor

0 commit comments

Comments
 (0)