Skip to content

Commit 45e9dc4

Browse files
authored
fix(ci): credit takeover authors and exempt stacked PR bases (#742)
* fix(ci): credit takeover authors and exempt stacked PR bases Release notes attributed maintainer takeovers to the landing author only, and enforce-pr-target drafted stacked children that correctly targeted a parent head. Rewrite takeover credits to name the original creator, skip wrong-base for open stacked parents, and drop the retired dual-track essay from AGENTS.md so agent guidance stays current-policy only. * fix(release): harden takeover credits and document stacked PR exception Grant pull-requests:read for PR author lookups, fail closed on non-404 gh api errors, rewrite carried preview notes as well as the delta, and record the stacked-child wrong-base exemption in AGENTS.md. * fix(ci): address CodeRabbit follow-ups on takeover credits and stacked PRs Tighten stacked ancestry regression inputs, skip non-takeover gh lookups, tolerate missing landing PRs, and paginate open-PR fixtures so stacked parents beyond page one stay covered.
1 parent 0666b41 commit 45e9dc4

10 files changed

Lines changed: 582 additions & 37 deletions

.github/scripts/enforce-pr-target.test.cjs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,17 @@ describe("enforce-pr-target workflow", () => {
6161
assert.match(workflow, /collectPrQualityFailures/);
6262
});
6363

64+
it("checks stacked bases via open PR heads before wrong_base enforcement", () => {
65+
assert.match(workflow, /stackedBase/);
66+
assert.match(workflow, /github\.rest\.pulls\.list/);
67+
assert.match(workflow, /treating as stacked/);
68+
const qualityCall = workflow.match(
69+
/collectPrQualityFailures\(\{([\s\S]*?)\}\);/,
70+
);
71+
assert.ok(qualityCall, "must call collectPrQualityFailures");
72+
assert.match(qualityCall[1], /stackedBase/);
73+
});
74+
6475
it("strips stale WRONG BRANCH prefix on failure when base is corrected", () => {
6576
const failureBlock = workflow.match(
6677
/if \(failures\.length > 0\) \{([\s\S]*?)core\.setFailed\(/,

.github/scripts/pr-quality.cjs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,15 +134,20 @@ function collectPrQualityFailures({
134134
authorPermission,
135135
permissionLookupFailed = false,
136136
ancestryLookupFailed = false,
137+
/** True when baseRef is another open PR's head (stacked child). */
138+
stackedBase = false,
137139
}) {
138140
const failures = [];
139-
const wrongBase = !allowedBases.includes(baseRef);
141+
const wrongBase = !allowedBases.includes(baseRef) && !stackedBase;
140142
if (wrongBase) {
141143
failures.push({ code: "wrong_base" });
142144
} else {
143145
// Permission lookup fails closed (still evaluate ancestry). Compare API
144146
// failures skip ancestry — zeros would falsely pass the #644 heuristic.
147+
// Stacked children skip ancestry against the integration base; their parent
148+
// PR is the temporary target.
145149
const skipAncestry =
150+
stackedBase ||
146151
ancestryLookupFailed ||
147152
(!permissionLookupFailed && authorHasPushPermission(authorPermission));
148153
if (

.github/scripts/pr-quality.test.cjs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,4 +242,39 @@ describe("collectPrQualityFailures", () => {
242242
});
243243
assert.ok(!failures.some((f) => f.code === "wrong_ancestry"));
244244
});
245+
246+
it("skips wrong_base when stackedBase is set", () => {
247+
const failures = collectPrQualityFailures({
248+
baseRef: "feature/parent",
249+
allowedBases: allowed,
250+
body: [
251+
"## Summary",
252+
"This change updates the Windows tray launcher so it resolves CODEX_HOME through the shared helper instead of a hardcoded path.",
253+
"",
254+
"## Test plan",
255+
"- Launch the tray app after setting CODEX_HOME",
256+
"- Confirm the listener and launcher use the same workspace root",
257+
].join("\n"),
258+
behindMain: 0,
259+
behindBase: 44,
260+
aheadMain: 1,
261+
authorPermission: "read",
262+
stackedBase: true,
263+
});
264+
assert.ok(!failures.some((f) => f.code === "wrong_base"));
265+
assert.ok(!failures.some((f) => f.code === "wrong_ancestry"));
266+
});
267+
268+
it("still flags wrong_base for non-allow-list bases without stackedBase", () => {
269+
const failures = collectPrQualityFailures({
270+
baseRef: "main",
271+
allowedBases: allowed,
272+
body: "fix stuff",
273+
behindMain: 0,
274+
behindBase: 0,
275+
authorPermission: "read",
276+
stackedBase: false,
277+
});
278+
assert.ok(failures.some((f) => f.code === "wrong_base"));
279+
});
245280
});

.github/workflows/enforce-pr-target.yml

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,39 @@ jobs:
281281
let ancestryLookupFailed = false;
282282
const baseAllowed = ALLOWED_BASES.includes(pr.base.ref);
283283
284+
// Stacked PR exception: base is another open PR's head branch (same
285+
// head repo as this PR's base repo). Closed/missing parent stays wrong_base.
286+
let stackedBase = false;
287+
if (!baseAllowed) {
288+
try {
289+
const openPrs = await github.paginate(github.rest.pulls.list, {
290+
owner,
291+
repo,
292+
state: "open",
293+
per_page: 100
294+
});
295+
const baseOwner =
296+
pr.base.repo?.owner?.login ?? owner;
297+
const baseName = pr.base.repo?.name ?? repo;
298+
stackedBase = openPrs.some(
299+
other =>
300+
other.number !== pull_number &&
301+
other.head?.ref === pr.base.ref &&
302+
(other.head?.repo?.owner?.login ?? owner) === baseOwner &&
303+
(other.head?.repo?.name ?? repo) === baseName
304+
);
305+
if (stackedBase) {
306+
core.info(
307+
`Base ${pr.base.ref} matches an open PR head; treating as stacked (skip wrong_base).`
308+
);
309+
}
310+
} catch (error) {
311+
core.warning(
312+
`Could not list open PRs for stacked-base check: ${error.message}`
313+
);
314+
}
315+
}
316+
284317
if (baseAllowed) {
285318
const headSha = pr.head.sha;
286319
try {
@@ -317,7 +350,8 @@ jobs:
317350
aheadMain,
318351
authorPermission,
319352
permissionLookupFailed,
320-
ancestryLookupFailed
353+
ancestryLookupFailed,
354+
stackedBase
321355
});
322356
323357
if (failures.length > 0) {

.github/workflows/release.yml

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ on:
3232
permissions:
3333
contents: write # create the matching GitHub Release + version tag after npm publish
3434
actions: read # verify the release commit already passed Cross-platform CI
35+
pull-requests: read # credit-takeovers looks up landing/source PR authors via gh api
3536
id-token: write # OIDC auth for Trusted Publishing + automatic provenance attestation
3637

3738
concurrency:
@@ -399,6 +400,22 @@ jobs:
399400
echo "::notice::No previous channel tag; skipping generate-notes (commits-only notes)"
400401
fi
401402
403+
# Rewrite takeover credits on both carried preview notes and the since-preview
404+
# delta. Carried bodies may predate this helper and would otherwise keep
405+
# landing-author-only attribution on stable releases.
406+
if [ -s "$carried_file" ]; then
407+
bun scripts/release-notes.ts credit-takeovers \
408+
--repo "$GITHUB_REPOSITORY" \
409+
--in "$carried_file" \
410+
--out "$carried_file"
411+
fi
412+
if [ -s "$delta_file" ]; then
413+
bun scripts/release-notes.ts credit-takeovers \
414+
--repo "$GITHUB_REPOSITORY" \
415+
--in "$delta_file" \
416+
--out "$delta_file"
417+
fi
418+
402419
if [ -n "$notes_range_start" ]; then
403420
commit_range="${notes_range_start}..${GITHUB_SHA}"
404421
git log --pretty=format:'- %s (%h)' "$commit_range" > "$commits_file"

AGENTS.md

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -101,33 +101,18 @@ non-trivial change. CI runs these on Linux, Windows, and macOS.
101101
from `dev` (releases, docs deploys). Do not open feature PRs against `main`.
102102
- `preview` — prerelease train (`x.y.z-preview.*` versions).
103103

104-
### The retired `dev2-go` line
105-
106-
The project previously ran a parallel `dev2-go` integration line that was
107-
rebuilding the runtime as a Go native port, and every merge into `dev` had to be
108-
carried onto it. That dual-track policy is over: maintaining two integration
109-
lines cost more than the port returned, and dogfooding the Go runtime kept
110-
surfacing new defects.
111-
112-
`dev2-go` has been deleted, along with the `codex/260728-go-port-*` and
113-
`tmp/dev2-go-source-export` side branches. The full history lives in
114-
[lidge-jun/opencodex-go-archive](https://github.com/lidge-jun/opencodex-go-archive)
115-
and the final tip is tagged `archive/dev2-go` in this repository. There is no
116-
carry or port obligation attached to a `dev` merge any more, and the
117-
`needs-go-port` label is gone.
118-
119-
Bun-native TypeScript is the only runtime line. If native code returns, the
120-
expectation is an incremental module (for example Rust via N-API) landing on
121-
`dev`, not a second full-runtime branch.
122-
123-
The Claude Desktop integration formerly carried on the `claudedesktop` branch is
124-
now fully merged into `dev`, and that branch has been retired. Desktop work
125-
continues as normal pull requests against `dev`.
126-
127-
Porting and rebase pull requests are welcome. Forward-porting a fix from one
128-
integration line to another, or rebasing a stale branch onto the current head,
129-
is ordinary maintenance rather than noise — open it as a normal pull request
130-
and name the source commits in the description.
104+
Bun-native TypeScript on `dev` is the only runtime line. If native code
105+
returns, the expectation is an incremental module (for example Rust via N-API)
106+
landing on `dev`, not a second full-runtime branch.
107+
108+
Stacked child pull requests that target another **open** PR's head branch are
109+
an intentional review workflow, not an alternate integration line. The
110+
**`enforce-target`** check skips the wrong-base gate for those children; after
111+
the parent lands or closes, retarget the child to `dev`.
112+
113+
Rebase pull requests are welcome. Bringing a stale branch onto the current head
114+
is ordinary maintenance — open it as a normal pull request and name the source
115+
commits in the description.
131116

132117
The **`enforce-target`** CI check rejects pull requests whose head
133118
ancestry sits on the **`main`** tip while far behind **`dev`**, and rejects

0 commit comments

Comments
 (0)