From 6f45aa4cf283225b8323d0c19f8f95a580e3ac61 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 29 Sep 2026 06:55:06 -0700 Subject: [PATCH] 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. --- scripts/run-oxlint-shards.mts | 22 +++++-- scripts/run-oxlint.mts | 50 +++++++++----- test/scripts/oxlint-config-lifetime.test.ts | 73 +++++++++++++++++++++ test/scripts/oxlint-report-memory.test.ts | 7 +- test/scripts/run-oxlint.test.ts | 2 +- 5 files changed, 131 insertions(+), 23 deletions(-) create mode 100644 test/scripts/oxlint-config-lifetime.test.ts diff --git a/scripts/run-oxlint-shards.mts b/scripts/run-oxlint-shards.mts index a39a3cf7cbc8..ed161eb2b513 100644 --- a/scripts/run-oxlint-shards.mts +++ b/scripts/run-oxlint-shards.mts @@ -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", diff --git a/scripts/run-oxlint.mts b/scripts/run-oxlint.mts index 750051906962..28c1c2d19287 100644 --- a/scripts/run-oxlint.mts +++ b/scripts/run-oxlint.mts @@ -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 { 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)}`); diff --git a/test/scripts/oxlint-config-lifetime.test.ts b/test/scripts/oxlint-config-lifetime.test.ts new file mode 100644 index 000000000000..c353947b5e5f --- /dev/null +++ b/test/scripts/oxlint-config-lifetime.test.ts @@ -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()), + 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(); + } +}); diff --git a/test/scripts/oxlint-report-memory.test.ts b/test/scripts/oxlint-report-memory.test.ts index 40dc8d75ede3..bca619d91569 100644 --- a/test/scripts/oxlint-report-memory.test.ts +++ b/test/scripts/oxlint-report-memory.test.ts @@ -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(); diff --git a/test/scripts/run-oxlint.test.ts b/test/scripts/run-oxlint.test.ts index 5800217f85f7..92f73bd48850 100644 --- a/test/scripts/run-oxlint.test.ts +++ b/test/scripts/run-oxlint.test.ts @@ -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"), "{}");