Files
orca/config/scripts/check-changed-code-quality.mjs
Neil 22ce8d69a1 fix(lint): enable anti-slop/no-module-mocking (#20783)
The rule rejects `vi.mock` / `vi.doMock` / `vi.unstable_mockModule` and the
`jest` equivalents, on the argument that a test which rewrites the module graph
asserts against a stand-in the production code never sees. It is already off for
`**/*.test.{ts,tsx}`, `**/*.spec.{ts,tsx}`, `tests/**` and `**/__mocks__/**` via
the existing override in config/oxlint-anti-slop.json; that override is
unchanged here. What the rule actually catches is module mocking that has drifted
out of a spec and into a first-party `.ts` support module, where nothing marks it
as test-only.

73 violations at baseline, all of them in test-support code. 9 were relocated
back into spec files the override already exempts; the remaining 64 sit in 10
files that are test-only but do not match the override globs, and carry a
file-level disable naming the rule and the reason.

Relocated:
- terminal-hydration-store-test-bootstrap.ts: the sonner / sync-runtime-graph /
  pty-transport `vi.mock` calls moved into the two specs that import it
  (terminals-hydration-canonical-rows, terminals-hydration-canonical-pty-overlap).
  Vitest hoists `vi.mock` inside a test file, so registration is strictly earlier
  than the previous module-eval-time call; the bootstrap keeps only the preload
  API proxy. Both importers were updated.
- ipc-events-ssh-authority-test-fixtures.ts: the 6 direct-ssh `vi.doMock` calls
  moved into useIpcEvents-agent-status-ssh-authority.test.ts as a local
  `stubDirectSshModules()` helper, which also de-duplicates the three copies the
  spec already had inline. The fixture now returns the store state and coordinator
  doubles it builds, typed via the exported DirectSshReconnectCoordinatorDouble.

Suppressed, with justification (each is `/* oxlint-disable
anti-slop/no-module-mocking -- ... */`, rule named, no blanket disable):
- config/scripts/headless-serve-shutdown-matrix.test.mjs (1) - a genuine Vitest
  spec that the override misses only because its globs say {ts,tsx}. The script
  under test is a top-level CLI module; the alternative is spawning real docker.
- src/main/codex-accounts/runtime-home-service-test-harness.ts (1) - stubs one
  probe predicate in ../pty/shell-startup-env, imported directly by several
  main-process readers; 17 specs share it.
- src/main/computer/desktop-script-provider-test-harness.ts (2) - stubs
  child_process/fs-promises for a provider that shells out; 8 specs share it.
- src/main/github/work-item-search-test-harness.ts (4) - one consumer lives in
  tests/e2e, where the relative mock ids resolve differently, so moving the calls
  into the specs would silently stop mocking there.
- src/renderer/src/components/automations/automations-page-test-harness.tsx (14)
  - the mount rig for 10 AutomationsPage specs.
- src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-test-harness.ts
  (1) - stubs refreshWebRuntimeSessionTabsSnapshot, imported directly by several
  renderer runtime modules; 18 specs share it.
- src/renderer/src/hooks/ipc-events-agent-status-window-test-fixtures.ts (7) -
  stubReactSyncEffect/stubAuxiliaryModules, shared by 11 specs.
- src/renderer/src/hooks/ipc-events-close-routing-test-harness.ts (11) - stubs
  and hook invocation are one unit; 4 specs share it.
- src/renderer/src/hooks/ipc-events-terminal-create-test-harness.ts (13) - its
  only spec is at 799 of an 800 max-lines budget.
- src/renderer/src/hooks/ipc-events-test-harness.ts (10) - shared by 8 specs.

No violation was converted to real dependency injection, and no max-lines disable
was added.

Verified: the audit command exits 0 with no output (and reports errors on a
planted probe, so the rule is live); node config/scripts/run-typecheck-projects-in-parallel.mjs
exits 0; 354 spec files / 2506 tests covering every importer of every touched
file pass. No mobile/ file was touched.

The changed-code quality gate's root Oxlint scan runs without --config so it never
loads the anti-slop JS plugin, which made all 10 of those file-level suppressions
read as "Unused oxlint-disable directive". check-changed-code-quality.mjs now
exempts directives naming an anti-slop rule from that unused-directive warning,
the same carve-out isCastingDirectiveUnusedWarning already makes for the casting
suppressions the casting config enforces. Such a directive can never suppress a
root-config rule, so nothing the root scan would otherwise report is hidden;
audit:anti-slop remains the scan that enforces the rule.
2026-09-15 00:41:17 -07:00

455 lines
16 KiB
JavaScript

import { execFileSync, spawnSync } from 'node:child_process'
import { existsSync, readFileSync } from 'node:fs'
import path from 'node:path'
import process from 'node:process'
import { pathToFileURL } from 'node:url'
import { resolvePullRequestDiffBase } from './git-pull-request-diff-base.mjs'
import { resolveOxlintInvocation } from './oxlint-cli-invocation.mjs'
const SOURCE_FILE_PATTERN = /\.(?:[cm]?[jt]sx?)$/
const ROOT_CODE_QUALITY_IGNORED_PREFIXES = ['cloud/']
const CASTING_RULE = 'typescript/consistent-type-assertions'
const CASTING_DISABLE_PATTERN =
/\/[/*]\s*(?:oxlint|eslint)-disable(?:-next-line|-line)?\s[^\n]*typescript\/consistent-type-assertions/
const ANTI_SLOP_DISABLE_PATTERN =
/\/[/*]\s*(?:oxlint|eslint)-disable(?:-next-line|-line)?\s[^\n]*\banti-slop\//
export const OXLINT_SCANS = [
{
// Why: no --config, so Oxlint keeps discovering nested configs. Pinning the root
// config would apply root rules to mobile/, whose .oxlintrc.json turns them off.
label: 'code quality',
args: ['--report-unused-disable-directives-severity', 'warn']
},
{
label: 'casting code quality',
args: ['--config', 'config/oxlint-code-quality-casting.json']
},
{
label: 'type-aware code quality',
args: ['--type-aware', '--config', 'config/oxlint-code-quality-type-aware.json']
},
{
label: 'React Doctor',
args: ['--config', 'config/oxlint-react-doctor.json']
},
{
// Why changed-lines only: the renderer carries ~4.7k pre-existing restyle/raw-color
// findings. Gating added lines holds the line without a repo-wide migration.
label: 'design system',
args: ['--config', 'config/oxlint-design-system.json']
}
]
const SUPPRESSED_REACT_DOCTOR_DIAGNOSTICS = new Map([
[
'react-doctor(no-adjust-state-on-prop-change)',
new Set([
'src/renderer/src/components/use-task-page-github-issue-draft.ts',
'src/renderer/src/components/use-task-page-jira-creation-state.ts'
])
],
[
'react-doctor(no-derived-state-effect)',
new Set([
'src/renderer/src/components/editor/combined-diff/review-controls/use-combined-diff-view-preferences.ts'
])
],
[
// The rule wants one named handle cleared by name. Both startup effects arm a variable number
// of refresh timers, every one of them through addTimer into `timers`, which their cleanups
// clear -- a shape the rule reports whether the handles live in an array, a Set, or a nested
// helper. The finding predates this list; it surfaced when the effect body changed. This map
// keys on file, not line, so the entry covers both effects in it; nothing else in the file
// arms a timer, so widening it further is the only alternative, not a narrower option.
'react-doctor(effect-needs-cleanup)',
new Set(['mobile/src/session/use-mobile-session-startup.ts'])
]
])
export function parseAddedLineRanges(diff) {
const ranges = []
const hunkPattern = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/
for (const line of diff.split(/\r?\n/)) {
const match = hunkPattern.exec(line)
if (!match) {
continue
}
const start = Number.parseInt(match[1], 10)
const count = match[2] === undefined ? 1 : Number.parseInt(match[2], 10)
if (count > 0) {
ranges.push({ start, end: start + count - 1 })
}
}
return ranges
}
export function overlapsAddedLines(startLine, endLine, ranges) {
return ranges.some((range) => startLine <= range.end && endLine >= range.start)
}
function runGit(root, args, options = {}) {
return execFileSync('git', args, {
cwd: root,
encoding: options.encoding ?? 'utf8',
maxBuffer: 64 * 1024 * 1024
})
}
function splitNullDelimited(output) {
return output.split('\0').filter(Boolean)
}
export function isRootCodeQualityPath(file) {
return !ROOT_CODE_QUALITY_IGNORED_PREFIXES.some((prefix) => file.startsWith(prefix))
}
function resolveBase(root, requestedBase) {
for (const candidate of [
requestedBase,
process.env.ORCA_CODE_QUALITY_BASE,
'origin/main',
'main'
]) {
if (!candidate) {
continue
}
const result = spawnSync('git', ['rev-parse', '--verify', `${candidate}^{commit}`], {
cwd: root,
stdio: 'ignore'
})
if (result.status === 0) {
return candidate
}
}
throw new Error('Pass the pull request base SHA or make origin/main available locally.')
}
export function collectAddedLineRanges(root, requestedBase) {
const base = resolveBase(root, requestedBase)
const mergeBase = runGit(root, ['merge-base', base, 'HEAD']).trim()
const comparisonBase = resolvePullRequestDiffBase(root, mergeBase)
const changedFiles = splitNullDelimited(
runGit(root, ['diff', '--name-only', '-z', '--diff-filter=ACMRTUB', comparisonBase, '--'])
)
const untrackedFiles = splitNullDelimited(
runGit(root, ['ls-files', '--others', '--exclude-standard', '-z'])
)
const rangesByFile = new Map()
for (const file of changedFiles) {
if (
!isRootCodeQualityPath(file) ||
!SOURCE_FILE_PATTERN.test(file) ||
!existsSync(path.join(root, file))
) {
continue
}
const diff = runGit(root, ['diff', '--unified=0', '--no-color', comparisonBase, '--', file])
const ranges = parseAddedLineRanges(diff)
if (ranges.length > 0) {
rangesByFile.set(file, ranges)
}
}
for (const file of untrackedFiles) {
const absolutePath = path.join(root, file)
if (
!isRootCodeQualityPath(file) ||
!SOURCE_FILE_PATTERN.test(file) ||
!existsSync(absolutePath)
) {
continue
}
const lineCount = readFileSync(absolutePath, 'utf8').split(/\r?\n/).length
rangesByFile.set(file, [{ start: 1, end: lineCount }])
}
return { base, comparisonBase, rangesByFile }
}
function parseOxlintOutput(stdout, label) {
const start = stdout.indexOf('{')
const end = stdout.lastIndexOf('}')
if (start === -1 || end === -1) {
throw new Error(`${label} did not return Oxlint JSON output.`)
}
return JSON.parse(stdout.slice(start, end + 1))
}
function normalizedDiagnosticPath(root, filename) {
const absolutePath = path.isAbsolute(filename) ? filename : path.join(root, filename)
return path.relative(root, absolutePath).split(path.sep).join('/')
}
function diagnosticLineRange(root, filename, span) {
const startLine = span.line
if (!Number.isInteger(startLine)) {
return null
}
if (!Number.isInteger(span.offset) || !Number.isInteger(span.length) || span.length === 0) {
return { start: startLine, end: startLine }
}
const absolutePath = path.isAbsolute(filename) ? filename : path.join(root, filename)
const source = readFileSync(absolutePath)
const highlighted = source.subarray(span.offset, span.offset + span.length).toString('utf8')
return { start: startLine, end: startLine + (highlighted.match(/\n/g)?.length ?? 0) }
}
// Why: a file-splitting refactor makes every line of the new module an "added"
// line, so pre-existing lint debt in code that merely MOVED starts failing the
// changed-lines gate. The only way to satisfy it is to edit the moved code,
// which is exactly what a behavior-preserving refactor must not do. So a
// diagnostic is exempt when its highlighted lines already existed, verbatim and
// contiguous, somewhere in the base revision of the files this change touches.
function normalizeSourceLine(line) {
return line.replace(/\s+/g, ' ').trim()
}
export function collectBaseLineBlocks(root, comparisonBase, files = null) {
// Why: in a split, the moved code's base text lives in the ORIGINAL file, which is
// often deleted or renamed away. Deleted paths never reach the changed-file list
// (it filters to ACMRTUB), so read every path the diff touches, deletions included.
const paths =
files ??
splitNullDelimited(runGit(root, ['diff', '--name-only', '-z', comparisonBase, '--'])).filter(
(file) => SOURCE_FILE_PATTERN.test(file)
)
const blocks = []
for (const file of paths) {
const result = spawnSync('git', ['show', `${comparisonBase}:${file}`], {
cwd: root,
encoding: 'utf8',
maxBuffer: 64 * 1024 * 1024
})
if (result.status !== 0 || typeof result.stdout !== 'string') {
continue
}
blocks.push(
result.stdout
.split(/\r?\n/)
.map(normalizeSourceLine)
.filter((line) => line !== '')
)
}
return blocks
}
export function isMovedCode(highlightedLines, baseBlocks) {
const needle = highlightedLines.map(normalizeSourceLine).filter((line) => line !== '')
if (needle.length === 0) {
return false
}
// Why a near-match rather than an exact contiguous one: a split moves a block
// verbatim but a diagnostic's span often reaches past it — most commonly to a
// hook dependency array, which legitimately grows when closure variables become
// props. Requiring every line to match would report the moved body as new. So:
// the block must still start at the same line in the base and appear IN ORDER,
// and nearly all of it must be present. Genuinely new code shares neither the
// anchor nor the ordering, so it stays reported.
const MIN_COVERAGE = 0.9
return baseBlocks.some((rawHaystack) => {
const haystack = rawHaystack.map(normalizeSourceLine).filter((line) => line !== '')
for (let start = 0; start < haystack.length; start += 1) {
if (haystack[start] !== needle[0]) {
continue
}
let matched = 1
let cursor = start + 1
for (let index = 1; index < needle.length && cursor < haystack.length; index += 1) {
while (cursor < haystack.length && haystack[cursor] !== needle[index]) {
cursor += 1
}
if (cursor < haystack.length) {
matched += 1
cursor += 1
}
}
if (matched / needle.length >= MIN_COVERAGE) {
return true
}
}
return false
})
}
function diagnosticHighlightedLines(root, filename, span) {
const absolutePath = path.isAbsolute(filename) ? filename : path.join(root, filename)
const source = readFileSync(absolutePath, 'utf8').split(/\r?\n/)
const range = diagnosticLineRange(root, filename, span)
if (range === null) {
return []
}
return source.slice(range.start - 1, range.end)
}
export function diagnosticTouchesAddedLines(
diagnostic,
rangesByFile,
root = process.cwd(),
baseBlocks = []
) {
const file = normalizedDiagnosticPath(root, diagnostic.filename)
const ranges = rangesByFile.get(file)
if (!ranges) {
return false
}
return (diagnostic.labels ?? []).some((label) => {
const lineRange = diagnosticLineRange(root, diagnostic.filename, label.span)
if (lineRange === null || !overlapsAddedLines(lineRange.start, lineRange.end, ranges)) {
return false
}
return !isMovedCode(
diagnosticHighlightedLines(root, diagnostic.filename, label.span),
baseBlocks
)
})
}
function annotationValue(value) {
return String(value).replaceAll('%', '%25').replaceAll('\r', '%0D').replaceAll('\n', '%0A')
}
function printDiagnostic(diagnostic, root) {
const file = normalizedDiagnosticPath(root, diagnostic.filename)
const line = diagnostic.labels?.[0]?.span?.line ?? 1
const code = diagnostic.code ?? 'oxlint'
console.error(
`::error file=${annotationValue(file)},line=${line},title=${annotationValue(code)}::${annotationValue(diagnostic.message)}`
)
console.error(`${file}:${line} ${code}: ${diagnostic.message}`)
}
// Why: only the casting scan enforces `assertionStyle: never`, so under the root config an
// `as` cast is legal and the SAFETY: directive AGENTS.md mandates reads as unused. The untyped
// scan reports that as a warning, which the gate counts, so exempt exactly those directives.
export function isCastingDirectiveUnusedWarning(diagnostic, root) {
if (!/^Unused (?:oxlint|eslint)-disable/.test(diagnostic.message ?? '')) {
return false
}
return (diagnostic.labels ?? []).some((label) =>
diagnosticHighlightedLines(root, diagnostic.filename, label.span).some((line) =>
CASTING_DISABLE_PATTERN.test(line)
)
)
}
// Why: the anti-slop rules live in a JS plugin that only config/oxlint-anti-slop.json loads, so
// the root scan never sees those rule names and reports every anti-slop suppression as unused.
// `audit:anti-slop` is the scan that enforces them.
export function isAntiSlopDirectiveUnusedWarning(diagnostic, root) {
if (!/^Unused (?:oxlint|eslint)-disable/.test(diagnostic.message ?? '')) {
return false
}
return (diagnostic.labels ?? []).some((label) =>
diagnosticHighlightedLines(root, diagnostic.filename, label.span).some((line) =>
ANTI_SLOP_DISABLE_PATTERN.test(line)
)
)
}
// Why: oxlint cannot see the AGENTS.md requirement that every casting suppression carry a
// line-specific SAFETY: rationale, so the directive text itself is checked over added lines.
export function findCastingDirectivesMissingSafety(root, rangesByFile) {
const findings = []
for (const [file, ranges] of rangesByFile) {
const absolutePath = path.join(root, file)
if (!existsSync(absolutePath)) {
continue
}
readFileSync(absolutePath, 'utf8')
.split(/\r?\n/)
.forEach((text, index) => {
const line = index + 1
if (
CASTING_DISABLE_PATTERN.test(text) &&
!text.includes('SAFETY:') &&
overlapsAddedLines(line, line, ranges)
) {
findings.push({
filename: file,
code: `${CASTING_RULE} (missing SAFETY:)`,
message: `Suppressing ${CASTING_RULE} requires a line-specific "SAFETY:" explanation.`,
labels: [{ span: { line } }]
})
}
})
}
return findings
}
function isSuppressedDiagnostic(diagnostic, root) {
const files = SUPPRESSED_REACT_DOCTOR_DIAGNOSTICS.get(diagnostic.code)
return files?.has(normalizedDiagnosticPath(root, diagnostic.filename)) ?? false
}
function runOxlintScan(root, scan, files) {
const { command, prefixArgs } = resolveOxlintInvocation(root)
const result = spawnSync(command, [...prefixArgs, ...scan.args, '--format', 'json', ...files], {
cwd: root,
encoding: 'utf8',
maxBuffer: 128 * 1024 * 1024,
windowsHide: true
})
if (result.error) {
throw result.error
}
if (!result.stdout.trim()) {
process.stderr.write(result.stderr)
throw new Error(`${scan.label} failed before producing diagnostics.`)
}
return parseOxlintOutput(result.stdout, scan.label).diagnostics ?? []
}
export function main(
root = process.cwd(),
requestedBase = process.argv.slice(2).find((argument) => argument !== '--')
) {
const { base, comparisonBase, rangesByFile } = collectAddedLineRanges(root, requestedBase)
const files = [...rangesByFile.keys()]
if (files.length === 0) {
console.log(`Changed-code quality gate: no changed JavaScript or TypeScript since ${base}.`)
return 0
}
const baseBlocks = collectBaseLineBlocks(root, comparisonBase)
let failures = 0
for (const scan of OXLINT_SCANS) {
const diagnostics = runOxlintScan(root, scan, files).filter(
(diagnostic) =>
!isSuppressedDiagnostic(diagnostic, root) &&
!isCastingDirectiveUnusedWarning(diagnostic, root) &&
!isAntiSlopDirectiveUnusedWarning(diagnostic, root) &&
diagnosticTouchesAddedLines(diagnostic, rangesByFile, root, baseBlocks)
)
for (const diagnostic of diagnostics) {
printDiagnostic(diagnostic, root)
}
failures += diagnostics.length
console.log(
`${scan.label}: ${diagnostics.length} new finding(s) across ${files.length} changed file(s).`
)
}
const missingSafety = findCastingDirectivesMissingSafety(root, rangesByFile)
for (const diagnostic of missingSafety) {
printDiagnostic(diagnostic, root)
}
failures += missingSafety.length
console.log(
`casting SAFETY: rationale: ${missingSafety.length} new finding(s) across ${files.length} changed file(s).`
)
if (failures > 0) {
console.error(
`Changed-code quality gate failed with ${failures} finding(s) since ${comparisonBase.slice(0, 12)}.`
)
return 1
}
console.log(`Changed-code quality gate passed since ${comparisonBase.slice(0, 12)}.`)
return 0
}
if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {
process.exit(main())
}