Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
8 changes: 6 additions & 2 deletions .github/workflows/quality.yml
Original file line number Diff line number Diff line change
Expand Up @@ -230,16 +230,20 @@ jobs:
route-guard-membership test-discovery test-runner-api
mutation-test-coverage any-budget:t11 build-scope pack-policy
complexity-ratchets model-lifecycle
cycles lockfile duplication dead-code type-coverage compression-budget
lockfile duplication dead-code type-coverage compression-budget
# #8781: open-sse workspace typecheck gate — the workspace imports @/ which
# escapes to src/ via undeclared path aliases. See check-open-sse-typecheck.mjs.
open-sse-typecheck
# Hard Rule #16 — AI/bot attribution in PR commits, title or body (#14436). Reads the PR
# from GITHUB_EVENT_PATH; no-op on non-PR events. ci.yml only runs on PRs to main.
ai-attribution
)
# Ratchet-mode gates append --ratchet. `cycles` belongs here since #15281
# (69cb18dca8): check-cycles.mjs exits 1 on ANY cycle without --ratchet, and
# the frozen ceiling lives in quality-baseline.json → metrics.cycles — same
# `check:cycles:ratchet` semantics ci.yml runs (base-red #15306).
ratchet_gates=(
secrets vuln-ratchet workflows openapi-breaking
secrets vuln-ratchet workflows openapi-breaking cycles
)
failed=()
for g in "${gates[@]}"; do
Expand Down
3 changes: 3 additions & 0 deletions changelog.d/fixes/15590-drain-v3852-base-reds.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- **fix(ci):** the Fast Quality Gates loop runs the import-cycles gate in ratchet mode (`--ratchet`, ceiling `quality-baseline.json` → `metrics.cycles`) like ci.yml already does, so PRs against the release branch stop failing on the frozen 14-SCC baseline that #15281 turned into a hard exit (#15590)
- **fix(quality):** new-code ratchet gates (complexity, dead-code) diff against the GitHub merge ref's first parent — the live base tip — instead of the stale `pull_request.base.sha`, so base commits landing after a PR is opened are no longer counted as the PR's new code (#15590)
- **fix(dev):** the dev server runner (`run-next.mjs`) retries its listen with bounded backoff on `EADDRINUSE`, ending the integration-suite port race where a leftover socket claims the harness port during the ~15-18s `prepare()` window and kills the boot (#15590)
30 changes: 26 additions & 4 deletions scripts/check/newCodeMode.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,34 @@ function git(args, opts = {}) {
}).trim();
}

/** Merge-base between the PR base ref and HEAD (falls back to the ref itself). */
export function resolveMergeBase(baseRef) {
/**
* Merge-base between the PR base ref and HEAD (falls back to the ref itself).
*
* GitHub checks PRs out at `refs/pull/N/merge`: a merge commit whose FIRST parent
* is the live base tip at job time. `github.event.pull_request.base.sha` stays at
* the commit the PR was OPENED against, so once the base moves,
* `merge-base(ref, HEAD)` silently diffs the base's own newer commits onto the PR
* (#15346/#15322 — a #12740 base change blamed on unrelated PRs). When HEAD is a
* merge commit and `baseRef` is an ancestor of HEAD^1, compare against HEAD^1:
* that diff is exactly the PR's new code, matching GitHub's "Files changed" view.
*/
export function resolveMergeBase(baseRef, { cwd = ROOT } = {}) {
try {
const parents = git(["rev-list", "--parents", "-n", "1", "HEAD"], { cwd }).split(/\s+/);
const firstParent = parents[1];
if (parents.length > 2 && firstParent) {
// Throws (exit 1) when baseRef is NOT an ancestor of the live base tip —
// i.e. a local diverged branch, where merge-base semantics still apply.
git(["merge-base", "--is-ancestor", baseRef, firstParent], { cwd });
return firstParent;
}
} catch {
/* not a merge commit, or baseRef not an ancestor — fall through */
}
try {
return git(["merge-base", baseRef, "HEAD"]);
return git(["merge-base", baseRef, "HEAD"], { cwd });
} catch {
return git(["rev-parse", baseRef]);
return git(["rev-parse", baseRef], { cwd });
}
}

Expand Down
53 changes: 53 additions & 0 deletions scripts/dev/listen-with-retry.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// scripts/dev/listen-with-retry.mjs
// net.Server.listen with a bounded EADDRINUSE retry.
//
// The dev server binds its port only AFTER next prepare() (~15-18s), while test
// harnesses pick the port with a bind(0)-release at module load. Anything that
// claims the port inside that window (the previous test file's server still
// tearing down, a leftover process) used to kill the boot with EADDRINUSE and
// red the whole integration job. The conflicts this window actually produces
// are transient, so retry; a port that is still busy after the last attempt
// rejects with the original error, so a genuine conflict still fails loudly.

/**
* Listen on `port`/`host`, retrying while the error is EADDRINUSE.
*
* @param {import("node:net").Server} server
* @param {{ port: number, host?: string, attempts?: number, delayMs?: number,
* log?: (msg: string) => void }} options
* `attempts` is the total number of tries (default 6 — 1 try + 5 retries).
* @returns {Promise<import("node:net").AddressInfo | string | null>} the bound
* address (post-listen errors are the caller's business again).
*/
export function listenWithRetry(server, options) {
const { port, host, attempts = 6, delayMs = 5000, log = console.warn } = options;
let triesLeft = attempts;
return new Promise((resolve, reject) => {
const listen = () => {
const onListen = () => {
cleanup();
resolve(server.address());
};
const onError = (error) => {
cleanup();
triesLeft -= 1;
if (error?.code !== "EADDRINUSE" || triesLeft <= 0) {
reject(error);
return;
}
log(
`[listen] port ${port} busy (EADDRINUSE) — retrying in ${delayMs}ms (${triesLeft} attempt(s) left)`
);
setTimeout(listen, delayMs);
};
const cleanup = () => {
server.removeListener("listening", onListen);
server.removeListener("error", onError);
};
server.once("listening", onListen);
server.once("error", onError);
server.listen(port, host);
};
listen();
});
}
32 changes: 20 additions & 12 deletions scripts/dev/run-next.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { randomUUID } from "node:crypto";
import { getMainServerTimeoutConfig } from "./main-server-timeouts.mjs";
import { createSystemdNotifier } from "./systemd-notify.mjs";
import { attachRequestStreamGuards, installProcessCrashGuard } from "./httpClientAbortGuard.mjs";
import { listenWithRetry } from "./listen-with-retry.mjs";

const { maybeHandleDisallowedMethod } = methodGuard;
const { wrapRequestListenerWithHeadResponseGuard } = headResponseGuard;
Expand Down Expand Up @@ -230,11 +231,6 @@ async function start() {
}
});

server.on("error", (error) => {
console.error("[FATAL] Next custom server failed:", error);
process.exit(1);
});

let isShuttingDown = false;
const shutdown = async (signal) => {
if (isShuttingDown) {
Expand Down Expand Up @@ -267,14 +263,26 @@ async function start() {
process.on("SIGINT", () => void shutdown("SIGINT"));
process.on("SIGTERM", () => void shutdown("SIGTERM"));

server.listen(dashboardPort, hostname, () => {
const bundler = dev ? (useTurbopack ? "turbopack" : "webpack") : "production";
console.log(
`[Next] ${mode} server listening on http://${hostname}:${dashboardPort} (${bundler})`
);
systemdNotifier.ready();
systemdNotifier.startWatchdog();
// Bind with a bounded EADDRINUSE retry: test harnesses pick this port with a
// bind(0)-release at module load, ~15-18s before prepare() finishes, so a
// transient occupant must not kill the boot (base-red #15306).
try {
await listenWithRetry(server, { port: dashboardPort, host: hostname });
} catch (error) {
console.error("[FATAL] Next custom server failed:", error);
process.exit(1);
}
// Post-listen server errors stay fatal — during-listen ones were the retry helper's.
server.on("error", (error) => {
console.error("[FATAL] Next custom server failed:", error);
process.exit(1);
});
const bundler = dev ? (useTurbopack ? "turbopack" : "webpack") : "production";
console.log(
`[Next] ${mode} server listening on http://${hostname}:${dashboardPort} (${bundler})`
);
systemdNotifier.ready();
systemdNotifier.startWatchdog();
}

start().catch((error) => {
Expand Down
82 changes: 80 additions & 2 deletions tests/unit/build/new-code-mode.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import assert from "node:assert/strict";
import { execFileSync } from "node:child_process";
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
Expand All @@ -11,8 +12,78 @@ import {
filterScope,
newDeadSymbols,
perFileRuleCounts,
resolveMergeBase,
} from "../../../scripts/check/newCodeMode.mjs";

/**
* Build a temp repo shaped like GitHub's PR checkout (refs/pull/N/merge):
* HEAD is a merge commit whose FIRST parent is the live base tip and whose
* second parent is the PR head. The PR was opened against `base-old`, and the
* base moved (`base-tip`) afterwards — exactly the state where
* `github.event.pull_request.base.sha` is stale.
*/
function initPrMergeRepo() {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), "omni-newcode-git-"));
const g = (...args: string[]) =>
execFileSync("git", args, { cwd: dir, encoding: "utf8" }).toString().trim();
g("init", "--quiet", "--initial-branch=main");
g("config", "user.email", "test@example.com");
g("config", "user.name", "Test");
fs.mkdirSync(path.join(dir, "src"));
fs.writeFileSync(path.join(dir, "src/base-old.txt"), "old\n");
g("add", "-A");
g("commit", "--quiet", "-m", "base-old");
const baseOld = g("rev-parse", "HEAD");
fs.writeFileSync(path.join(dir, "src/base-tip.txt"), "new\n");
g("add", "-A");
g("commit", "--quiet", "-m", "base-tip");
const baseTip = g("rev-parse", "HEAD");
g("checkout", "--quiet", "-b", "pr", baseOld);
fs.writeFileSync(path.join(dir, "src/pr.txt"), "pr\n");
g("add", "-A");
g("commit", "--quiet", "-m", "pr");
g("checkout", "--quiet", "-b", "merge-main", baseTip);
g("merge", "--no-ff", "--no-edit", "--quiet", "pr");
return { dir, baseOld, baseTip, g };
}

function withPrMergeRepo(fn: (repo: ReturnType<typeof initPrMergeRepo>) => void) {
const repo = initPrMergeRepo();
try {
fn(repo);
} finally {
fs.rmSync(repo.dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
}
}

test("resolveMergeBase compares against the merge ref's first parent when the PR base moved (#15346)", () => {
withPrMergeRepo((repo) => {
// HEAD is GitHub's merge commit; baseOld is the stale base.sha. The PR's new
// code is HEAD^1...HEAD — the base's own newer commit must not be blamed on it.
const mergeBase = resolveMergeBase(repo.baseOld, { cwd: repo.dir });
assert.equal(mergeBase, repo.baseTip);
const changed = repo
.g("diff", "--name-only", "--diff-filter=ACMR", `${mergeBase}...HEAD`)
.split("\n")
.filter(Boolean)
.sort();
assert.deepEqual(changed, ["src/pr.txt"]);
});
});

test("resolveMergeBase keeps merge-base semantics for a linear (non-merge) HEAD", () => {
withPrMergeRepo((repo) => {
repo.g("checkout", "--quiet", "pr"); // single-parent HEAD, like a local dev branch
assert.equal(resolveMergeBase(repo.baseOld, { cwd: repo.dir }), repo.baseOld);
});
});

test("resolveMergeBase is unchanged when the PR base sha is already the live base tip", () => {
withPrMergeRepo((repo) => {
assert.equal(resolveMergeBase(repo.baseTip, { cwd: repo.dir }), repo.baseTip);
});
});

test("baseRefArg reads --base-ref <sha> and ignores a dangling flag", () => {
assert.equal(baseRefArg(["node", "x", "--base-ref", "abc123"]), "abc123");
assert.equal(baseRefArg(["node", "x", "--base-ref"]), null);
Expand Down Expand Up @@ -134,7 +205,10 @@ test("newDeadSymbols reports only symbols that are new on HEAD, in touched files
test("perFileRuleCounts normalizes symlinked cwd so canonical paths match (#14744)", () => {
const realBase = fs.realpathSync(os.tmpdir());
const realDir = fs.mkdtempSync(path.join(realBase, "omni-test-real-"));
const symDir = path.join(realBase, `omni-test-symlink-${Date.now()}-${Math.random().toString(36).slice(2)}`);
const symDir = path.join(
realBase,
`omni-test-symlink-${Date.now()}-${Math.random().toString(36).slice(2)}`
);
fs.symlinkSync(realDir, symDir, "dir");

const report = [
Expand All @@ -149,7 +223,11 @@ test("perFileRuleCounts normalizes symlinked cwd so canonical paths match (#1474
];

try {
const counts = perFileRuleCounts(report, new Set(["complexity", "max-lines-per-function"]), symDir);
const counts = perFileRuleCounts(
report,
new Set(["complexity", "max-lines-per-function"]),
symDir
);
assert.equal(counts.size, 2);
assert.equal(counts.get("open-sse/executors/commandCode.ts"), 2);
assert.equal(counts.get("src/relative.ts"), 1);
Expand Down
73 changes: 73 additions & 0 deletions tests/unit/run-next-listen-retry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import test from "node:test";
import assert from "node:assert/strict";
import http from "node:http";
import net, { AddressInfo } from "node:net";

import { listenWithRetry } from "../../scripts/dev/listen-with-retry.mjs";

// Reproduces the integration port race (base-red #15306, job 111855214488):
// harnesses pick the server port with bind(0)-release at module load, but
// run-next.mjs only binds it after ~15-18s of next prepare(). A transient
// occupant in that window used to kill the boot with EADDRINUSE. The unit test
// pins the contract: retry while the port is busy, reject with the original
// error once attempts are exhausted, resolve with the bound address otherwise.

function listen(server: net.Server | http.Server, port: number, host = "127.0.0.1") {
return new Promise<AddressInfo>((resolve, reject) => {
server.once("error", reject);
server.listen(port, host, () => {
const addr = server.address();
if (!addr || typeof addr === "string") reject(new Error("no TCP address"));
else resolve(addr);
});
});
}

function close(server: net.Server | http.Server) {
return new Promise<void>((resolve) => server.close(() => resolve()));
}

const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms));

test("listenWithRetry binds once the port frees up mid-retry", async () => {
const squatter = net.createServer();
const { port } = await listen(squatter, 0);
const server = http.createServer();
const pending = listenWithRetry(server, { port, host: "127.0.0.1", attempts: 5, delayMs: 50 });
await sleep(120); // first attempt fails against the squatter
await close(squatter); // occupant releases the port mid-retry
const addr = await pending;
assert.equal(addr.port, port);
await close(server);
});

test("listenWithRetry rejects with EADDRINUSE after exhausting its attempts", async () => {
const squatter = net.createServer();
const { port } = await listen(squatter, 0);
try {
const server = http.createServer();
await assert.rejects(
listenWithRetry(server, { port, host: "127.0.0.1", attempts: 2, delayMs: 20 }),
{ code: "EADDRINUSE" }
);
await close(server);
} finally {
await close(squatter);
}
});

test("listenWithRetry resolves with the bound address on a free port", async () => {
const server = http.createServer();
try {
const addr = await listenWithRetry(server, {
port: 0,
host: "127.0.0.1",
attempts: 2,
delayMs: 10,
});
assert.equal(typeof addr.port, "number");
assert.ok(addr.port > 0);
} finally {
await close(server);
}
});
Loading