Skip to content

feat: add lint feedback (in-isolate extraStrict + external Biome CI gate)#57

Merged
jonnyparris merged 3 commits intomainfrom
feat/lint-gate
May 7, 2026
Merged

feat: add lint feedback (in-isolate extraStrict + external Biome CI gate)#57
jonnyparris merged 3 commits intomainfrom
feat/lint-gate

Conversation

@jonnyparris
Copy link
Copy Markdown
Owner

Summary

Two-layer lint feedback for Dodo. No real linter (oxlint, biome) can run inside a session DO — both ship as either Rust-native binaries or wasm bundles too large for the Workers 10 MB compressed script limit (Biome's wasm-bundler is 8 MB gzipped on its own, dodo already uses ~1.5 MB on typescript). So this PR builds the next-best thing.

Layer 1 — In-isolate: typecheck tool gains an extraStrict: true option that layers noUnusedLocals, noUnusedParameters, noImplicitReturns, and noFallthroughCasesInSwitch on top of the user's tsconfig. Reuses the already-bundled TypeScript compiler. Zero extra bundle weight. The agent gets sub-second feedback for unused-locals, missing-returns, and switch-fallthrough.

Layer 2 — External (CI): Biome runs in dodo-verify.yml for every PR and dispatched verify run. Catches unused imports + suspicious patterns + double-equals + the wider rule set. Configured in biome.jsonc with correctness + suspicious rules only — formatting is intentionally off (separate decision), style rules are off (would need weeks of cleanup to be useful as a gate).

What's in the diff

feat(typecheck): add extraStrict flag (commit 1)

  • src/typecheck.ts: new extraStrict option on TypecheckOptions and EXTRA_STRICT_OPTIONS map
  • src/agentic.ts: tool surface gains the flag, description updated
  • src/coding-agent.ts: system prompt mentions the flag and explicitly notes there's no in-isolate linter
  • scripts/smoke-typecheck.mjs: 3 new test cases covering unused-locals, implicit-returns, and tsconfig-override behaviour

feat(ci): add Biome lint gate (commit 2)

  • biome.jsonc (new): rule set scoped to correctness + suspicious. Uses Biome 2.x !! negation in files.includes (the experimentalScannerIgnores field is deprecated)
  • package.json: npm run lint and npm run lint:fix scripts; @biomejs/biome as a dev dep
  • .github/workflows/dodo-verify.yml: new Lint (Biome) step before typecheck and tests
  • AGENTS.md: new "Linting" section explaining the two layers and why
  • Six pre-existing findings cleaned up: dead patchSession() in mcp.ts, unused err binding in artifacts-read.ts, unused now in user-control.ts, three unused destructures in tests

Validation

  • npm run typecheck
  • npm run lint ✅ (clean — 108 files, 0 errors)
  • npm test ✅ (630/630 passing)
  • npm run test:typecheck-smoke ✅ (9/9 including the 3 new cases)

Why not just bundle Biome?

The @biomejs/wasm-bundler package is 34 MB raw / 8 MB gzipped. Workers' compressed script size limit is 10 MB. After the existing typescript bundle (~1.5 MB gzip) and dodo's own code, there's no room. Confirmed by inspecting the published package — there's no smaller subset distribution.

Why not oxlint?

oxlint ships as platform-specific Rust-native binaries (oxlint-darwin-arm64, etc). No wasm or pure-JS distribution exists in 2026. The @oxc-parser/wasm package is deprecated. Even the parser-only path would only give us AST without the rule engine.

Future work

If oxc ships a wasm linter, or if Biome publishes a slimmed-down lint-only wasm bundle, we can revisit and load it in-isolate — that would close the feedback gap to sub-second. For now, the CI gate is the practical path.

Layers noUnusedLocals, noUnusedParameters, noImplicitReturns, and
noFallthroughCasesInSwitch on top of the user's tsconfig when the agent
opts in. A real linter (oxlint, biome) can't run in-isolate — biome's
wasm bundle is 8 MB gzipped, well over the Workers script size budget,
and oxlint ships only Rust-native binaries. extraStrict reuses the
already-bundled TypeScript compiler to surface ~30% of what a linter
would catch at zero extra bundle cost.

The flag is opt-in per call so existing typecheck behaviour is
unchanged and the user's tsconfig still wins by default. When the
caller asks for it, the four flags override whatever tsconfig set.
External lint check, configured in biome.jsonc and run by the existing
dodo-verify workflow. Biome can't be loaded in-isolate (8 MB gzip wasm
bundle, Workers script size limit is 10 MB compressed and dodo already
spends ~1.5 MB on the bundled typescript compiler). The CI gate is the
agent's only feedback loop for unused-imports / suspicious patterns;
the in-isolate `typecheck` tool with `extraStrict: true` covers the
TypeScript-side overlap.

Scope is intentionally narrow: correctness + suspicious rules only.
Formatting is off (the project hasn't adopted a formatter and that's a
separate decision). Style rules are off too — they generate too much
noise on the existing codebase to be a useful CI gate without weeks of
cleanup. The current rule set is what biome already finds and what we
can fix in this PR.

Findings cleaned up in this commit:
- src/artifacts-read.ts: drop unused `err` binding from a routine catch
- src/mcp.ts: delete dead patchSession() function (no callers)
- src/user-control.ts: drop unused `now` binding in DELETE handler
- test/architecture-gaps.test.ts: drop redundant devUser lookup
- test/compaction-e2e.test.ts: drop unused sessionId destructure
- test/dodo.test.ts: drop dead initText that read but never parsed body

The Biome 2.x `experimentalScannerIgnores` field is deprecated; using
the `!!` negation prefix in `files.includes` instead.
Self-audit findings on this PR before merge:

1. biome.jsonc had four `"off"` rule entries (noUnusedFunctionParameters,
   useExhaustiveDependencies, noExplicitAny, noEmptyBlockStatements) that
   were dead config. With `recommended: false` on the rules block, every
   rule defaults to off — explicit "off" entries don't change behaviour
   and just add noise. Verified by removing them and re-running biome:
   identical output.
2. javascript.linter.enabled: true was the default when linter.enabled is
   true. Also dead. Removed.
3. The smoke test only covered two of the four extraStrict flags
   (noUnusedLocals, noImplicitReturns). Added cases for the other two
   (noFallthroughCasesInSwitch via TS7029 and noUnusedParameters via
   TS6133) so every flag claimed in the PR description has executable
   coverage.

No source-code or workflow changes — pure config + test cleanup.
@jonnyparris jonnyparris marked this pull request as ready for review May 7, 2026 09:01
@jonnyparris jonnyparris merged commit 7d1157e into main May 7, 2026
2 checks passed
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