Skip to content

chore(lint): enforce cyclomatic complexity ceiling - #861

Merged
benvinegar merged 1 commit into
refactor/simplify-session-cli-dispatchfrom
chore/enforce-complexity-ceiling
Aug 26, 2026
Merged

benvinegar merged 1 commit into
refactor/simplify-session-cli-dispatchfrom
chore/enforce-complexity-ceiling

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Problem

The codebase has no automated ceiling for cyclomatic complexity, so a function can accumulate new control-flow paths without lint feedback. The recent audit found the session command dispatcher at 91 before #857 split it into focused handlers.

Approach

  • enable Oxlint's ESLint-compatible complexity rule globally;
  • set an initial maximum of 80;
  • rely on the existing bun run lint CI step to enforce it.

This is intentionally an initial regression ceiling rather than the long-term target. On this branch the highest existing score is 78 (App), followed by 76 (validateFileViewLayout), so 80 adds enforcement without grandfathering or suppressions. The ceiling can be ratcheted downward as existing hotspots are simplified.

This PR is stacked on #857 and should merge after it. Targeting its branch keeps this diff to the Oxlint configuration and an empty maintenance Changeset.

Validation

Tested on Linux:

  • bun run lint — 782 files, 93 rules, zero warnings/errors;
  • bun run typecheck;
  • focused oxfmt --check for the changed files;
  • generated complexity-80 boundary probe passes;
  • generated complexity-81 probe fails with Maximum allowed is 80;
  • git diff --check.

Follow-up

Lower the ceiling as the remaining high-complexity functions are simplified. A global ceiling does not prevent a function below 80 from growing toward it.

This PR description was generated by Pi using OpenAI GPT-5.6 Sol

@vercel

vercel Bot commented Aug 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hunk-web Ignored Ignored Aug 26, 2026 1:50pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds an Oxlint cyclomatic-complexity ceiling as an initial regression guard without changing runtime behavior.

  • Enables the ESLint-compatible complexity rule globally with a maximum of 80.
  • Adds the repository-standard empty Changeset for maintenance-only work.

Confidence Score: 5/5

The PR appears safe to merge with no actionable correctness, security, or release-process issues identified.

The lint configuration is limited to static analysis, and the empty Changeset matches the repository’s documented and tested maintenance-release workflow.

Important Files Changed

Filename Overview
.oxlintrc.json Adds the global complexity rule at the documented ceiling; no actionable defect was established.
.changeset/enforce-complexity-ceiling.md Adds a canonical empty maintenance Changeset consistent with repository release conventions.

Reviews (1): Last reviewed commit: "chore(lint): enforce complexity ceiling" | Re-trigger Greptile

@benvinegar
benvinegar merged commit cc829d0 into refactor/simplify-session-cli-dispatch Aug 26, 2026
13 checks passed
aviggiano added a commit to monad-developers/ultrafuzz that referenced this pull request Sep 29, 2026
…y ceiling

#1005 deliberately added no baseline or suppression registry, and #960 asks for
a complexity ceiling in the style of modem-dev/hunk#861: one global maximum set
at today's worst function, with nothing grandfathered, lowered as hotspots are
simplified. Drop eslint-suppressions.json and the always-on size budgets, keep
the diff-limited strict budgets for changed lines, and fail any function above
complexity 90 (synchronizeLinkedWorkflowRun is at 90 once the pending
simplification PRs land). The changelog entry moves to the consolidated
release notes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
aviggiano added a commit to monad-developers/ultrafuzz that referenced this pull request Sep 29, 2026
…chet complexity with a lint baseline (#1184)

* fix(config): accept smol-toml 1.9 tables when loading project config

smol-toml 1.9.0 (released 2026-09-22) builds every parsed table with
Object.create(null). The config loader treats only objects whose
prototype is Object.prototype as tables, and it sorts
Object.entries(table) with the default comparator, which stringifies each
value. A packed install has no lockfile and resolves the ^1.7.1 range to
1.9.0, so `ultrafuzz init` failed while loading the shipped defaults
("project must be a table; run must be a table; ..."). This is the
packed-install failure that turned the package-gates lane red in main run
36495460966.

Rebuild the parsed document with structuredClone, which returns the same
data as ordinary objects, so the loader sees what it saw with 1.7.
`__proto__` keys stay own data properties.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: run every release validation lane on pull requests and never cancel main runs

Pull requests ran only the five runtime lanes. package-gates, cli and
benchmark-history-typecheck (which holds the workspace typecheck) first ran
after merge, so their regressions landed on main: the latest main run,
36495460966, fails package-gates. Every lane now runs on every event. The lane
table keeps its test that it runs each gate scripts/validate-release.mjs
defines exactly once; the pull-request filter, PULL_REQUEST_REQUIRED_GATES and
the --event argument are gone.

The lanes no longer wait for the build and super-linter jobs, and the PR-only
runtime and CLI smoke runners are deleted because the lanes run those suites in
full. release-gates still requires every job. Measured on existing runs: a pull
request took about 60 minutes (8.0 min for build and smoke, then 51.5 min for
the slowest lane in run 36279711018); on the latest main push the new PR lanes
took 12.9 (package-gates), 41.5 (cli) and 9.8 minutes against 26.8-39.5 minutes
for the runtime lanes.

All pushes to main shared one concurrency group; 24 of the last 40 main push
runs were cancelled (as of 2026-09-29). cancel-in-progress: false alone would
not stop that, because GitHub also cancels a pending run in the group when a
newer run queues, and main pushes arrive in bursts (three within 9 seconds on
2026-09-28). Runs other than pull requests now get their own group, keyed by
github.run_id.

Delete the ci.yml line-by-line assertions in packages/modal/test/ci-config.test.ts
and the timeout_minutes === 120 change detector. The remaining guard parses
ci.yml and checks that release validation is not gated off pull requests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: run the release validation lanes under eatmydata

Every runtime and CLI test that launches a run copies a sealed execution
snapshot and fsyncs each file, twice (at write and again when the snapshot is
sealed). The deep analysis measured 55,998 fsyncs taking 222 s of a 163-261 s
test locally; under eatmydata the same test took 23-33 s, and a CLI test went
from 169 s to 35 s. A CI runner is discarded after the job, so durability
across a crash buys nothing there.

Install eatmydata in the release-validation job and run validate:release
under it; its LD_PRELOAD reaches every child the gate commands spawn.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: report unused exports without blocking the build

`pnpm -w knip` leaves out knip's exports, types and duplicates categories, so
unused exports accumulate unseen; on this tree knip reports 52 unused exports
and 38 unused exported types. Report them in a continue-on-error step that
reuses the pinned knip script. Several open pull requests delete dead code;
once they land, these categories can move into the blocking knip step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test: give vitest packages a 30 s default test timeout

config, topology, prompts, evals, evmbench and modal run a bare `vitest run`
with no config file, so every test gets vitest's 5 s default. Main went red on
it in runs 34967618488 (modal ci-config and modal-documents: "Test timed out in
5000ms") and 34634052856 (modal worker-result), and #1126 raised individual
cases instead. Pass --testTimeout=30000 in the six package test scripts. Cases
that set their own timeout keep it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(cli): remove each test's temporary project when the test ends

cli.test.ts, lifecycle-commands.test.ts and audit-profile-commands.test.ts
created their project with mkdtemp and never removed it. On main, one run-
launching test ("status surfaces a terminal product and live workflow
lifecycle divergence") left 446 MB and 34,091 entries in TMPDIR; the sealed
snapshot directories are mode dr-x, so a plain `rm -rf` cannot delete them.
A CI lane accumulated every test's run directories until the job ended.

temporaryRoot(prefix, t) creates the canonical directory and registers a
t.after hook that restores owner write permission on the way down and
removes it, together with the `<root>-fake-bin` directory the fake engine
fixtures create beside it. The tests pass their context to tempProject.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* build(lint): enforce complexity and size budgets on all code against a recorded baseline

The complexity and size rules ran only in the diff-limited strict lint, which
reports a message only when its line changed. ESLint reports these rules on a
function's first line (max-lines on the first line past the limit), so adding
branches inside an over-budget function passed, while editing only its
signature failed, forcing an unrelated refactor on a fix.

Move complexity, max-depth, max-lines, max-lines-per-function,
max-nested-callbacks, max-params and max-statements into the always-on config,
with the same relaxed values for tests and templates, and record the 1,003
existing violations (197 files; 701 of them in 108 packages/*/src files) with
ESLint's bulk suppressions in eslint-suppressions.json. `pnpm -w lint` now
fails when a file gains a violation of a rule and when a recorded count is
higher than the file's violations; `pnpm -w lint:prune` lowers the counts.

The diff-limited strict lint keeps the type-aware strict rules, no-console and
the TODO/FIXME check. It passes --pass-on-unpruned-suppressions because it
sees only changed lines, so its counts are below the recorded ones by design.
ESLint writes the suppressions file without a final newline, so prettier
ignores it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: record the CI, lint baseline and smol-toml changes in the changelog

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: stop super-linter failing every edit to CHANGELOG.md on line length

Super-Linter lints each changed Markdown file in full with its default
markdownlint rules, including MD013 at 400 characters. Four of the 109
tracked Markdown files have longer lines: CHANGELOG.md keeps each entry on
one line (44 such lines), and docs/reference/agent-adapter-boundaries.md,
docs/explanation/provider-harness-research.md and docs/CODE_OF_CONDUCT.md
have long table rows or paragraphs. Any pull request that touched one of
them failed `External static analysis` on lines it did not change, and no
merged pull request has edited CHANGELOG.md since Super-Linter was added.

Use Super-Linter's default markdownlint rules from the pinned v8.7.0
template, with MD013 off, as .github/linters/.yamllint.yml already does for
YAML line length. markdownlint-cli 0.49.0 (the version Super-Linter runs)
reports 46 MD013 errors on this branch's changed Markdown with the default
template and none with this file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(modal): stop pinning the package test script to an exact string

The smoke-isolation test asserted `scripts.test === "vitest run"`, so adding
--testTimeout=30000 failed package-gates in this pull request's first run. The
property the test protects, that the unit-test script never runs the real
Modal smoke, is still asserted by `not.toContain("smoke")` two lines later.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: note the Super-Linter line-length change in the changelog

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(cli): check that temporaryRoot removes a sealed tree and its fake-bin sibling

The per-test cleanup was only measured, so a regression would bring the
temporary-directory leak back silently. The new test builds the shape a
launched run leaves (read-only files in dr-x directories) plus the
`<root>-fake-bin` sibling inside a subtest and asserts both are gone once the
subtest ends. It fails if the helper skips restoring owner write permission
(the hook throws EACCES) or skips the sibling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: stale lint suppressions no longer fail lint

New violations still fail because a file's count for a rule rises above the
recorded count. Fixed violations leave a stale count, which lint:prune removes;
failing on them would make every parallel pull request that deletes code
rewrite eslint-suppressions.json and conflict with the others.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* build(lint): lint:fix reuses lint's flags, so stale counts no longer fail it

`pnpm -w lint` passes --pass-on-unpruned-suppressions, so a fixed violation
whose count was not pruned is accepted. `lint:fix` repeated the file globs
without that flag, so in exactly that state it applied its fixes and then
exited 2 with "There are suppressions left that do not occur anymore".
Defining it as `pnpm -w lint --fix` keeps the two scripts from drifting.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* docs: describe the lint ratchet as it behaves when a fix is not pruned

Since lint passes --pass-on-unpruned-suppressions, a recorded count only goes
down when someone runs `pnpm -w lint:prune`. Until then the room a fixed
violation leaves lets a new violation of the same rule in the same file pass.
contributing.md, the eslint.config.js comment and the changelog said a new
violation always fails, and contributing.md promised periodic pruning on
main that nothing performs. They now state the actual rule: lint fails when
a file has more violations of a rule than its recorded count.

The changelog also claimed every CLI test deletes its temporary project. Only
the tests in cli.test.ts, lifecycle-commands.test.ts and
audit-profile-commands.test.ts do; those include every CLI test that launches
a run, so the entry now says that.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: stop the unused-exports report annotating every run as failed

The informational knip step exited 1 whenever it found anything, which is
every run until the dead-code removals land. continue-on-error kept the job
green, but each Build gates check still carried a "Process completed with
exit code 1." failure annotation. --no-exit-code (knip 6.33.0) prints the same
report (52 unused exports, 38 types, 4 duplicates on this tree) and exits 0.
continue-on-error stays so a knip crash in this step still cannot block.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* ci: install eatmydata only when the runner lacks it, and scope what it covers

The lane comment and docs/reference/development.md said eatmydata makes fsync
a no-op for the whole lane. It does so in the test processes, which is where
startRun and runCli copy and fsync the sealed snapshot, but
smithersCommandEnv rebuilds each Smithers engine child's environment from an
allowlist that has no LD_PRELOAD, so engine processes still sync.

`sudo apt-get install -y eatmydata` without `apt-get update` only worked
because the current image already ships the package. GitHub's notice on this
PR's Build gates job says ubuntu-latest migrates to Ubuntu 26 from
2026-10-19; on an image without eatmydata and without package lists, that
install would fail every lane. The step now skips apt entirely when eatmydata
is present, as it is today, and otherwise updates the index before installing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* test(ci): catch step-level pull-request gating of release validation

The guard parsed ci.yml but checked only the release-validation job's own
`if`. Adding `if: github.event_name != 'pull_request'` to the "Validate release
lane" step, the step that runs the suites, still passed it; the text guard it
replaced caught that. It also pinned the exact `if` string of the release-gates
requirement step, so an equivalent rewrite failed it.

The guard now rejects an event or pull_request condition on the job and on
every one of its steps, and checks that the requirement step tests
`needs.release-validation.result` without an event condition. Mutations run
against the old and new test: a step-level PR `if` passes the old guard and
fails the new one; a job-level PR `if` and an event-gated requirement step
fail both; wrapping the requirement condition in `${{ }}` fails only the old.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* build(lint): replace the suppression baseline with a global complexity ceiling

#1005 deliberately added no baseline or suppression registry, and #960 asks for
a complexity ceiling in the style of modem-dev/hunk#861: one global maximum set
at today's worst function, with nothing grandfathered, lowered as hotspots are
simplified. Drop eslint-suppressions.json and the always-on size budgets, keep
the diff-limited strict budgets for changed lines, and fail any function above
complexity 90 (synchronizeLinkedWorkflowRun is at 90 once the pending
simplification PRs land). The changelog entry moves to the consolidated
release notes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant