mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-06 05:51:15 +08:00
fix(ci): scope PR merge diff checks to first parent (#90287)
Summary: - This PR adds opt-in first-parent merge-head diff-base handling for CI changed-scope, changed-lanes, and OpenGrep PR scans, plus synthetic merge coverage and small lint/type cleanups. - PR surface: Source +6, Tests +204, Config +1, Other +179. Total +390 across 15 files. - Reproducibility: yes. The synthetic merge tests and PR body live-ref proof show the stale payload-base path can include main-only files, and first-parent mode narrows it to PR-owned paths. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(ci): update workflow guard expectations - PR branch already contained follow-up commit before automerge: fix(ci): resolve plugin guardrail lint failures - PR branch already contained follow-up commit before automerge: fix(ci): preserve plugin run context typing - PR branch already contained follow-up commit before automerge: fix(ci): scope PR merge diff checks to first parent Validation: - ClawSweeper review passed for head40235e8c3d. - Required merge gates passed before the squash merge. Prepared head SHA:40235e8c3dReview: https://github.com/openclaw/openclaw/pull/90287#issuecomment-4621155576 Co-authored-by: Mason Huang <masonxhuang@tencent.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: hxy91819 Co-authored-by: hxy91819 <8814856+hxy91819@users.noreply.github.com>
This commit is contained in:
@@ -71,6 +71,71 @@ function parseChangedLaneOutput(output: string): {
|
||||
};
|
||||
}
|
||||
|
||||
function writeRepoFile(repoDir: string, filePath: string, contents: string): void {
|
||||
const absolutePath = path.join(repoDir, filePath);
|
||||
mkdirSync(path.dirname(absolutePath), { recursive: true });
|
||||
writeFileSync(absolutePath, contents, "utf8");
|
||||
}
|
||||
|
||||
function createSyntheticMergeRepo(prefix: string): { dir: string; staleBase: string } {
|
||||
const dir = makeTempRepoRoot(tempDirs, prefix);
|
||||
git(dir, ["init", "-q", "--initial-branch=main"]);
|
||||
writeRepoFile(dir, "README.md", "base\n");
|
||||
git(dir, ["add", "."]);
|
||||
git(dir, [
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"-c",
|
||||
"user.name=Test User",
|
||||
"commit",
|
||||
"-q",
|
||||
"-m",
|
||||
"base",
|
||||
]);
|
||||
const staleBase = git(dir, ["rev-parse", "HEAD"]);
|
||||
|
||||
git(dir, ["switch", "-q", "-c", "feature"]);
|
||||
writeRepoFile(dir, "src/pr.ts", "export const pr = true;\n");
|
||||
git(dir, ["add", "."]);
|
||||
git(dir, [
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"-c",
|
||||
"user.name=Test User",
|
||||
"commit",
|
||||
"-q",
|
||||
"-m",
|
||||
"feature",
|
||||
]);
|
||||
|
||||
git(dir, ["switch", "-q", "main"]);
|
||||
writeRepoFile(dir, "src/main-only.ts", "export const mainOnly = true;\n");
|
||||
git(dir, ["add", "."]);
|
||||
git(dir, [
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"-c",
|
||||
"user.name=Test User",
|
||||
"commit",
|
||||
"-q",
|
||||
"-m",
|
||||
"main only",
|
||||
]);
|
||||
git(dir, [
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"-c",
|
||||
"user.name=Test User",
|
||||
"merge",
|
||||
"--no-ff",
|
||||
"feature",
|
||||
"-m",
|
||||
"synthetic merge",
|
||||
]);
|
||||
|
||||
return { dir, staleBase };
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
cleanupCorepackPnpmShimDir();
|
||||
cleanupTempDirs(tempDirs);
|
||||
@@ -251,6 +316,23 @@ describe("scripts/changed-lanes", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("uses the merge commit first parent instead of a stale PR payload base", () => {
|
||||
const { dir, staleBase } = createSyntheticMergeRepo("openclaw-changed-lanes-merge-");
|
||||
|
||||
expect(listChangedPathsFromGit({ base: staleBase, cwd: dir, includeWorktree: false })).toEqual([
|
||||
"src/main-only.ts",
|
||||
"src/pr.ts",
|
||||
]);
|
||||
expect(
|
||||
listChangedPathsFromGit({
|
||||
base: staleBase,
|
||||
cwd: dir,
|
||||
includeWorktree: false,
|
||||
mergeHeadFirstParent: true,
|
||||
}),
|
||||
).toEqual(["src/pr.ts"]);
|
||||
});
|
||||
|
||||
it("ignores local Crabbox metadata in the default local diff", () => {
|
||||
const dir = makeTempRepoRoot(tempDirs, "openclaw-changed-lanes-crabbox-");
|
||||
git(dir, ["init", "-q", "--initial-branch=main"]);
|
||||
|
||||
@@ -59,8 +59,9 @@ describe("ci workflow guards", () => {
|
||||
expect(checkoutStep.run, jobName).toContain("timed out on attempt $attempt; retrying");
|
||||
expect(checkoutStep.run, jobName).not.toContain("if timeout --signal=TERM");
|
||||
expect(checkoutStep.run, jobName).toContain("-c protocol.version=2");
|
||||
const expectedDepth = jobName === "preflight" ? 2 : 1;
|
||||
expect(checkoutStep.run, jobName).toContain(
|
||||
"fetch --no-tags --prune --no-recurse-submodules --depth=1 origin",
|
||||
`fetch --no-tags --prune --no-recurse-submodules --depth=${expectedDepth} origin`,
|
||||
);
|
||||
if (jobName !== "skills-python") {
|
||||
expect(checkoutStep.run, jobName).toContain('if [ "$fetch_status" = "124" ]');
|
||||
|
||||
@@ -214,7 +214,6 @@ describe("production lint suppressions", () => {
|
||||
"src/plugin-sdk/test-helpers/public-surface-loader.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugin-sdk/test-helpers/subagent-hooks.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugins/hooks.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugins/host-hook-runtime.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugins/host-hook-state.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugins/host-hooks.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
"src/plugins/lazy-service-module.ts|typescript/no-unnecessary-type-parameters|1",
|
||||
|
||||
@@ -15,6 +15,17 @@ function writeFile(filePath: string, content: string): void {
|
||||
fs.writeFileSync(filePath, content);
|
||||
}
|
||||
|
||||
function copyRunOpengrepFiles(repo: string): void {
|
||||
const scriptSource = path.resolve("scripts/run-opengrep.sh");
|
||||
const helperSource = path.resolve("scripts/lib/merge-head-diff-base.mjs");
|
||||
writeFile(path.join(repo, "scripts/run-opengrep.sh"), fs.readFileSync(scriptSource, "utf8"));
|
||||
writeFile(
|
||||
path.join(repo, "scripts/lib/merge-head-diff-base.mjs"),
|
||||
fs.readFileSync(helperSource, "utf8"),
|
||||
);
|
||||
fs.chmodSync(path.join(repo, "scripts/run-opengrep.sh"), 0o755);
|
||||
}
|
||||
|
||||
describe("run-opengrep.sh", () => {
|
||||
it("validates the rulepack when only OpenGrep rulepack files changed", () => {
|
||||
const repo = createTempDir("openclaw-run-opengrep-");
|
||||
@@ -22,9 +33,7 @@ describe("run-opengrep.sh", () => {
|
||||
git(repo, "config", "user.email", "test@example.com");
|
||||
git(repo, "config", "user.name", "Test User");
|
||||
|
||||
const scriptSource = path.resolve("scripts/run-opengrep.sh");
|
||||
writeFile(path.join(repo, "scripts/run-opengrep.sh"), fs.readFileSync(scriptSource, "utf8"));
|
||||
fs.chmodSync(path.join(repo, "scripts/run-opengrep.sh"), 0o755);
|
||||
copyRunOpengrepFiles(repo);
|
||||
writeFile(path.join(repo, "security/opengrep/precise.yml"), "rules: []\n");
|
||||
git(repo, "add", ".");
|
||||
git(repo, "commit", "-qm", "initial");
|
||||
@@ -57,4 +66,58 @@ describe("run-opengrep.sh", () => {
|
||||
const args = fs.readFileSync(path.join(repo, "opengrep-args.txt"), "utf8");
|
||||
expect(args).toContain("security/opengrep/precise.yml");
|
||||
});
|
||||
|
||||
it("scans PR files instead of main-only files when the payload base is stale", () => {
|
||||
const repo = createTempDir("openclaw-run-opengrep-merge-");
|
||||
git(repo, "init", "-q", "--initial-branch=main");
|
||||
git(repo, "config", "user.email", "test@example.com");
|
||||
git(repo, "config", "user.name", "Test User");
|
||||
|
||||
copyRunOpengrepFiles(repo);
|
||||
writeFile(path.join(repo, "security/opengrep/precise.yml"), "rules: []\n");
|
||||
writeFile(path.join(repo, "README.md"), "base\n");
|
||||
git(repo, "add", ".");
|
||||
git(repo, "commit", "-qm", "base");
|
||||
const staleBase = git(repo, "rev-parse", "HEAD");
|
||||
|
||||
git(repo, "switch", "-q", "-c", "feature");
|
||||
writeFile(path.join(repo, "src/pr.ts"), "export const pr = true;\n");
|
||||
git(repo, "add", ".");
|
||||
git(repo, "commit", "-qm", "feature");
|
||||
|
||||
git(repo, "switch", "-q", "main");
|
||||
writeFile(path.join(repo, "src/main-only.ts"), "export const mainOnly = true;\n");
|
||||
git(repo, "add", ".");
|
||||
git(repo, "commit", "-qm", "main only");
|
||||
git(repo, "merge", "--no-ff", "feature", "-m", "synthetic merge");
|
||||
|
||||
const argsPath = path.join(repo, "opengrep-args.txt");
|
||||
const binDir = path.join(repo, "bin");
|
||||
fs.mkdirSync(binDir);
|
||||
writeFile(
|
||||
path.join(binDir, "opengrep"),
|
||||
[
|
||||
"#!/usr/bin/env bash",
|
||||
`printf '%s\\n' "$@" > ${JSON.stringify(argsPath)}`,
|
||||
"exit 0",
|
||||
"",
|
||||
].join("\n"),
|
||||
);
|
||||
fs.chmodSync(path.join(binDir, "opengrep"), 0o755);
|
||||
|
||||
execFileSync("bash", ["scripts/run-opengrep.sh", "--changed"], {
|
||||
cwd: repo,
|
||||
env: {
|
||||
...process.env,
|
||||
PATH: `${binDir}${path.delimiter}${process.env.PATH ?? ""}`,
|
||||
OPENCLAW_OPENGREP_BASE_REF: `${staleBase}...HEAD`,
|
||||
OPENCLAW_OPENGREP_MERGE_HEAD_FIRST_PARENT: "1",
|
||||
},
|
||||
encoding: "utf8",
|
||||
});
|
||||
|
||||
const args = fs.readFileSync(argsPath, "utf8");
|
||||
expect(args).toContain("src/pr.ts");
|
||||
expect(args).not.toContain("src/main-only.ts");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user