From b729a3a4130585042dbffc1ed4d8cb6138908271 Mon Sep 17 00:00:00 2001 From: debuggingfuture Date: Wed, 19 Aug 2026 03:24:49 +0800 Subject: [PATCH] fix(oxlint): pin the version the gate enforces, and let a repo pin its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `oxlint` run fetched `npx --yes oxlint@1`, so the rule set it gated on was whichever 1.x the registry served that minute. oxlint selects rules by category and 1.79.0 moved five React rules into `correctness`, turning consumers red on every open PR at once for findings that predated all of them — unfixable from any consumer repo, because the version lives here. `VERSION_DEFAULT` is now an exact version, resolved through dispatch input -> `oxlint.version:` -> `oxlint.version` -> the default, so a repo a bump breaks pins itself out in one `wrangler kv key put` instead of waiting on a deploy. `pr-review`'s grounding block shares the same constant, so the findings quoted to the reviewer are the findings that decide the check. --- runs/README.md | 34 ++++++++++++ runs/oxlint.test.ts | 122 +++++++++++++++++++++++++++++++++++++++++--- runs/oxlint.ts | 95 ++++++++++++++++++++++++++++++---- runs/pr-review.ts | 12 ++++- 4 files changed, 245 insertions(+), 18 deletions(-) diff --git a/runs/README.md b/runs/README.md index f94ae5a..f66ba0c 100644 --- a/runs/README.md +++ b/runs/README.md @@ -188,6 +188,40 @@ command needs `node_modules` / a lockfile install. } ``` +## `oxlint` — install-free Oxc gate, on a version pinned in the run + +Clone → `npx --yes oxlint@` → upload log → green/red +`flare-dispatch/oxlint`. No install, no per-repo command, no `.oxlintrc.json` +required, so it is droppable on any repo. + +The version is an **exact pin** in [`oxlint.ts`](./oxlint.ts) (`VERSION_DEFAULT`), +not a range and not a dist-tag. It was the major line `1` until 2026-08-18, when +oxlint 1.79.0 moved five React rules into the `correctness` category: because +oxlint selects rules by category, every consumer tracking `@1` went red within +the hour, on every open PR at once, for findings that predated all of them — +and no consumer could fix it, because the version lives here. The same release +left each repo's own `pnpm lint` green, since that runs the repo's pinned +devDependency; a floating gate means the two lanes enforce different rule sets +and nothing says so until they disagree. + +Bumping `VERSION_DEFAULT` can turn consumers red, so it belongs in its own PR. + +### Pinning one repo (`oxlint.version:`) + +A repo that a bump breaks — or one that wants a newer oxlint before the default +moves — sets its own version without waiting on a deploy: + +```bash +wrangler kv key put --binding=CONFIG_KV \ + "oxlint.version:owner/repo" "1.74.0" +``` + +Resolution is dispatch input → `oxlint.version:` → the dispatcher-wide +`oxlint.version` → `VERSION_DEFAULT`. An Action-mode dispatch that passes +`version` skips the lookup entirely (and the `resolve-version` step with it). +Set an exact version: a range here re-resolves on every run and reinstates the +same problem one repo at a time. + ## `offload-test` — webhook mode needs two CONFIG_KV keys to run a real suite `offload-test`'s `pull_request` trigger can only pass what it computes from the diff --git a/runs/oxlint.test.ts b/runs/oxlint.test.ts index 56750ea..1a616b5 100644 --- a/runs/oxlint.test.ts +++ b/runs/oxlint.test.ts @@ -9,7 +9,10 @@ // Effect *fails* with `AcceptanceFailed` carrying the exit // (c) advisory — `failOnNonZeroExit: false` → exit 1 is a successful Effect // (d) command — `version` + `args` compose the `npx oxlint@ ` line -// (e) webhook trig — the pull_request payload maps to inputs; gate skips +// (e) version — resolution ladder: dispatch → per-repo CONFIG_KV → +// dispatcher-wide CONFIG_KV → the pinned default, and the +// default is an EXACT version, never a range +// (f) webhook trig — the pull_request payload maps to inputs; gate skips // drafts/dependabot // // Spec: specs/03-dsl.md § Unit-testing runs. @@ -20,19 +23,32 @@ import { it } from "@effect/vitest"; import { Cause, Effect, Exit, Option } from "effect"; import { describe, expect } from "vitest"; import { makeCFRuntimeTest } from "@fractalboxdev/flare-dispatch-core/testing"; -import { oxlint } from "./oxlint"; +import { oxlint, VERSION_DEFAULT } from "./oxlint"; -/** Default decoded input — version "1", empty args, fail-on-nonzero ON. */ +/** + * Default decoded input — an EXPLICIT version, empty args, fail-on-nonzero ON. + * Carrying the version keeps these cases on the three-step Action-mode shape; + * the resolution ladder that webhook mode walks has its own block below. + */ const baseInput = { repo: "owner/name", sha: "abc123", args: "", - version: "1", + version: "1.74.0", failOnNonZeroExit: true, } as const; /** The command the default input produces. */ -const CMD_DEFAULT = "npx --yes oxlint@1"; +const CMD_DEFAULT = "npx --yes oxlint@1.74.0"; + +/** The command a version-less dispatch produces with nothing in CONFIG_KV. */ +const CMD_PINNED = `npx --yes oxlint@${VERSION_DEFAULT}`; + +/** + * `ensureWorkspace`'s checkout probe, which precedes the oxlint command in + * every run — the version cases assert on the command that follows it. + */ +const PROBE = "test -d /workspace/name/.git"; describe("oxlint", () => { it.effect("green path — oxlint exits 0, no install, three steps", () => { @@ -210,6 +226,98 @@ describe("oxlint", () => { }).pipe(Effect.provide(layer)); }); + // --- Version resolution ---------------------------------------------------- + // + // The gate's rule set is decided by which oxlint it fetches, so the version + // must never be a moving target: oxlint selects rules by CATEGORY, and a + // minor release that moves a rule into `correctness` turns every consumer on + // a floating range red at once, for findings that predated all of them. + + it("the default is an exact version — never a range or a dist-tag", () => { + expect(VERSION_DEFAULT).toMatch(/^\d+\.\d+\.\d+$/); + }); + + it.effect("no version anywhere — the run uses the pinned default", () => { + const { layer, handles } = makeCFRuntimeTest({ + sandboxProgram: { [CMD_PINNED]: { exitCode: 0 } }, + }); + const { version: _omitted, ...input } = baseInput; + + return Effect.gen(function* () { + const result = yield* oxlint.run(input); + expect(result.exitCode).toBe(0); + expect(handles.sandbox.execs.map((e) => e.command)).toEqual([PROBE, CMD_PINNED]); + // Resolution is its own checkpointed step in webhook mode. + expect(handles.executions.steps.map((s) => s.name)).toEqual([ + "resolve-version", + "checkout", + "exec", + "upload-log", + ]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("per-repo CONFIG_KV pins the version, beating the dispatcher-wide key", () => { + const command = "npx --yes oxlint@1.74.0"; + const { layer, handles } = makeCFRuntimeTest({ + sandboxProgram: { [command]: { exitCode: 0 } }, + config: { + "oxlint.version:owner/name": "1.74.0", + "oxlint.version": "1.78.0", + }, + }); + const { version: _omitted, ...input } = baseInput; + + return Effect.gen(function* () { + yield* oxlint.run(input); + expect(handles.sandbox.execs.map((e) => e.command)).toEqual([PROBE, command]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("dispatcher-wide CONFIG_KV applies when no per-repo key is set", () => { + const command = "npx --yes oxlint@1.78.0"; + const { layer, handles } = makeCFRuntimeTest({ + sandboxProgram: { [command]: { exitCode: 0 } }, + config: { "oxlint.version": "1.78.0" }, + }); + const { version: _omitted, ...input } = baseInput; + + return Effect.gen(function* () { + yield* oxlint.run(input); + expect(handles.sandbox.execs.map((e) => e.command)).toEqual([PROBE, command]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("a dispatched version wins over both keys, and skips the resolve step", () => { + const { layer, handles } = makeCFRuntimeTest({ + sandboxProgram: { [CMD_DEFAULT]: { exitCode: 0 } }, + config: { "oxlint.version:owner/name": "1.78.0", "oxlint.version": "1.77.0" }, + }); + + return Effect.gen(function* () { + yield* oxlint.run(baseInput); + expect(handles.sandbox.execs.map((e) => e.command)).toEqual([PROBE, CMD_DEFAULT]); + expect(handles.executions.steps.map((s) => s.name)).toEqual([ + "checkout", + "exec", + "upload-log", + ]); + }).pipe(Effect.provide(layer)); + }); + + it.effect("a blank CONFIG_KV value falls through rather than producing `oxlint@`", () => { + const { layer, handles } = makeCFRuntimeTest({ + sandboxProgram: { [CMD_PINNED]: { exitCode: 0 } }, + config: { "oxlint.version:owner/name": " ", "oxlint.version": "" }, + }); + const { version: _omitted, ...input } = baseInput; + + return Effect.gen(function* () { + yield* oxlint.run(input); + expect(handles.sandbox.execs.map((e) => e.command)).toEqual([PROBE, CMD_PINNED]); + }).pipe(Effect.provide(layer)); + }); + // --- Webhook trigger ------------------------------------------------------- const prPayload = (overrides: Record = {}): Record => ({ @@ -233,9 +341,11 @@ describe("oxlint", () => { repo: "owner/name", sha: "abcdef0123456789cafe", args: "", - version: "1", failOnNonZeroExit: true, }); + // The version is deliberately ABSENT — pinning it here would pin every + // repo the dispatcher serves, unreachable from any of them. + expect(trigger?.inputs(ctx)).not.toHaveProperty("version"); expect(trigger?.idempotencyKey(ctx)).toBe("oxlint:owner_name:abcdef012345"); }); diff --git a/runs/oxlint.ts b/runs/oxlint.ts index aa95c86..7bc2433 100644 --- a/runs/oxlint.ts +++ b/runs/oxlint.ts @@ -14,9 +14,10 @@ // // Contract mirrors `offload-test` (specs/02-runs.md § 1, specs/03-dsl.md // § Top-level shape) — the only differences are the baked-in command (oxlint, -// resolved from the catalog via `npx`, no per-repo CONFIG_KV lookup) and that -// `failOnNonZeroExit` defaults ON: a lint finding has no other pass/fail -// signal, so a non-zero exit must turn the check red in every mode. +// resolved from the catalog via `npx`, with one optional per-repo CONFIG_KV +// key, `oxlint.version:`) and that `failOnNonZeroExit` defaults ON: a +// lint finding has no other pass/fail signal, so a non-zero exit must turn the +// check red in every mode. // // Because it installs nothing and configures nothing, the gate is droppable on // ANY repo — including one that never adopted oxlint (no `.oxlintrc.json`, no @@ -34,6 +35,7 @@ import { Effect, Schema } from "effect"; import { AcceptanceFailed, artifact, + config, defineRun, sandbox, step, @@ -55,10 +57,16 @@ const OxlintInput = Schema.Struct({ */ args: Schema.optionalWith(Schema.String, { default: () => "" }), /** - * npm dist-tag / version of oxlint to fetch via `npx`. Defaults to the major - * line `"1"` so a consumer tracks oxlint 1.x without redeploying this run. + * Exact oxlint version to fetch via `npx`. OPTIONAL: a webhook dispatch omits + * it and the run body resolves it from CONFIG_KV, falling back to + * `VERSION_DEFAULT` — see `repoVersionKey`. + * + * A range or dist-tag (`"1"`, `"^1.74.0"`, `"latest"`) is accepted and + * defeats the point: `npx` re-resolves it on every run, so what this gate + * enforces becomes whatever the registry served that minute. Pass an exact + * version. See `VERSION_DEFAULT` for what that costs when it is not. */ - version: Schema.optionalWith(Schema.String, { default: () => "1" }), + version: Schema.optional(Schema.String), /** * Fail the run Effect (→ red `flare-dispatch/oxlint` check) on a non-zero * exit. Defaults ON — unlike `offload-test`, a lint run has no GHA step that @@ -95,6 +103,45 @@ const OxlintOutput = Schema.Struct({ /** Default `exec` timeout — lint is fast, so a tighter ceiling than tests. */ const TIMEOUT_SEC_DEFAULT = 300; +/** + * The oxlint version this gate runs when nothing overrides it — an EXACT + * version, never a range or a dist-tag. + * + * It was the major line `"1"` until 2026-08-18, which handed the registry the + * power to change what this gate enforces with no commit, no PR and no review + * on either side. oxlint selects rules by CATEGORY, and a minor release may + * move a rule into `correctness`: 1.79.0 moved five React rules in, and every + * consumer tracking `@1` went red within the hour — on every open PR at once, + * for findings that predated all of them. Nothing in those PRs caused it, and + * nothing in those repos could fix it, because the version lived here. + * + * A consumer's own lint step runs its PINNED devDependency, so a floating gate + * also means the two lanes enforce different rule sets, with no signal of the + * divergence until the day they disagree. + * + * Bumping this is a deliberate, reviewed change here — and a change that can + * turn consumers red, so it belongs in its own PR. A consumer needing a + * different version pins it per-repo (`repoVersionKey`) without waiting on a + * release. + */ +export const VERSION_DEFAULT = "1.79.0"; + +/** + * CONFIG_KV keys the run body resolves `version` from when a dispatch omits it + * (webhook mode — the `pull_request` trigger's `inputs` is a sync, + * payload-only callback that cannot read config). Per-repo wins over the + * dispatcher-wide key, which wins over `VERSION_DEFAULT`: + * + * wrangler kv key put --binding=CONFIG_KV "oxlint.version:owner/repo" "1.74.0" + * + * This is the whole of oxlint's per-repo config. The run still needs no + * command, no install and no `.oxlintrc.json`, so it stays droppable on any + * repo; the key exists so a consumer that a bump breaks can pin itself out of + * it in one command instead of waiting on a deploy here. + */ +const VERSION_KEY = "oxlint.version"; +const repoVersionKey = (repo: string): string => `${VERSION_KEY}:${repo}`; + /** * Platform-failure retries — see the `exec` step. A verdict is never retried: * a command that runs and exits non-zero is a normal `ExecResult`, so neither @@ -109,7 +156,7 @@ const RETRY_ON = ["ExecFailed", "StepFailed"] as const; export const oxlint = defineRun({ name: "oxlint", - version: "1.1.0", + version: "1.2.0", // Webhook-mode trigger — the zero-GHA lint gate. Fires on every PR push; the // run resolves oxlint from `npx`, so the sync, payload-only callback carries @@ -127,13 +174,19 @@ export const oxlint = defineRun({ gate: ({ payload }) => payload.pull_request?.draft !== true && payload.pull_request?.user?.login !== "dependabot[bot]", - inputs: ({ payload }) => ({ + // The return type is pinned to the schema's Type: with `version` omitted, + // an unannotated object literal is also a valid candidate for the run's + // input type, and TS infers the NARROWER one — which then drops `version` + // from the `run` body's `input`. + inputs: ({ payload }): typeof OxlintInput.Type => ({ repo: String(payload.repository?.full_name ?? "unknown/unknown"), sha: String(payload.pull_request?.head?.sha ?? ""), // The decoded input type carries these (their schema defaults); the // trigger restates them since `inputs` returns the decoded shape. args: "", - version: "1", + // `version` stays OMITTED so the run body resolves it from CONFIG_KV. + // Restating a literal here would pin every repo the dispatcher serves + // to one version again, unreachable from any of them. failOnNonZeroExit: true, }), }, @@ -150,6 +203,28 @@ export const oxlint = defineRun({ run: (input) => Effect.gen(function* () { + // resolve-version — a webhook dispatch omits `version` (its trigger + // callback is sync and cannot read config), so resolve it here: + // `oxlint.version:` → the dispatcher-wide `oxlint.version` → + // `VERSION_DEFAULT`. Unlike `offload-test`'s command, an unresolvable + // version is not a failure — the default always answers, so the gate + // never needs configuring to work. + // + // The step is SKIPPED when the dispatch carried a version (the `??` + // short-circuits before the `yield*`), so an Action-mode caller keeps the + // historical `checkout → exec → upload-log` step shape. + const version = + input.version ?? + (yield* step("resolve-version", () => + Effect.gen(function* () { + const perRepo = (yield* config.get(repoVersionKey(input.repo)))?.trim(); + if (perRepo !== undefined && perRepo.length > 0) return perRepo; + const dispatcherWide = (yield* config.get(VERSION_KEY))?.trim(); + if (dispatcherWide !== undefined && dispatcherWide.length > 0) return dispatcherWide; + return VERSION_DEFAULT; + }), + )); + // checkout — acquire a container (default lean image; oxlint needs only // Node for `npx`), clone at the SHA. NO install: oxlint lints source // directly, so `node_modules` is never needed (the whole point — a @@ -164,7 +239,7 @@ export const oxlint = defineRun({ // exec — fetch + run oxlint via `npx`. The args are appended verbatim; // the `.trim()` collapses the trailing space when `args` is empty. - const command = `npx --yes oxlint@${input.version} ${input.args}`.trim(); + const command = `npx --yes oxlint@${version} ${input.args}`.trim(); // Retries cover the PLATFORM, never the verdict. oxlint exiting non-zero // is a normal `ExecResult` decided below; only `ExecFailed` fails the // Effect, and that is the container rather than the code (observed diff --git a/runs/pr-review.ts b/runs/pr-review.ts index 1590cdb..68e4a4b 100644 --- a/runs/pr-review.ts +++ b/runs/pr-review.ts @@ -101,6 +101,7 @@ import { type StructuredOutputInvalid, type Tier, } from "@fractalboxdev/flare-dispatch-review-agent"; +import { VERSION_DEFAULT as OXLINT_VERSION_DEFAULT } from "./oxlint"; // Local helper — true if the PR carries the given label. const hasLabel = (payload: WebhookPayload, name: string): boolean => @@ -183,8 +184,15 @@ const DIFF_FILE = "/tmp/pr-review.diff"; /** Where the oxlint run writes its findings inside the container. */ const OXLINT_FILE = "/tmp/pr-review.oxlint.txt"; -/** oxlint version line fetched via `npx` — tracks the 1.x major. */ -const OXLINT_VERSION = "1"; +/** + * oxlint version fetched via `npx` for the grounding block — the SAME exact + * pin the `oxlint` gate runs, so the findings quoted to the reviewer are the + * findings that decide the check. Tracked the `1.x` major until 2026-08-18, + * when a minor release moved five rules into `correctness` and changed what + * every consumer's gate enforced overnight; see `VERSION_DEFAULT` in + * `runs/oxlint.ts` for the full account. + */ +const OXLINT_VERSION = OXLINT_VERSION_DEFAULT; /** Cap the changed-file list passed to oxlint (bounds the command line). */ const OXLINT_MAX_FILES = 60; /** Cap the grounding block prepended to the model context. */