Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/scripts/enforce-pr-target.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,17 @@ describe("enforce-pr-target workflow", () => {
assert.match(workflow, /collectPrQualityFailures/);
});

it("checks stacked bases via open PR heads before wrong_base enforcement", () => {
assert.match(workflow, /stackedBase/);
assert.match(workflow, /github\.rest\.pulls\.list/);
assert.match(workflow, /treating as stacked/);
const qualityCall = workflow.match(
/collectPrQualityFailures\(\{([\s\S]*?)\}\);/,
);
assert.ok(qualityCall, "must call collectPrQualityFailures");
assert.match(qualityCall[1], /stackedBase/);
});

it("strips stale WRONG BRANCH prefix on failure when base is corrected", () => {
const failureBlock = workflow.match(
/if \(failures\.length > 0\) \{([\s\S]*?)core\.setFailed\(/,
Expand Down
7 changes: 6 additions & 1 deletion .github/scripts/pr-quality.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -134,15 +134,20 @@ function collectPrQualityFailures({
authorPermission,
permissionLookupFailed = false,
ancestryLookupFailed = false,
/** True when baseRef is another open PR's head (stacked child). */
stackedBase = false,
}) {
const failures = [];
const wrongBase = !allowedBases.includes(baseRef);
const wrongBase = !allowedBases.includes(baseRef) && !stackedBase;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve dev-only enforcement for stacked PRs

When stackedBase is true, this condition allows a PR targeting any non-allow-listed branch to pass the required target check, and the subsequent branch also skips ancestry enforcement. An open PR with a matching head therefore permits changes to be merged into a non-dev branch, contrary to the repository policy whose only stated exceptions are releases and maintainer promotions; retain wrong_base enforcement or obtain and document an explicit stacked-PR policy exception.

AGENTS.md reference: AGENTS.md:L132-L133

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept the stacked exemption (intentional review workflow, not a second integration line). Documented it in AGENTS.md: open-parent-head children skip wrong-base; after the parent lands/closes, retarget to dev.

Also addressed the P2s in this push: pull-requests: read on the release workflow, fail-closed takeover lookups (404 on source PR still leaves the line unchanged), and credit-takeovers on both carried + delta notes.

if (wrongBase) {
failures.push({ code: "wrong_base" });
} else {
// Permission lookup fails closed (still evaluate ancestry). Compare API
// failures skip ancestry — zeros would falsely pass the #644 heuristic.
// Stacked children skip ancestry against the integration base; their parent
// PR is the temporary target.
const skipAncestry =
stackedBase ||
ancestryLookupFailed ||
(!permissionLookupFailed && authorHasPushPermission(authorPermission));
if (
Expand Down
35 changes: 35 additions & 0 deletions .github/scripts/pr-quality.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -242,4 +242,39 @@ describe("collectPrQualityFailures", () => {
});
assert.ok(!failures.some((f) => f.code === "wrong_ancestry"));
});

it("skips wrong_base when stackedBase is set", () => {
const failures = collectPrQualityFailures({
baseRef: "feature/parent",
allowedBases: allowed,
body: [
"## Summary",
"This change updates the Windows tray launcher so it resolves CODEX_HOME through the shared helper instead of a hardcoded path.",
"",
"## Test plan",
"- Launch the tray app after setting CODEX_HOME",
"- Confirm the listener and launcher use the same workspace root",
].join("\n"),
behindMain: 0,
behindBase: 0,
aheadMain: 0,
authorPermission: "read",
stackedBase: true,
});
assert.ok(!failures.some((f) => f.code === "wrong_base"));
assert.ok(!failures.some((f) => f.code === "wrong_ancestry"));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

it("still flags wrong_base for non-allow-list bases without stackedBase", () => {
const failures = collectPrQualityFailures({
baseRef: "main",
allowedBases: allowed,
body: "fix stuff",
behindMain: 0,
behindBase: 0,
authorPermission: "read",
stackedBase: false,
});
assert.ok(failures.some((f) => f.code === "wrong_base"));
});
});
36 changes: 35 additions & 1 deletion .github/workflows/enforce-pr-target.yml
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,39 @@ jobs:
let ancestryLookupFailed = false;
const baseAllowed = ALLOWED_BASES.includes(pr.base.ref);

// Stacked PR exception: base is another open PR's head branch (same
// head repo as this PR's base repo). Closed/missing parent stays wrong_base.
let stackedBase = false;
if (!baseAllowed) {
try {
const openPrs = await github.paginate(github.rest.pulls.list, {
owner,
repo,
state: "open",
per_page: 100
});
const baseOwner =
pr.base.repo?.owner?.login ?? owner;
const baseName = pr.base.repo?.name ?? repo;
stackedBase = openPrs.some(
other =>
other.number !== pull_number &&
other.head?.ref === pr.base.ref &&
(other.head?.repo?.owner?.login ?? owner) === baseOwner &&
(other.head?.repo?.name ?? repo) === baseName
);
if (stackedBase) {
core.info(
`Base ${pr.base.ref} matches an open PR head; treating as stacked (skip wrong_base).`
);
}
} catch (error) {
core.warning(
`Could not list open PRs for stacked-base check: ${error.message}`
);
}
}

if (baseAllowed) {
const headSha = pr.head.sha;
try {
Expand Down Expand Up @@ -317,7 +350,8 @@ jobs:
aheadMain,
authorPermission,
permissionLookupFailed,
ancestryLookupFailed
ancestryLookupFailed,
stackedBase
});

if (failures.length > 0) {
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,10 @@ jobs:
pr_notes="$(gh api "${generate_notes_api[@]}" --jq '.body')"
# Drop generate-notes' trailing compare link; we re-append it after the commit list.
printf '%s\n' "$pr_notes" | sed '/^\*\*Full Changelog\*\*:/d' > "$delta_file"
bun scripts/release-notes.ts credit-takeovers \
--repo "$GITHUB_REPOSITORY" \
--in "$delta_file" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Rewrite credits in carried preview notes

When a stable release carries a matching preview created before this helper was introduced, the takeover PR is excluded from the generated delta because notes_range_start advances to the preview tag. This invocation rewrites only delta_file, while the uncorrected carried_file goes directly into assemble, so the stable release still credits only the landing maintainer; run the rewrite over carried content or the final assembled notes as well.

Useful? React with 👍 / 👎.

--out "$delta_file"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Grant read access before querying pull requests

This new command calls GET /repos/{owner}/{repo}/pulls/{number}, but the workflow-level permissions at lines 32–35 grant only contents, actions, and id-token, so the GITHUB_TOKEN has no pull-requests: read permission. The pull-request lookups will be rejected, and because ghJson converts those errors to null, the release still succeeds with every takeover credit unchanged; add the least-privilege pull-requests: read permission and assert it in the workflow test.

Useful? React with 👍 / 👎.

else
# First release on this channel: never call generate-notes without previous_tag_name.
# GitHub would baseline the newest repo tag, which may belong to the other channel.
Expand Down
34 changes: 7 additions & 27 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,33 +101,13 @@ non-trivial change. CI runs these on Linux, Windows, and macOS.
from `dev` (releases, docs deploys). Do not open feature PRs against `main`.
- `preview` — prerelease train (`x.y.z-preview.*` versions).

### The retired `dev2-go` line

The project previously ran a parallel `dev2-go` integration line that was
rebuilding the runtime as a Go native port, and every merge into `dev` had to be
carried onto it. That dual-track policy is over: maintaining two integration
lines cost more than the port returned, and dogfooding the Go runtime kept
surfacing new defects.

`dev2-go` has been deleted, along with the `codex/260728-go-port-*` and
`tmp/dev2-go-source-export` side branches. The full history lives in
[lidge-jun/opencodex-go-archive](https://github.com/lidge-jun/opencodex-go-archive)
and the final tip is tagged `archive/dev2-go` in this repository. There is no
carry or port obligation attached to a `dev` merge any more, and the
`needs-go-port` label is gone.

Bun-native TypeScript is the only runtime line. If native code returns, the
expectation is an incremental module (for example Rust via N-API) landing on
`dev`, not a second full-runtime branch.

The Claude Desktop integration formerly carried on the `claudedesktop` branch is
now fully merged into `dev`, and that branch has been retired. Desktop work
continues as normal pull requests against `dev`.

Porting and rebase pull requests are welcome. Forward-porting a fix from one
integration line to another, or rebasing a stale branch onto the current head,
is ordinary maintenance rather than noise — open it as a normal pull request
and name the source commits in the description.
Bun-native TypeScript on `dev` is the only runtime line. If native code
returns, the expectation is an incremental module (for example Rust via N-API)
landing on `dev`, not a second full-runtime branch.

Rebase pull requests are welcome. Bringing a stale branch onto the current head
is ordinary maintenance — open it as a normal pull request and name the source
commits in the description.

The **`enforce-target`** CI check rejects pull requests whose head
ancestry sits on the **`main`** tip while far behind **`dev`**, and rejects
Expand Down
138 changes: 138 additions & 0 deletions scripts/release-notes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* bun scripts/release-notes.ts matching-preview-tags <version>
* bun scripts/release-notes.ts previous-release-tag <version>
* bun scripts/release-notes.ts has-meaningful [body-file]
* bun scripts/release-notes.ts credit-takeovers --repo <owner/name> --in <file> --out <file>
* bun scripts/release-notes.ts assemble --npm-metadata ... --out ...
*/

Expand Down Expand Up @@ -207,6 +208,69 @@ export function selectNewestCarriedPreviewTag(
return newest;
}

/**
* Parse a maintainer-takeover source PR number from title/body text.
* Matches forms already used in-repo: `takeover of #N`, `takeover #N`,
* `maintainer takeover of #N` (case-insensitive).
*/
export function parseTakeoverSourcePr(title: string, body = ""): number | null {
const text = `${title}\n${body}`;
const match = /\b(?:maintainer\s+)?takeover(?:\s+of)?\s+#(\d+)\b/i.exec(text);
if (!match) return null;
const n = Number(match[1]);
return Number.isInteger(n) && n > 0 ? n : null;
}

const GENERATE_NOTES_PR_LINE =
/^(?<prefix>\* .+? by @)(?<author>[A-Za-z0-9-]+)(?<mid> in https:\/\/github\.com\/[^/\s]+\/[^/\s]+\/pull\/)(?<pr>\d+)(?<suffix>\s*)$/;

export type TakeoverCreditLookup = {
title: string;
body: string;
authorLogin: string;
};

/**
* Rewrite generate-notes lines so maintainer-takeover PRs also credit the
* original PR creator: `by @Original (takeover by @Landing) in …/pull/P`.
*/
export async function rewriteTakeoverCredits(
notesBody: string,
resolveLanding: (prNumber: number) => Promise<TakeoverCreditLookup | null>,
resolveOriginalAuthor: (sourcePrNumber: number) => Promise<string | null>,
): Promise<string> {
const lines = notesBody.replace(/\r\n/g, "\n").split("\n");
const out: string[] = [];
for (const line of lines) {
const match = GENERATE_NOTES_PR_LINE.exec(line);
if (!match?.groups) {
out.push(line);
continue;
}
const landingPr = Number(match.groups.pr);
const landingAuthor = match.groups.author!;
const landing = await resolveLanding(landingPr);
if (!landing) {
out.push(line);
continue;
}
const sourcePr = parseTakeoverSourcePr(landing.title, landing.body);
if (sourcePr == null) {
out.push(line);
continue;
}
const original = await resolveOriginalAuthor(sourcePr);
if (!original || original.toLowerCase() === landingAuthor.toLowerCase()) {
out.push(line);
continue;
}
out.push(
`${match.groups.prefix}${original} (takeover by @${landingAuthor})${match.groups.mid}${match.groups.pr}${match.groups.suffix ?? ""}`,
);
}
return out.join("\n");
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

export function assembleReleaseNotes(input: {
npmMetadata: string;
carriedPreviewNotes?: string;
Expand Down Expand Up @@ -324,6 +388,79 @@ async function main(argv: string[]): Promise<void> {
return;
}

if (cmd === "credit-takeovers") {
const args = new Map<string, string>();
for (let i = 0; i < rest.length; i += 1) {
const key = rest[i];
if (!key?.startsWith("--")) continue;
const value = rest[i + 1];
if (!value || value.startsWith("--")) {
console.error(`Missing value for ${key}`);
process.exit(1);
}
args.set(key.slice(2), value);
i += 1;
}
const repo = args.get("repo");
const inputPath = args.get("in");
const outPath = args.get("out");
if (!repo || !inputPath || !outPath) {
console.error("Usage: bun scripts/release-notes.ts credit-takeovers --repo <owner/name> --in <file> --out <file>");
process.exit(1);
}
const [owner, name] = repo.split("/");
if (!owner || !name || repo.split("/").length !== 2) {
console.error(`Invalid --repo value: ${repo}`);
process.exit(1);
}

async function ghJson(path: string): Promise<unknown | null> {
const proc = Bun.spawn(["gh", "api", path], {
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([
new Response(proc.stdout).text(),
new Response(proc.stderr).text(),
proc.exited,
]);
if (exitCode !== 0) {
console.error(`gh api ${path} failed: ${stderr.trim() || `exit ${exitCode}`}`);
return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fail the release when takeover lookups fail

When GitHub returns a transient error, rate limit, permission error, or malformed response for either the landing or source PR, this path logs the error and returns null; the resolvers then retain the old attribution and the command exits successfully, after which the workflow publishes release notes it never edits. Propagate lookup and JSON errors as a nonzero exit so the release can be retried instead of silently publishing incorrect credits.

AGENTS.md reference: scripts/AGENTS.md:L14-L18

Useful? React with 👍 / 👎.

}
try {
return JSON.parse(stdout) as unknown;
} catch {
console.error(`gh api ${path} returned non-JSON`);
return null;
}
}

const body = await Bun.file(inputPath).text();
const rewritten = await rewriteTakeoverCredits(
body,
async (prNumber) => {
const data = await ghJson(`repos/${owner}/${name}/pulls/${prNumber}`);
if (!data || typeof data !== "object") return null;
const pr = data as { title?: unknown; body?: unknown; user?: { login?: unknown } };
if (typeof pr.title !== "string" || typeof pr.user?.login !== "string") return null;
return {
title: pr.title,
body: typeof pr.body === "string" ? pr.body : "",
authorLogin: pr.user.login,
};
},
async (sourcePrNumber) => {
const data = await ghJson(`repos/${owner}/${name}/pulls/${sourcePrNumber}`);
if (!data || typeof data !== "object") return null;
const pr = data as { user?: { login?: unknown } };
return typeof pr.user?.login === "string" ? pr.user.login : null;
},
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
await Bun.write(outPath, rewritten.endsWith("\n") ? rewritten : rewritten + "\n");
return;
}

if (cmd === "assemble") {
const args = new Map<string, string>();
for (let i = 0; i < rest.length; i += 1) {
Expand Down Expand Up @@ -372,6 +509,7 @@ Usage:
bun scripts/release-notes.ts matching-preview-tag <version> # tags on stdin
bun scripts/release-notes.ts matching-preview-tags <version> # tags on stdin, oldest→newest
bun scripts/release-notes.ts previous-release-tag <version> # tags on stdin
bun scripts/release-notes.ts credit-takeovers --repo <owner/name> --in <file> --out <file>
bun scripts/release-notes.ts assemble --npm-metadata ... --out ...`);
process.exit(1);
}
Expand Down
Loading
Loading