mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 09:39:25 +00:00
fix(lint): coordinate temporary configs with build ownership
Preserve Oxlint relative configuration semantics while holding the existing artifact owner across derived config creation, lint execution, and cleanup. Share that owner across a lint batch so its child shards remain parallel and a concurrent compiler cannot capture transient JSON metadata. Keep all compiler namespace and source-identity checks intact.
This commit is contained in:
parent
0c6971d5e0
commit
6f45aa4cf2
5 changed files with 131 additions and 23 deletions
|
|
@ -60,7 +60,11 @@ type RunnerOptions = {
|
|||
extraArgs: string[];
|
||||
runner: string;
|
||||
};
|
||||
type ShardRunnerOptions = RunnerOptions & { shard: OxlintShard; onCompleted?: () => void };
|
||||
type ShardRunnerOptions = RunnerOptions & {
|
||||
shard: OxlintShard;
|
||||
onCompleted?: () => void;
|
||||
ownsArtifacts?: boolean;
|
||||
};
|
||||
type ShardBatchOptions = RunnerOptions & {
|
||||
concurrency: number;
|
||||
entries: OxlintShard[];
|
||||
|
|
@ -385,7 +389,9 @@ export async function main(
|
|||
completed = results.completed;
|
||||
return results.statuses.find((status) => status !== 0) ?? 0;
|
||||
};
|
||||
const status = needsArtifacts ? await withDistArtifactOwnership(process.cwd(), run) : await run();
|
||||
// One batch owner lets lint children overlap without exposing their transient
|
||||
// configuration files to a concurrent compiler's input snapshot.
|
||||
const status = await withDistArtifactOwnership(process.cwd(), run);
|
||||
if (evidenceId && completed === selectedShards.length && !isParentTerminationRequested()) {
|
||||
console.log(
|
||||
`[ci-static:oxlint:completion] ${JSON.stringify({
|
||||
|
|
@ -673,6 +679,7 @@ async function runShards({
|
|||
extraArgs,
|
||||
runner,
|
||||
shard,
|
||||
ownsArtifacts: true,
|
||||
onCompleted: () => {
|
||||
completed++;
|
||||
},
|
||||
|
|
@ -683,7 +690,14 @@ async function runShards({
|
|||
return { statuses: results.filter((status) => status !== undefined), completed };
|
||||
}
|
||||
|
||||
export async function runShard({ env, extraArgs, runner, shard, onCompleted }: ShardRunnerOptions) {
|
||||
export async function runShard({
|
||||
env,
|
||||
extraArgs,
|
||||
runner,
|
||||
shard,
|
||||
onCompleted,
|
||||
ownsArtifacts,
|
||||
}: ShardRunnerOptions) {
|
||||
console.error(`[oxlint:${shard.name}] starting`);
|
||||
const startedAt = Date.now();
|
||||
const heartbeatMs = resolveShardHeartbeatMs(env);
|
||||
|
|
@ -693,7 +707,7 @@ export async function runShard({ env, extraArgs, runner, shard, onCompleted }: S
|
|||
// errors to the private artifact entry without catching or reporting them here.
|
||||
const args =
|
||||
runner === path.resolve("scripts", "run-oxlint.mts")
|
||||
? shouldPrepareExtensionPackageBoundaryArtifactsForShards([shard], extraArgs)
|
||||
? ownsArtifacts || shouldPrepareExtensionPackageBoundaryArtifactsForShards([shard], extraArgs)
|
||||
? distArtifactEntryArgs(runner, [...shard.args, ...extraArgs])
|
||||
: [
|
||||
"--import",
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import { parseStaticDiagnostics } from "./lib/ci-static-check-evidence.mjs";
|
|||
import { isDirectRunUrl } from "./lib/direct-run.mjs";
|
||||
import {
|
||||
distArtifactEntryArgs,
|
||||
resolveDistArtifactLockPath,
|
||||
withDistArtifactOwnership,
|
||||
} from "./lib/dist-artifact-ownership.mts";
|
||||
import {
|
||||
|
|
@ -148,6 +149,7 @@ async function runWithAdvisoryLimits(
|
|||
bin: string,
|
||||
args: string[],
|
||||
env: NodeJS.ProcessEnv,
|
||||
ownedDirectory?: string,
|
||||
): Promise<OxlintRunResult> {
|
||||
const configOption = oxlintOption(args, "--config", "-c");
|
||||
const configPath = path.resolve(configOption.value ?? ".oxlintrc.json");
|
||||
|
|
@ -215,6 +217,18 @@ async function runWithAdvisoryLimits(
|
|||
return { status: await runManagedCommand(command) };
|
||||
}
|
||||
|
||||
if (enabled) {
|
||||
const configRoot = fs.realpathSync(path.dirname(configPath));
|
||||
const directory = resolveDistArtifactLockPath(configRoot);
|
||||
if (ownedDirectory !== directory) {
|
||||
// Oxlint anchors inherited globs at this directory. Keep its transient
|
||||
// config here, but exclude compilers from the entire create/remove lifetime.
|
||||
return await withDistArtifactOwnership(configRoot, () =>
|
||||
runWithAdvisoryLimits(bin, args, env, directory),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// CLI --warn cannot replace scoped severities and enables rules outside their file scopes.
|
||||
// Keep the transient config beside its owner so relative globs and plugin paths do not move.
|
||||
const advisoryConfig = enabled
|
||||
|
|
@ -645,26 +659,28 @@ export async function runOxlint(
|
|||
return { status: 0 };
|
||||
}
|
||||
|
||||
if (needsArtifactPreparation) {
|
||||
// Declaration compilation owns its Go policy; lint limits belong to the oxlint child.
|
||||
await prepareExtensionPackageBoundaryArtifacts(localEnv);
|
||||
}
|
||||
return await runWithAdvisoryLimits(
|
||||
oxlintPath,
|
||||
finalArgs,
|
||||
resolveOxlintToolchainEnv(oxlintPath, env),
|
||||
);
|
||||
const run = async (ownedDirectory?: string) => {
|
||||
if (needsArtifactPreparation) {
|
||||
// Declaration compilation owns its Go policy; lint limits belong to the oxlint child.
|
||||
await prepareExtensionPackageBoundaryArtifacts(localEnv);
|
||||
}
|
||||
return await runWithAdvisoryLimits(
|
||||
oxlintPath,
|
||||
finalArgs,
|
||||
resolveOxlintToolchainEnv(oxlintPath, env),
|
||||
ownedDirectory,
|
||||
);
|
||||
};
|
||||
// Skip-prepare callers still consume shared declarations. Hold one owner across
|
||||
// preparation and lint; source-only lint acquires it only for transient config.
|
||||
const root = process.cwd();
|
||||
return !focusedConfig && shouldPrepareExtensionPackageBoundaryArtifacts(argv)
|
||||
? await withDistArtifactOwnership(root, () => run(resolveDistArtifactLockPath(root)))
|
||||
: await run();
|
||||
}
|
||||
|
||||
if (isDirectRunUrl(process.argv[1], import.meta.url)) {
|
||||
const argv = process.argv.slice(2);
|
||||
// Skip-prepare callers still consume shared declarations. Source-only lint
|
||||
// remains independent; sharded lint inherits its parent's owner.
|
||||
const result =
|
||||
!argv.includes(OPENCLAW_FOCUSED_CONFIG_FLAG) &&
|
||||
shouldPrepareExtensionPackageBoundaryArtifacts(argv)
|
||||
? await withDistArtifactOwnership(process.cwd(), () => runOxlint(argv))
|
||||
: await runOxlint(argv);
|
||||
const result = await runOxlint();
|
||||
process.exitCode = result.status;
|
||||
if (result.evidence) {
|
||||
console.log(`\n[ci-static:oxlint:leaf] ${JSON.stringify(result.evidence)}`);
|
||||
|
|
|
|||
73
test/scripts/oxlint-config-lifetime.test.ts
Normal file
73
test/scripts/oxlint-config-lifetime.test.ts
Normal file
|
|
@ -0,0 +1,73 @@
|
|||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
import { CompilerInputSnapshot } from "../../scripts/lib/compiler-input-snapshot.mts";
|
||||
import { acquireDistArtifactOwnership } from "../../scripts/lib/dist-artifact-lock.mts";
|
||||
import { runManagedCommand } from "../../scripts/lib/managed-child-process.mts";
|
||||
import { runOxlint } from "../../scripts/run-oxlint.mts";
|
||||
import { createScriptTestHarness } from "./test-helpers.js";
|
||||
|
||||
vi.mock("../../scripts/lib/managed-child-process.mts", async (original) => ({
|
||||
...(await original<typeof import("../../scripts/lib/managed-child-process.mts")>()),
|
||||
runManagedCommand: vi.fn(async () => 0),
|
||||
}));
|
||||
afterEach(() => vi.clearAllMocks());
|
||||
const { createTempDir } = createScriptTestHarness();
|
||||
|
||||
it("keeps concurrent compiler input identity stable when lint retires its config", async () => {
|
||||
const root = fs.realpathSync(createTempDir("oxlint-config-lifetime-"));
|
||||
const config = path.join(root, ".oxlintrc.json");
|
||||
fs.writeFileSync(config, JSON.stringify({ rules: { "max-lines": "error" } }));
|
||||
fs.writeFileSync(path.join(root, "source.ts"), "export const value = 1;\n");
|
||||
fs.writeFileSync(
|
||||
path.join(root, "tsconfig.json"),
|
||||
JSON.stringify({ compilerOptions: { types: [] }, files: ["source.ts"] }),
|
||||
);
|
||||
const snapshot = () =>
|
||||
new CompilerInputSnapshot(root, { toolchainFiles: [], generatorInputs: [] });
|
||||
const lintFailure = new Error("controlled lint child failure");
|
||||
let before: CompilerInputSnapshot | undefined;
|
||||
let transient: string | undefined;
|
||||
let compilerBlocked = false;
|
||||
vi.mocked(runManagedCommand).mockImplementationOnce(async (options) => {
|
||||
const index = options.args?.indexOf("--config") ?? -1;
|
||||
transient = options.args?.[index + 1];
|
||||
expect(transient).not.toBe(config);
|
||||
expect(transient && fs.existsSync(transient)).toBe(true);
|
||||
const compiler = await acquireDistArtifactOwnership(root).catch((error: unknown) => {
|
||||
expect(String(error)).toContain("Could not acquire");
|
||||
compilerBlocked = true;
|
||||
});
|
||||
if (compiler) {
|
||||
try {
|
||||
before = snapshot();
|
||||
before.signature("tsconfig.json", [], ["source.ts"]);
|
||||
} finally {
|
||||
await compiler.release();
|
||||
}
|
||||
}
|
||||
throw lintFailure;
|
||||
});
|
||||
await expect(
|
||||
runOxlint(["--openclaw-focused-config", "--config", config, "source.ts"], {
|
||||
...process.env,
|
||||
GITHUB_ACTIONS: "true",
|
||||
OPENCLAW_CI_STATIC_EVIDENCE: "0",
|
||||
}),
|
||||
).rejects.toBe(lintFailure);
|
||||
expect(transient && fs.existsSync(transient)).toBe(false);
|
||||
const compiler = await acquireDistArtifactOwnership(root);
|
||||
try {
|
||||
if (!before) {
|
||||
before = snapshot();
|
||||
before.signature("tsconfig.json", [], ["source.ts"]);
|
||||
}
|
||||
const captured = before;
|
||||
expect(() =>
|
||||
snapshot().seal("tsconfig.json", [], ["source.ts"], captured, Date.now()),
|
||||
).not.toThrow();
|
||||
expect(compilerBlocked).toBe(true);
|
||||
} finally {
|
||||
await compiler.release();
|
||||
}
|
||||
});
|
||||
|
|
@ -4,6 +4,7 @@ import fs from "node:fs";
|
|||
import path from "node:path";
|
||||
import { PassThrough } from "node:stream";
|
||||
import { afterEach, expect, it, vi } from "vitest";
|
||||
import { resolveDistArtifactLockPath } from "../../scripts/lib/dist-artifact-ownership.mts";
|
||||
import { runManagedCommand } from "../../scripts/lib/managed-child-process.mts";
|
||||
import { runOxlint } from "../../scripts/run-oxlint.mts";
|
||||
import { createScriptTestHarness } from "./test-helpers.js";
|
||||
|
|
@ -78,7 +79,11 @@ it.for([false, true].flatMap((evidence) => [0, 1].map((status) => ({ evidence, s
|
|||
expect(fs.readFileSync(summary, "utf8")).toContain(
|
||||
"Individual advisory annotations and static evidence were skipped",
|
||||
);
|
||||
expect(fs.readdirSync(root)).toEqual(["config.json", "summary.md"]);
|
||||
expect(fs.readdirSync(root).filter((name) => name !== ".artifacts")).toEqual([
|
||||
"config.json",
|
||||
"summary.md",
|
||||
]);
|
||||
expect(fs.existsSync(path.join(resolveDistArtifactLockPath(root), "owner.json"))).toBe(false);
|
||||
} finally {
|
||||
writer.mockRestore();
|
||||
warnings.mockRestore();
|
||||
|
|
|
|||
|
|
@ -962,7 +962,7 @@ describe("run-oxlint", () => {
|
|||
join(cwd, ".oxlintrc.json"),
|
||||
JSON.stringify({
|
||||
categories: { correctness: "off" },
|
||||
rules: { "no-var": "error" },
|
||||
rules: { "no-var": "error", "max-lines": ["error", { max: 10 }] },
|
||||
}),
|
||||
);
|
||||
writeFileSync(join(cwd, "config/tsconfig/oxlint.core.json"), "{}");
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue