Skip to content

refactor(typescript-sdk): enable + clean root-package eslint debt (~269 problems; CI does not lint root) #565

Description

@drewdrewthis

For Humans

The TypeScript lint gate has holes, but not the ones this issue was filed about — and the shipped library is already clean. PR #755 (merged 2026-07-09) added a lint:lib CI step that gates javascript/src/** excluding tests. That surface is now 0 problems. The remaining ungated debt is 230 problems, and it lives in test files (209), one example package that CI silently skips (19), and two root-level files (2).

Ask: ratify the revised acceptance criteria below (two of the original five are now wrong) before implementation starts.

  • The "~269 problems" figure re-measures to 253 today, and 0 of them are in the shipped library.
  • The "18 import/no-unresolved" are not root-package debt and not a resolver bug — they are a nested React example that lints green under its own config.
  • New: the lint gate silently depends on pnpm build having run, and pnpm -r run lint silently skips any package without a lint script.

Status: refactor, P3 - low, tech-debt, investigating. Awaiting AC ratification. Full spec below.

For Agents — full spec

Investigation

Root finding: the issue's premise was correct when filed and is now half-obsolete. The shipped library is already gated and already clean; the real remaining debt is test files, one silently-skipped example package, and two structural gaps in how the gate is composed.

Measurement environment

All numbers below were measured this turn, not inherited:

Repo state langwatch/scenario @ 88aec40 (main, 2026-07-30)
Command pnpm install --frozen-lockfilepnpm buildeslint . --format json, run in javascript/
ESLint 9.39.4 (with nested-config lookup active — it picks up examples/openai-realtime-demo/realtime-client/eslint.config.js)
Node / pnpm v24.13.0 / 10.22.0

Headline numbers (post-build)

pnpm lint (= eslint . from javascript/) → 253 problems (241 errors, 12 warnings). 106 are autofixable; 145 remain after eslint . --fix.

Coverage map — what CI actually gates today

Scope Problems Gated in CI today After --fix
src/** non-test — the shipped library 0 lint:lib, added by #755 0
src/** tests (__tests__/, *.test.ts) 209 ❌ nothing 107
Root-level files (demo-sliding-deadline.ts, scripts/generate-noise-samples.mjs) 2 ❌ nothing 1
examples/vitest 0 lint:all 0
examples/openai-realtime-demo 0 lint:all (only agents/ index.ts) 0
examples/…/realtime-client 23 under the root config / 0 under its own config lint:all (own config, exit 0)
examples/custom-observability 19 no lint script — silently skipped 14
Total ungated 230 122

Rule composition of the 209 in src/** tests: 106 @typescript-eslint/no-explicit-any, 98 import/order, 5 unused-eslint-disable directives. 105 of the 106 any are in a single file, src/agents/__tests__/red-team.test.ts.

Finding 1 — AC 1 is already half-shipped

lint:all is pnpm -r --parallel run lint. pnpm -r excludes the workspace root even though pnpm-workspace.yaml lists . in packages: — verified empirically (pnpm -r --parallel exec node -e "console.log(process.cwd())" prints only the four examples/* paths). So the issue's "lint:all is examples-only" claim is correct.

But #755 added a second CI step, lint:lib (eslint 'src/**/*.ts' --ignore-pattern '**/*.test.ts' --ignore-pattern '**/__tests__/**'), in .github/workflows/javascript-ci.yml:92-94. That step is green and the surface it covers is 0 problems. The gap AC 1 describes is now: test files, root-level files, and examples/custom-observability — not "the root package."

Finding 2 — AC 2 targets a non-problem (the 18 import/no-unresolved)

All 18 post-build import/no-unresolved errors are in examples/openai-realtime-demo/realtime-client/**.tsx@/lib/utils, @/components/ui/* (its Vite @/* path alias) and @openai/agents/realtime (its own dependency). They appear only when the root ESLint is pointed at that subtree, because the root tsconfig.json has no @/* paths and that package's deps live in its own node_modules.

That package has its own eslint.config.js and its own lint script, and pnpm lint inside it exits 0. So this is not "an eslint-import-resolver config gap" in the root package — it is a cross-package-cwd artifact of linting a nested app from the parent. The correct action is to exclude that subtree from any root-level gate (it is already covered by lint:all), not to add resolver config or suppressions.

Finding 3 — NEW: the lint gate silently depends on build state

Before pnpm build, pnpm lint:all fails — 63 import/no-unresolved errors in examples/vitest, all Unable to resolve path to module '@langwatch/scenario', because the workspace self-reference resolves through dist/. After pnpm build it is green (exit 0). Same effect on the root: eslint . is 321 problems pre-build vs 253 post-build.

CI passes only because Lint runs after Build and Smoke-load dist in javascript-ci.yml. A contributor running pnpm lint:all on a fresh clone gets 63 spurious errors, and any future reordering of CI steps turns the gate red for a reason unrelated to code quality.

Finding 4 — NEW: pnpm -r run lint silently skips packages with no lint script

examples/custom-observability has no lint script in its package.json. pnpm -r --parallel run lint skips it without erroring, so its 19 problems (8 no-explicit-any, 4 @typescript-eslint/no-unused-vars, 3 import/order, 3 unused-imports/no-unused-vars, 1 unused-imports/no-unused-imports) are invisible. This is the same invisibility failure the issue is about, one level up — and it re-opens the moment anyone adds a sixth workspace package.

examples/openai-realtime-demo's script is eslint agents/ index.ts, which happens to cover everything it owns today, but is enumerated rather than glob-complete and will drift the same way.

Finding 5 — config bug: Node globals not applied to scripts/

eslint.config.mjs applies globals.browser to **/*.{js,mjs,cjs,ts} and globals.node only to **/*.config.{js,mjs,cjs,ts} + eslint.config.mjs. So scripts/generate-noise-samples.mjs — a Node script — is linted with browser globals and reports no-undef on Buffer. This is one of only two root-level problems, and it is a real config defect, not source debt.

Challenge findings

Ran per create-issue/steps/challenge.md against the proposal as written. Three flips:

Challenge Effect on the proposal
"Just add pnpm lint to CI" is the obvious implementation of AC 1 and it is wrong — it duplicates lint:all over examples, re-imports the 18 false positives from Finding 2, and inherits the build-order coupling from Finding 3. AC 1 must specify the gate's scope, not just its existence.
AC 4 (no-explicit-any → zero) is the largest cost and the weakest value: 106 of 114 are in test files, 105 in one file. Retyping test mocks changes what the test exercises — real regression risk, near-zero user-visible benefit, and the type contract that actually ships is already at 0. Scope AC 4 to "no NEW any" / warn in tests, rather than zero.
A gate that is red on day one gets disabled. If the gate lands before the cleanup, the next contributor adds --max-warnings or deletes the step. Order matters: the gate must be green at the moment it lands.

Strategies considered

Strategy Result
Add pnpm lint (eslint .) as a CI step ❌ Rejected — 253 problems, duplicates lint:all, re-imports the 18 false positives, build-order coupled
Widen lint:lib to include src/** tests ⚠️ Viable but 209 problems must clear first; 98 clear with --fix
Per-package gate + root gate scoped to root-owned files (src/** incl. tests, root-level *.ts/scripts/), excluding examples/ ✅ Recommended — no duplication, no cross-package false positives
Fix resolver config for the 18 ❌ Rejected — not a resolver bug (Finding 2)
Add a workspace check that fails when a package has no lint script ✅ Recommended — closes Finding 4 permanently

Findings for the implementer

  • Do not measure with eslint . pre-build. Always pnpm build first, or the numbers are inflated by 68.
  • --fix clears 106 of 253: import/order goes 102 → 1, and the 5 unused-eslint-disable directives go to 0. Everything left is no-explicit-any (114), import/no-unresolved (18, all Finding-2 false positives), unused-vars (7), react-hooks (4), no-undef (1).
  • red-team.test.ts alone is 105 of the 106 test-file any. Treat it as its own unit of work; do not bundle it with the gate change.
  • Ruled out: the import/no-unresolved errors are not caused by eslint-import-resolver-typescript misconfiguration. tsc --noEmit resolving fine is consistent with Finding 2 (different cwd, different tsconfig), not with a resolver gap.

Caveats

  • Nested-config lookup behaviour is ESLint-version-dependent (measured on 9.39.4). If the pinned ESLint changes, re-measure before trusting the examples/ column.
  • examples/openai-realtime-demo has pnpm.overrides in a non-root package.json; pnpm warns it has no effect. Unrelated to lint, but it surfaces on every -r run and will show up in any new gate's logs.

Proposed AC changes

Ratified under dec.2026-08-01-scenario-565-lint-ac-set and graded by ac-reviewer, which returned 7 Must-Fix against the first draft; all are folded in below.

# Original AC Disposition Basis
1 Root-package lint runs in CI Rewritten to name the scope #755 already gates src/** non-test (0 problems). The residual is tests + root-level files + one skipped example.
2 Fix the resolver so the 18 import/no-unresolved clear DROPPED Measured: all 18 are in examples/…/realtime-client/**.tsx (its own @/* Vite alias) and that package exits 0 under its own config. Not root-package debt, not a resolver bug.
3 import/order auto-fixed Kept, sharpened 98 in src/** tests + 1 root-level.
4 no-explicit-any driven to zero Scoped to a severity policy + ratchet 106 of 114 are tests, 105 in one file, and they are (agent as any).privateMember. Typing them away means widening the shipped API.
5 Lint green and stays enforced Split into AC9 + AC11 "Green" is meaningless on a draft PR whose ci-checks job was skipped.
Added AC5–AC8, AC10, AC11 The two new findings, plus the regression/ripple/escape-hatch surface.

Acceptance Criteria

  • AC1 — The gate covers every root-owned file, and each sub-scope is proven gated. One CI step runs pnpm lint:root, covering src/**/*.ts (including **/*.test.ts and **/__tests__/**), root-level *.ts, and scripts/**, and excluding examples/**. Evidence, both: (a) pnpm lint:root --format json | jq -r '[.[].filePath] | .[]' lists at least one path from each of src/ non-test, src/**/__tests__/, the root-level *.ts set, and scripts/, and zero paths under examples/; (b) three injected defects, each reverted — an import/order swap in red-team.test.ts, an unused variable in demo-sliding-deadline.ts, and an undefined global in scripts/generate-noise-samples.mjs — each turning the step red on its own.
  • AC2 — import/order is clean across the newly-gated surface. pnpm lint:root --format json | jq '[.[].messages[] | select(.ruleId=="import/order")] | length' prints 0 (baseline: 98 in src/** tests, 1 in demo-sliding-deadline.ts). --fix resolves 99; the residual one in src/voice/__tests__/playback.test.ts is fixed by moving the import, not by suppressing the rule.
  • AC3 — no-explicit-any is error in src/** non-test and ratcheted, not unbounded, in tests. The rule stays error for src/**/*.ts excluding tests (0 today). In tests it is warn under a hard ceiling: lint:root runs --max-warnings=106, so a 107th warning fails CI. This is the one --max-warnings use AC9 exempts. Evidence, both: (a) red on an any added to a non-test src/ file; (b) red on a 107th any in red-team.test.ts. Explicitly not required: driving the existing 106 to zero.
  • AC4 — Linting without a build reports the real cause instead of 63 resolver errors. Reframed from "the gate must not depend on build state": the examples consume @langwatch/scenario through its published exports map into dist/, so needing a build is legitimate consumer behaviour — the defect was that its absence produced 63 misleading import/no-unresolved errors. Evidence: with dist/ removed, pnpm lint:all exits non-zero, names javascript/dist as missing, tells the reader to run pnpm build, and emits no @langwatch/scenario resolver errors.
  • AC5 — No workspace package can escape the gate, including one added tomorrow. A guard enumerates packages at runtime from pnpm list -r --depth -1 --json, never a hardcoded list, and fails naming the package when any lacks a lint script or carries an enumerated one (eslint agents/ index.ts) that a new sibling file would slip past. Evidence, three mutations: (a) red when lint is deleted from examples/vitest; (b) red when a new package with no lint script is added; (c) red on an enumerated lint command — each green again on revert.
  • AC6 — examples/custom-observability is gated, clean, and its cleanup provably did not gut a probe. It gains "lint": "eslint ." and reports 0 problems (baseline 19). Because nothing in CI executes this package — no lint, no typecheck, no test invocation — the cleanup is proven by running it. Evidence, three: (a) pnpm -F custom-observability-example lint exits 0 with zero messages; (b) pnpm -F custom-observability-example test:all exits 0 with its PASS lines quoted; (c) test-no-auto-init.ts still performs await import("@langwatch/scenario") for its side effect — the unused binding was dropped, the import was not.
  • AC7 — scripts/** is linted with Node globals. scripts/generate-noise-samples.mjs reports 0 problems (baseline: 1 × no-undef on Buffer) with no inline suppression. Proven in-scope by AC1(b)'s third mutation, not merely by a local run.

Consequence & failure-mode coverage

  • AC8 — lint:lib and lint:all still cover at least what they covered before. Exit 0 alone would also pass for a gutted script, so the criterion is set-based: each command's linted-file set on this branch is a superset of its set on main, and both exit 0.
  • AC9 — The gate is green on a run that actually executed it, with no escape hatch added. ⚠ A draft PR yields a green javascript-ci with the lint steps never runci-checks is gated on draft == false (javascript-ci.yml:47), there is no ready_for_review trigger, and javascript-complete (:127) reports success when an upstream job is skipped. So the evidence is the ci-checks job's own conclusion, never the aggregate. Plus: no added line introduces eslint-disable, --quiet, continue-on-error, or no-error-on-unmatched-pattern, and no --max-warnings beyond the AC3 ratchet. Added lines only — --fix removes 5 pre-existing eslint-disable directives that an unqualified diff grep would falsely flag.
  • AC10 — The import reorder changed no runtime behaviour. A differential, not merely "green": pnpm test before and after the autofix reports identical passed/failed/skipped counts, and pnpm typecheck:all exits 0.
  • AC11 — The baseline was cleared by fixing code, not by disabling rules or widening ignores. Every bare "reports 0 problems" count above is also satisfiable by import/order: "off", a new ignores: entry, or an --ignore-pattern. Evidence, both: (a) the eslint.config.mjs diff shows no rule moved from error to warn/off and no new top-level ignores: entry, with the single AC3-sanctioned exception; (b) lint:root's linted-file count is the count on main — the file count must not shrink while the problem count falls.

Plan

Landed as two commits on issue565/enable-clean-root-package-eslint, split so the 40-file mechanical reorder is reviewable apart from the design change.

  1. Commit 1 — the autofix. eslint . --ignore-pattern 'examples/**' --fix over src/** and demo-sliding-deadline.ts. 43 files, import statements only (plus one obsolete comment block in playback.test.ts whose subject — an import placed below vitest — no longer exists). Clears 99 import/order and 5 unused eslint-disable directives.
  2. Commit 2 — the gate.
    • package.json: new lint:root = eslint . --ignore-pattern 'examples/**' --max-warnings=106; lint:all gains the precondition guard.
    • eslint.config.mjs: globals.node extended to scripts/**; new warn-severity no-explicit-any block scoped to src/** tests.
    • scripts/check-lint-coverage.mjs: runtime workspace enumeration; fails on a missing lint script, an enumerated lint script, or a missing dist/.
    • javascript-ci.yml: new Lint (root package) step; the existing two relabelled and re-commented.
    • examples/custom-observability: gains "lint": "eslint ."; 19 problems fixed by typing (AgentInput, a ScopedSpan cross-version type) rather than suppression — the langwatch: "disabled" as any casts turned out to typecheck without the cast.
    • examples/openai-realtime-demo: lint moved from the enumerated eslint agents/ index.ts to eslint . --ignore-pattern 'realtime-client/**'.
    • specs/typescript-lint-gate-coverage.feature: the AC coverage map.

Deliberately out of scope

Two findings belong to the repo, not to #565, and are named here rather than silently absorbed:

  • The draft-gate hole is repo-wide. ci-checks skipping on draft + javascript-complete treating skipped as success + no ready_for_review trigger means every required JS check is bypassable by opening as draft and marking ready without a push. AC9 works around it; it does not fix it.
  • examples/custom-observability is executed by nothing in CI. AC6 gates its lint. Its typecheck and test gaps remain — it has no typecheck script, and the workflow runs only pnpm -F vitest-examples test.

Evidence

Raw measurement artifacts for this investigation (this box, session-local): /tmp/claude-1000/565-lint-full.json (pre-build, 321), /tmp/claude-1000/565-lint-full-postbuild.json (post-build, 253), /tmp/claude-1000/565-lint-postfix.json (post---fix, 145). All reproducible from 88aec40 with the commands in "Measurement environment".

Metadata

Metadata

Assignees

No one assigned

    Labels

    P3 - lowLow priority, nice to havehuman-managedOwner drives this personally; agents must not pick it up or push to itinvestigatingWorkflow: investigatingjavascriptPull requests that update javascript coderefactorCode restructuring, no behavior changetech-debt

    Type

    No type

    Projects

    Status
    Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions