Skip to content

🔧 Share one Oxlint policy between the gate and the sensor - #401

Merged
taras merged 2 commits into
mainfrom
agent/issue-395-oxlint-policy
Aug 9, 2026
Merged

🔧 Share one Oxlint policy between the gate and the sensor#401
taras merged 2 commits into
mainfrom
agent/issue-395-oxlint-policy

Conversation

@taras

@taras taras commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Why

The repository lint gate and the review sensor each carried a full copy of the Oxlint catalog, and the copies had drifted apart. The sensor turned pedantic and style on; the gate turned them off. The sensor's test override was missing **/tests/**. The two also ran different Oxlint versions — 1.56 installed, 1.74 downloaded by the review job — so the profiles could not meaningfully be compared at all.

The visible cost is on PR #375's own review: the sensor reported almost entirely generic style noise, which is exactly the signal the density model is supposed to keep clean.

Closes #395.

What changes

oxlint.shared.json becomes the one project-owned built-in policy, and both profiles extend it instead of carrying a catalog each.

oxlint.shared.json              plugins, categories, 14 curated signals, 17 denials, test override
├── .oxlintrc.json              gate: local JavaScript plugin + 5 blocking rules
└── .reviews/.oxlintrc.json     sensor: advisory, nothing else

Before: over the three representative files plus the 17 TypeScript files PR #375 changed, the sensor reports 96 diagnostics across 19 rulesfunc-names (37), no-ternary (13), id-length (12), max-lines-per-function (9), max-statements (8), and so on.

After: the same file set reports 2 diagnostics:

unicorn(no-array-sort)                 packages/code-review-agent/src/doctor.ts:59
unicorn(consistent-function-scoping)   scripts/tests/review-infrastructure.test.ts:461

Both come from the shared correctness/suspicious baseline. Neither is fixed here — this PR does not touch source to lower its own count.

The gate moves too, in the same direction. deno task lint goes from 1411 advisory warnings to 369 under the same Oxlint 1.74. The difference is exactly the two rules the settled policy now denies: require-yield (1039) and preserve-caught-error (3). No rule appears that did not appear before.

How it works

oxlint -c <profile> → extends resolves oxlint.shared.json → merged profile → report

Oxlint resolves a JSON extends path relative to the declaring configuration and merges the shared source under whatever the profile adds, including merging inherited overrides arrays. The gate therefore ends with two test-override blocks (the shared one plus its own local/no-redundant-test-scope), and the sensor with one.

Toolchain versions are pinned exactly to what .reviews/components/EnsureOxlint.md already provisions — Oxlint 1.74.0, tsgolint 0.25.0 — so both uses evaluate the same policy with the same linter.

Review guide

Start with: oxlint.shared.json

Then review:

  1. .oxlintrc.json and .reviews/.oxlintrc.json — what each profile still owns
  2. deno.json / package.json — the exact pins
  3. packages/code-review-agent/src/categories.ts and doctor.ts — one catalog, no duplicate
  4. scripts/tests/oxlint-policy.test.ts — the conformance suite
  5. specs/oxlint-sensor-spec.md §4 and §14, specs/code-review-agent-spec.md §13.1 and §14

Look carefully at:

  • Both profiles repeat "plugins". Oxlint 1.74 reads the plugin set from the entry configuration only: a list reached through extends is additive over Oxlint's defaults rather than replacing them, so a profile that inherited plugins silently gained the 17 oxc/* rules the current configs exclude. Repeating the list preserves today's rule set exactly; the alternative would have adopted 17 rules Unify Oxlint gate and review sensor policy #395 explicitly excludes. oxlint.shared.json keeps the list as the canonical declaration, and OP2/OP4 assert all three agree.

What must stay true

  • The sensor adds no built-in rule of its own, and its active built-in set stays a subset of the gate'snot enforced by inheritance. Oxlint merges first to last and the later configuration wins, so a profile that redeclared an inherited rule would override it. This invariant is a property of what the committed profiles contain, and the tests are what make it binding: OP2 fails if either profile grows a catalog, OP5 fails if the sensor's active set stops being a subset for any representative path.
  • The sensor stays advisory — no JavaScript plugin, no local/* rule, no error severity; checked by OP8.
  • The curated catalog has one ownerSENSOR_RULES in categories.ts; the native JSON must match it, checked by OP2, OP6, and OP12.
  • A version cannot drift silently — checked by OP13 across EnsureOxlint.md, deno.json, package.json, and the executed binary. Two of the four are additionally protected by the frozen lockfile, which refuses to run at all on a drifted manifest.

How to verify it

scripts/tests/oxlint-policy.test.ts runs under Deno, Node, and Bun (14 cases, OP1–OP14). Each was mutation-tested: the mutation was applied to a file copy, the suite run, the file restored.

Mutation Caught by
Sensor extends duplicated OP1, OP10
unicorn/filename-case denial deleted OP2, OP7
Sensor re-enables pedantic OP2, OP5
Sensor drops its plugins declaration OP4, OP5
Gate loses its local test override OP9, OP10
Shared drops no-debugger OP2
Shared test override loses **/tests/** OP2, OP10, OP11
EnsureOxlint.md pins a different Oxlint tag OP13
package.json drifts to ^1.74.0 OP13

A tenth mutation checks the invariant this PR actually relies on: making the sensor profile redeclare unicorn/filename-case: "warn", overriding a shared denial. It fails OP2, OP5, OP7, and OP8. That is the mechanism, since extends does not itself forbid the override.

OP11 is the one that needed a real lint rather than --print-config: the printed config keeps override blocks separate instead of flattening them into top-level rules, so a printed no-console: warn proves nothing about a test path. It lints a tests/ file and a sibling in a scratch directory and asserts only the sibling reports no-console.

Local run:

deno task lint          exit 0 (369 advisory warnings, 0 errors, format clean)
deno task check         exit 0
deno task check:jsr     Success Dry run complete
deno task test          413 passed (2843 steps), 0 failed
git diff --check        clean
tsc --project tsconfig.node.json --noEmit   exit 0 (Node 22.23.2)
tsx --test <4 suites>   34 pass, 0 fail (Node 22.23.2)
bun test <3 suites>     34 pass, 0 fail

Because this PR moves dependency state, deno task verify:clean ran against the committed revision. It passed on 7073d12: the release-target preparation held, every build phase stayed cache-pure, and the full battery ran with the site pair applying.

ok  vendor 4.3s   lint 3.9s   check 36.5s   test 638s   check:jsr 26.8s
ok  tsc 22.3s     test:node 207.8s          test:bun 304.1s
ok  docs 21.6s    site:check 7.4s           site:build 23.2s

the battery passed, and the tracked tree is unchanged
the chain holds: preparation installs, and nothing else does

Production evidence

The review job on this PR runs the real sensor path — the downloaded Oxlint 1.74.0 binary against .reviews/.oxlintrc.json — and reports:

Oxlint: 1 diagnostic across 1 file (1 rule)
Density: 0.001 violations/added-line

no-array-sort (1): packages/code-review-agent/src/doctor.ts

Issue #395 recorded 424 diagnostics for the state this replaces. The one surviving diagnostic comes from the shared correctness/suspicious baseline and is deliberately not fixed here.

The same report flags scope: 1167 lines changed across 6 directories, mixing config and source. That is the settled shape of this change. Of those lines, 430 are the new conformance suite and 287 are regenerated lockfiles — 61% between them. Only 27 lines of package source change. The plan for #395 established one atomic PR because configuration, toolchain versions, catalogs, and specifications must move together: splitting them would leave either the running sensor or the documented policy knowingly contradictory on main.

Scope

Included

  • One shared native policy, both profiles reduced to what they alone own
  • Exact Oxlint 1.74.0 / tsgolint 0.25.0 alignment with the production sensor
  • SENSOR_RULES as the single TypeScript catalog; Doctor's duplicate removed
  • The portable conformance suite
  • The two Oxlint specifications reconciled to the current policy

Intentionally unchanged

  • .reviews/components/EnsureOxlint.md — its pins and checksums are the production authority this PR matches, not something to edit
  • The 14 curated signals, their options, and the four-rule type-aware subset — no rule is adopted or dropped
  • The undecided candidates named by Unify Oxlint gate and review sensor policy #395 (no-unsafe-*, preserve-caught-error, no-duplicate-imports) stay explicitly off, pending individual adoption
  • DoctorResult, density calibration, diagnostic category names, normalized output, review workflow lifecycle, component protocols, and all public package APIs
  • The two diagnostics the sensor still reports, and every other pre-existing finding

New abstractions

  • SENSOR_RULES (categories.ts) exists because Doctor and the native JSON both needed the curated catalog and each kept its own copy. Consumers: doctor.ts and the conformance suite. It is internal policy structure — mod.ts is not widened and no changeset is added.
  • runOxlint(config, args) (scripts/tests/oxlint.ts) generalizes the existing helper to any config and returns the exit code and stderr, which the conformance suite needs to fail loudly on a broken invocation. oxlint() now delegates to it; the rule tests are unchanged.

Generated or mechanical changes

  • deno.lock, pnpm-lock.yaml, and bun.lock were regenerated by deno install --frozen=false, pnpm install, and bun install --lockfile-only. The churn is confined to oxlint and tsgolint resolutions and their platform-binding integrity hashes; no unrelated package moved.

Revision at 60a6fdf

Review feedback on 7073d12, addressed without changing the settled policy design:

  1. The specifications overclaimed. They said extends made a profile-level override impossible. Oxlint merges first to last and the later configuration wins, so it does not. specs/oxlint-sensor-spec.md §4.1, specs/code-review-agent-spec.md §13.1, and this description now state the real invariant — the committed sensor profile declares no built-in rule of its own, its active built-in set stays a subset of the gate's, and OP2/OP5 are what make that binding. The new tenth mutation above demonstrates it.
  2. runOxlint shared one TextDecoder between stdout and stderr. A multi-byte character split across chunks on one pipe could take its continuation bytes from the other, and a character still pending at exit was dropped. Each pipe now has its own decoder and both are flushed after the process settles. The scoped lifecycle and captured exit status are unchanged.
  3. The conformance suite reached past @effectionx/fs. Directory and file operations now use ensureDir, writeTextFile, and rm; node:fs remains only for mkdtempSync, which that package does not provide, with the rationale stated at the call site. One comment that narrated ordinary code is gone.

Re-verified at this head: fmt, lint (exit 0, 369 advisory warnings), check, check:jsr, the policy and helper-dependent suites under Deno (8 files, 64 steps, 0 failed), and the policy suite under Node 22 and Bun (34/34 each).

Risks and limitations

  • Oxlint 1.56 → 1.74 registers 10 built-in rules the old toolchain did not. The before/after gate comparison shows none produced a new finding in this tree, but future code may encounter them.
  • The conformance suite invokes npx --yes oxlint, matching the existing rule tests; it resolves the local node_modules/.bin copy on a prepared worktree.
  • Rollback: revert the single commit. Do not revert oxlint.shared.json alone — both profiles extend it.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

The repository lint gate and the review sensor each carried a full copy of
the Oxlint catalog, and the copies had drifted: the sensor turned pedantic
and style on, so it reported 96 diagnostics over PR #375's files, nearly all
generic style noise. The two also ran different Oxlint versions, so the
profiles could not be compared at all.

oxlint.shared.json becomes the one built-in policy. Both profiles extend it
through a native JSON extends path, so a profile can no longer enable a rule
the shared policy disables. The gate adds only its local JavaScript plugin
and blocking rules; the sensor adds nothing.

Oxlint and tsgolint are pinned to the exact versions the production review
sensor already provisions, so a routine lock refresh cannot expand the rule
set without changing the sensor.

SENSOR_RULES in categories.ts becomes the one TypeScript catalog, and Doctor
derives its available and missing rules from it.

scripts/tests/oxlint-policy.test.ts proves inheritance, catalog ownership,
the sensor-subset relation, override survival, and version agreement under
Deno, Node and Bun.

Closes #395
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown

PR #401: 🔧 Share one Oxlint policy between the gate and the sensor

14 files, +882 / -310

Scope

🔴 PR has 1192 lines changed. Split into focused PRs.

🟡 1192 lines changed. PRs under 400 receive more thorough review.

🟡 Changes span 6 directories.

🟡 PR mixes config and source changes.

Structural

✅ No structural bloat detected.

Slop

  • specs/oxlint-sensor-spec.md:347// .oxlintrc.json
  • specs/oxlint-sensor-spec.md:367// .reviews/.oxlintrc.json

Static Analysis

Oxlint: 1 diagnostic across 1 file (1 rule)
Density: 0.001 violations/added-line

no-array-sort (1): packages/code-review-agent/src/doctor.ts

Correctness

No extraneous code patterns detected. All changes directly support the shared Oxlint policy implementation, toolchain alignment, and conformance verification as described. The repeated plugin lists, version pins, and test suite are all required for the stated purpose.

The specifications claimed extends made a profile-level override impossible.
Oxlint merges first to last and the later configuration wins, so a profile
that redeclared an inherited rule would override it. The real invariant is
that the committed sensor profile declares no built-in rule of its own and
that its active built-in set stays a subset of the gate's, with OP2 and OP5
failing verification when either stops holding.

runOxlint gave stdout and stderr one shared TextDecoder, so a multi-byte
character split across chunks on one pipe could take its continuation bytes
from the other, and a character left pending at exit was dropped. Each pipe
now has its own decoder, and both are flushed after the process settles.

The conformance suite takes its directory and file operations from
@effectionx/fs, keeping node:fs only for mkdtempSync, which that package
does not provide.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 2 redundant comments. Inline suggestions to remove them below.

test override. The sensor adds nothing:

```jsonc
// .oxlintrc.json

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// .oxlintrc.json

]
}

// .reviews/.oxlintrc.json

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant comment — restates what the code does.

Suggested change
// .reviews/.oxlintrc.json

@taras
taras marked this pull request as ready for review August 9, 2026 03:15
@taras
taras enabled auto-merge (squash) August 9, 2026 03:15
@taras
taras merged commit 2fc2fb2 into main Aug 9, 2026
11 checks passed
@taras
taras deleted the agent/issue-395-oxlint-policy branch August 9, 2026 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unify Oxlint gate and review sensor policy

1 participant