refactor(lint): gate every root-owned TypeScript file and close two silent-skip holes (#565) by drewdrewthis · Pull Request #865 · langwatch/scenario

drewdrewthis · 2026-08-01T08:18:12Z

For Humans

The lint gate had holes — but not the ones #565 was filed about, and the shipped library was already clean. PR #755 (merged 2026-07-09) added a lint:lib step that gates javascript/src/** excluding tests; that surface measures 0 problems today. The real ungated debt was 230: test files (209), one example package CI silently skipped (19), and two root-level files (2).

What this PR does: widens the gate to every root-owned file, closes two silent-skip holes, and clears the debt by fixing code — not by turning rules off.

Two of the issue's five original ACs described the wrong surface and were revised (dec.2026-08-01-scenario-565-lint-ac-set); ac-reviewer then returned 7 Must-Fix against my first draft and all are folded in. Details in #565.

For Agents — full detail

The real numbers

The issue estimated ~269 problems. Re-measured at 88aec40, after pnpm install --frozen-lockfile and pnpm build (this matters — see below), eslint 9.39.4:

pnpm lint = 253 problems (241 errors, 12 warnings); 106 autofixable; 145 survive --fix.

Scope	Problems	Gated before this PR	Now
`src/**` non-test — the shipped library	0	✅ `lint:lib` (#755)	✅ unchanged
`src/**` tests	209	❌ nothing	✅ `lint:root`
Root-level (`demo-sliding-deadline.ts`, `scripts/`)	2	❌ nothing	✅ `lint:root`
`examples/custom-observability`	19	❌ no `lint` script — skipped silently	✅ `lint:all`
`examples/vitest`, `examples/openai-realtime-demo`	0	✅ `lint:all`	✅ unchanged
`examples/…/realtime-client`	0 under its own config	✅ `lint:all`	✅ unchanged

Pre-build the same command reports 321, not 253. That 68-problem gap is the subject of AC4.

Two AC corrections worth reading

The 18 import/no-unresolved are not root-package debt. Every one is in examples/openai-realtime-demo/realtime-client/**.tsx — its @/* Vite alias and its own dependencies. They appear only when the root ESLint is pointed at that subtree. That package has its own eslint.config.js and pnpm lint inside it exits 0. There was nothing to fix; lint:root excludes examples/** instead.

no-explicit-any is not driven to zero. 114 total: 0 in the shipped library, 8 in an example, 106 in tests — 105 of them in red-team.test.ts, shaped (agent as any).turnScores.set(...) and (agent as any).getAttackPlan = vi.fn(). That is deliberate private-member access from tests. Removing it means widening the production API or mirroring its privates across ~100 sites. Instead: error on the library (0 today, stays 0), warn in tests behind a --max-warnings=106 ratchet so a 107th fails CI. The count is capped, not ignored.

Two structural findings the issue did not have

1. pnpm -r run lint skips a package with no lint script, without erroring. That is how examples/custom-observability accumulated 19 invisible problems. scripts/check-lint-coverage.mjs now enumerates workspace packages at runtime from pnpm list -r --depth -1 --json — never a hardcoded list — so a package added tomorrow cannot escape either. It also rejects enumerated lint scripts (eslint agents/ index.ts lints only what someone remembered that day); examples/openai-realtime-demo moved to eslint . --ignore-pattern 'realtime-client/**'.

2. Linting before a build emitted 63 misleading errors. The examples import @langwatch/scenario through its published exports map, which points at dist/. On a fresh clone pnpm lint:all failed with 63 Unable to resolve path to module '@langwatch/scenario'. CI only passed because Lint runs after Build. The dist/ dependency is legitimate — the examples exercise the real consumer path — so this PR does not remove it; it makes the failure honest. The guard now names dist/ and says to run pnpm build.

Also fixed: globals.node was scoped to *.config.* only, so scripts/ was linted with browser globals and reported no-undef on Buffer.

How I can prove I was successful

Falsifiability battery — 16/16. Each gate goes red on its own injected defect and green again on revert. Script: /tmp/claude-1000/565-discrimination.sh (session-local; every mutation is reproducible from the descriptions below).

=== BASELINE (no mutation) — every gate must be GREEN ===
  PASS  lint:root baseline green (exit=0)
  PASS  lint:lib  baseline green (exit=0)
  PASS  lint:all  baseline green (exit=0)
=== M1: 107th 'any' in a test file — the --max-warnings ratchet must fire ===
  PASS  lint:root RED on 107th test 'any' (exit=1)
=== M2: new 'any' in NON-test src — must be an ERROR, not a warning ===
  PASS  lint:root RED on non-test 'any' (exit=1)
  PASS  lint:lib  RED on non-test 'any' (exit=1)
=== M3: import/order violation in a TEST file (was ungated before this PR) ===
  PASS  lint:root RED on test-file import/order (exit=1)
=== M4: unused variable in a ROOT-LEVEL file (was ungated before this PR) ===
  PASS  lint:root RED on root-level unused var (exit=1)
=== M5: undefined global in scripts/ — proves scripts/ is IN the gate's scope ===
  PASS  lint:root RED on scripts/ no-undef (exit=1)
=== M6: remove a package's lint script — silent-skip guard must fire ===
  PASS  lint:all RED when a lint script is removed (exit=1)
=== M7: ADD a new workspace package with no lint script (the stated failure) ===
  PASS  lint:all RED when a NEW package has no lint script (exit=1)
=== M8: enumerated lint script — drift guard must fire ===
  PASS  lint:all RED on an enumerated lint script (exit=1)
=== M9: missing dist/ — build-precondition guard must fire with a real message ===
  PASS  lint:all RED when dist/ is absent (exit=1)
  PASS  message names the real cause (not 63 resolver errors)
        import/no-unresolved lines emitted: 1 (was 63 before this PR)
=== FINAL: tree restored, gates green again ===
  PASS  lint:root green after revert (exit=0)
  PASS  lint:all  green after revert (exit=0)
RESULT: 16 passed, 0 failed

The battery caught one flaw in itself on an earlier run: the unused-variable probe was named __probeUnused, which matches the config's varsIgnorePattern: ^_, so M4 passed a mutation the gate correctly ignores. Renaming the probe made it discriminate. Worth stating, because a battery that never fails is not evidence.

AC1 — the gate's scope, not just its exit code (eslint . --ignore-pattern 'examples/**' --format json):

files linted: 232        (231 before this PR; the +1 is check-lint-coverage.mjs itself)
  src/ non-test                 125  e.g. src/agents/claude-code/claude-code-agent.adapter.ts
  src/ tests                    101  e.g. src/agents/__tests__/claude-code-adapter.test.ts
  root-level *.ts                 4  e.g. demo-sliding-deadline.ts
  scripts/                        2  e.g. scripts/check-lint-coverage.mjs
  examples/ (must be 0)           0  NONE

The file count rose, so the problem count did not fall by narrowing what gets linted (AC11).

AC6 — custom-observability's probes still work. Nothing in CI executes this package, so lint-clean is not enough; I ran it:

Provider before import: ProxyTracerProvider
Provider after import:  ProxyTracerProvider
PASS: Importing @langwatch/scenario did NOT auto-initialize OpenTelemetry
PASS: Scenario runs correctly with custom observability config
PASS: Both scenario and custom-scoped spans are captured
✅ PASS: scenarioOnly filtering works via scenario.config.mjs

The lint error there was const scenario = await import(...) — "assigned but never used". Deleting the line would have satisfied the linter and destroyed the probe. The binding was dropped; the side-effect import was kept.

AC10 — the 43-file import reorder changed nothing. Identical before and after, not merely "green":

Test Files  92 passed | 1 skipped (93)
     Tests  1079 passed | 4 skipped (1083)
typecheck:all=0

AC8 — no coverage was lost. lint:lib's command is byte-identical in the diff. lint:all gained only a precondition prefix; the pnpm -r --parallel run lint half is unchanged, and both example changes are strictly widening (one package gained a lint script, one moved from an enumerated list to a glob). The superset property follows from the diff.

AC9 — the gate passed in real CI, on a run that actually executed it.

ci-checks (24.x)   conclusion=success  status=completed  6m38s
  Lint (workspace packages)  success
  Lint (library)             success
  Lint (root package)        success

https://github.com/langwatch/scenario/actions/runs/30691499000/job/91346933892

That is the ci-checks job's own conclusion, not javascript-complete's — the latter reports success when an upstream job is skipped, which is exactly what a draft PR produces.

Deliberately out of scope

The draft-gate hole is repo-wide — filed as fix(ci): draft PRs bypass every required JavaScript check, and leaving draft never re-triggers them #866. ci-checks is gated on draft == false (javascript-ci.yml:47), there is no ready_for_review trigger, and javascript-complete (:127) succeeds when an upstream job is skipped — so a draft PR yields a green javascript-ci with lint never run. This PR is deliberately not a draft so its own gate actually executes. Read the ci-checks job conclusion, not the aggregate.
examples/custom-observability is executed by nothing in CI — filed as test(ci): examples/custom-observability is type-checked and tested by nothing in CI #867. — no lint (fixed here), no typecheck script, and the workflow runs only pnpm -F vitest-examples test. The typecheck and test gaps remain.

Review notes

Commit 1 is the mechanical --fix (43 files, imports only + one obsolete comment). Commit 2 is the design change. Review commit 2.
Nothing was cleared by disabling a rule or widening an ignore. The only severity change is the AC3-sanctioned no-explicit-any: warn block scoped to src/**/*.test.ts + src/**/__tests__/**.
--fix removed 5 pre-existing eslint-disable directives (unused ones). A naive git diff | grep eslint-disable will match those deletions — check added lines only.

Closes #565

Mechanical `eslint --fix` output over the surface that PR #755's lint:lib step does not reach: src/** test files and the root-level demo. 99 of the 100 import/order violations were autofixed; the one in playback.test.ts was moved by hand because its builtin import sat below the vitest import behind a comment block explaining a hoisting concern that vi.hoisted() already handles — the comment went with it. No non-import changes. Verified byte-identical test results before and after: 92 files / 1079 passed / 4 skipped, and typecheck:all exits 0. Refs #565

Measured at 88aec40 (post `pnpm install --frozen-lockfile` + `pnpm build`, eslint 9.39.4): `eslint .` in javascript/ reports 253 problems, not the ~269 the issue estimated, and 0 of them are in the shipped library — PR #755's lint:lib step already gates src/ non-test and it is clean. The real ungated debt was 230: 209 in src/** tests, 19 in an example package CI never linted, and 2 root-level. Two of the issue's original five ACs described the wrong surface and were revised under dec.2026-08-01-scenario-565-lint-ac-set: - The 18 import/no-unresolved are NOT root-package debt. Every one is in examples/openai-realtime-demo/realtime-client, from its own `@/*` Vite alias, and that package exits 0 under its own eslint config. Nothing to fix; the root gate excludes examples/ instead. - no-explicit-any is not driven to zero. 106 of the 114 are in tests and 105 of those are `(agent as any).privateMember` in one file; typing them away would mean widening the production API to satisfy a linter. It stays `error` on the library (0 today) and drops to `warn` in tests behind a --max-warnings=106 ratchet, so the count cannot grow silently. New lint:root step covers src/** INCLUDING tests, root-level *.ts and scripts/, excluding examples/. New check-lint-coverage.mjs runs before lint:all and turns two previously-silent failures loud: - `pnpm -r run lint` skips a package with no lint script without erroring. examples/custom-observability sat that way with 19 problems; it now has a lint script and reports 0. The guard enumerates packages at runtime from `pnpm list -r`, so a package added tomorrow cannot escape either. - Linting before a build emitted 63 import/no-unresolved errors, because the examples consume @langwatch/scenario through its exports map into dist/. That dependency is legitimate; the misleading output was not. The guard now names dist/ and says to run `pnpm build`. Also fixes a config defect: globals.node was scoped to *.config.* only, so scripts/ was linted with browser globals and reported no-undef on Buffer. All 16 mutations in the falsifiability battery behave correctly — each gate goes red on its own injected defect and green again on revert. Nothing was cleared by disabling a rule or widening an ignore. Closes #565

coderabbitai · 2026-08-01T08:18:55Z

Walkthrough

The JavaScript workspace adds root lint coverage, validates package lint scripts and build prerequisites, updates CI lint steps, and cleans import ordering and TypeScript lint findings across tests and examples.

Changes

TypeScript lint coverage

Layer / File(s)	Summary
Lint gate implementation `.github/workflows/javascript-ci.yml`, `javascript/package.json`, `javascript/eslint.config.mjs`, `javascript/scripts/check-lint-preconditions.mjs`, `javascript/eslint-suppressions.json`, `specs/typescript-lint-gate-coverage.feature`	Root lint runs separately from workspace lint. Preconditions validate build output, workspace packages, and complete lint scripts. ESLint scopes and suppression baselines are updated.
Example package lint cleanup `javascript/examples/custom-observability/*`, `javascript/examples/openai-realtime-demo/package.json`	Example lint scripts are added or expanded. Custom observability examples share `getScopeName` and use typed callback inputs.
Import and test lint cleanup `javascript/demo-sliding-deadline.ts`, `javascript/src/*/__tests__/`	Imports are reordered, duplicate imports are removed, and obsolete console suppressions are deleted without changing test behavior.

Possibly related issues

#565 — Covers the root ESLint gate, workspace preconditions, lint cleanup, and custom-observability validation.
#6167 — Strengthens lint verification with root/workspace coverage, suppression ratcheting, and CI integration.

Suggested labels: grinding, hound-checked, review: deep

Suggested reviewers: rogeriochaves

Poem

I hop through scripts where lint rules gleam,
I tidy imports in a TypeScript stream.
The root gate checks each package with care,
While scopes and suppressions stay fair.
CI thumps its paws: all checks are green!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: expanded root-owned TypeScript lint coverage and closure of two silent-skip paths.
Description check	✅ Passed	The description directly explains the lint coverage expansion, silent-skip fixes, validation, and related implementation details.
Docstring Coverage	✅ Passed	Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch issue565/enable-clean-root-package-eslint

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

javascript/examples/custom-observability/test-config-file.ts (1)
22-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated ScopedSpan/getScopeName helper across three example files. All three files define the identical type and function to resolve the OTel instrumentation-scope name across SDK v1/v2 shapes. The shared root cause is the missing shared helper module; a future OTel API change requires editing all three files in lockstep.

javascript/examples/custom-observability/test-config-file.ts#L22-L42: import ScopedSpan/getScopeName from a new shared module instead of redefining them.

javascript/examples/custom-observability/test-custom-scopes.ts#L16-L38: import ScopedSpan/getScopeName from the same shared module instead of redefining them.

javascript/examples/custom-observability/test-scenario-only.ts#L14-L38: import ScopedSpan/getScopeName from the same shared module instead of redefining them.
♻️ Proposed shared helper
// javascript/examples/custom-observability/otel-scope.ts
import type { ReadableSpan } from "`@opentelemetry/sdk-trace-base`";

/** The two shapes OTel has used for the instrumentation scope, across SDK majors. */
export type ScopedSpan = ReadableSpan & {
  instrumentationScope?: { name?: string };
  instrumentationLibrary?: { name?: string };
};

/**
 * Returns the instrumentation scope name for a span, handling both
 * OTel SDK v1 (instrumentationLibrary) and v2 (instrumentationScope).
 */
export function getScopeName(span: ReadableSpan): string {
  const s = span as ScopedSpan;
  return (
    s.instrumentationScope?.name ?? s.instrumentationLibrary?.name ?? "unknown"
  );
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@javascript/examples/custom-observability/test-config-file.ts` around lines 22
- 42, Create the shared otel-scope.ts helper containing ScopedSpan and
getScopeName, then remove the duplicate definitions and import both symbols in
javascript/examples/custom-observability/test-config-file.ts#L22-L42,
javascript/examples/custom-observability/test-custom-scopes.ts#L16-L38, and
javascript/examples/custom-observability/test-scenario-only.ts#L14-L38. Preserve
the existing OTel v1/v2 scope-name fallback behavior in getScopeName.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@javascript/examples/custom-observability/test-config-file.ts`:
- Around line 22-42: Create the shared otel-scope.ts helper containing
ScopedSpan and getScopeName, then remove the duplicate definitions and import
both symbols in
javascript/examples/custom-observability/test-config-file.ts#L22-L42,
javascript/examples/custom-observability/test-custom-scopes.ts#L16-L38, and
javascript/examples/custom-observability/test-scenario-only.ts#L14-L38. Preserve
the existing OTel v1/v2 scope-name fallback behavior in getScopeName.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 37c36f5a-cabb-4ae6-bad7-aeeb419b2899

📥 Commits

Reviewing files that changed from the base of the PR and between 88aec40 and dcf5dd7.

📒 Files selected for processing (54)

.github/workflows/javascript-ci.yml
javascript/demo-sliding-deadline.ts
javascript/eslint.config.mjs
javascript/examples/custom-observability/package.json
javascript/examples/custom-observability/test-config-file.ts
javascript/examples/custom-observability/test-custom-scopes.ts
javascript/examples/custom-observability/test-no-auto-init.ts
javascript/examples/custom-observability/test-scenario-only.ts
javascript/examples/openai-realtime-demo/package.json
javascript/package.json
javascript/scripts/check-lint-coverage.mjs
javascript/src/agents/__tests__/realtime-adapter-frames.test.ts
javascript/src/agents/__tests__/realtime-adapter-key-resolution.test.ts
javascript/src/agents/__tests__/realtime-echo-noleak.test.ts
javascript/src/agents/__tests__/realtime-echo-safe.test.ts
javascript/src/agents/__tests__/realtime-response-formatter.test.ts
javascript/src/agents/__tests__/realtime-response-guard.test.ts
javascript/src/agents/__tests__/red-team.test.ts
javascript/src/agents/__tests__/user-simulator-tts.test.ts
javascript/src/agents/__tests__/user-simulator-voice.test.ts
javascript/src/agents/__tests__/voice-assistant-role.test.ts
javascript/src/agents/judge/__tests__/judge-agent.test.ts
javascript/src/agents/judge/__tests__/judge-span-collector.test.ts
javascript/src/agents/judge/__tests__/judge-span-digest-formatter.test.ts
javascript/src/agents/judge/__tests__/judge-utils.test.ts
javascript/src/execution/__tests__/proceed-interruptions.test.ts
javascript/src/execution/__tests__/realtime-user-proceed-guard.test.ts
javascript/src/execution/__tests__/scenario-execution-inline-criteria.test.ts
javascript/src/execution/__tests__/scenario-role-attributes.test.ts
javascript/src/execution/__tests__/scenario-scope-attribute.test.ts
javascript/src/execution/__tests__/user-explicit-content-voice.test.ts
javascript/src/execution/__tests__/voice-agent-transcript.test.ts
javascript/src/runner/__tests__/run.test.ts
javascript/src/script/__tests__/interrupt-after-and-user-overrides.test.ts
javascript/src/voice/__tests__/adapter-lifecycle.test.ts
javascript/src/voice/__tests__/connected-state.test.ts
javascript/src/voice/__tests__/duration-fidelity.test.ts
javascript/src/voice/__tests__/factories.test.ts
javascript/src/voice/__tests__/fixtures/fake-adapter.ts
javascript/src/voice/__tests__/hooks.test.ts
javascript/src/voice/__tests__/judge-stt.test.ts
javascript/src/voice/__tests__/playback.test.ts
javascript/src/voice/__tests__/proceed-interrupt-errors.test.ts
javascript/src/voice/__tests__/proceed-interrupt.test.ts
javascript/src/voice/__tests__/result-audio.test.ts
javascript/src/voice/__tests__/transcribe-stale-transcript.test.ts
javascript/src/voice/__tests__/transcribe.test.ts
javascript/src/voice/__tests__/vad-fallback.test.ts
javascript/src/voice/adapters/__tests__/elevenlabs-agent-hangup.test.ts
javascript/src/voice/adapters/__tests__/elevenlabs.test.ts
javascript/src/voice/adapters/__tests__/gemini-live-spans.test.ts
javascript/src/voice/adapters/__tests__/openai-realtime-response-guard.test.ts
javascript/src/voice/adapters/__tests__/openai-realtime-user-call-guard.test.ts
specs/typescript-lint-gate-coverage.feature

@e2e

…ssions Review found the ratchet did not do what its own docs claimed. `--max-warnings=106` is a TOTAL budget across every warn-level rule in the scope, not a no-explicit-any ceiling. Reproduced: adding one unused variable to demo-sliding-deadline.ts — not a test, not an `any` — took the count to 110 and failed with "ESLint found too many warnings (maximum: 106)". Two defects follow. The budget is fungible (delete 5 test `any`, add 5 unused vars, still green), and an unrelated violation fails CI with a message pointing at a comment about test `any`. ESLint 9.39 ships the right mechanism and I had hand-rolled a worse one. Verified `--suppress-rule` / `--suppressions-location` / `--prune-suppressions` exist in the pinned version, then switched to it: - no-explicit-any goes back to `error` EVERYWHERE, including tests. The severity carve-out is gone. - The 106 known violations are recorded per-file, per-rule in a committed eslint-suppressions.json (3 files). A new `any` fails pointing at the new `any`; an unrelated warn-level violation no longer consumes the allowance. - `--prune-suppressions` ratchets the baseline down as debt is paid, and because the file is committed, any widening is visible in review. Also from review: - check-lint-coverage.mjs -> check-lint-preconditions.mjs (the name now matches what the file says it does), with the decision half extracted as a pure exported function so the rules are testable without spawning a process. - The `/^eslint \./` detector was unsound both ways: it PASSED `eslint . || true` and `eslint . --ignore-pattern '**/*'` (each fully neutering the gate it was meant to prove) and REJECTED the stricter `eslint --max-warnings=0 .`. Now it parses positional arguments and rejects shell operators outright. - The guard could pass vacuously: an empty package list from `pnpm list` meant the loop never ran and it printed OK. Reproducing the exact failure class the file exists to remove. Now a hard error. - execFileSync/JSON.parse had no error handling, so a pnpm failure surfaced as a raw stack rather than the actionable message everything else in the file is careful to produce. - The dist/ check now looks for dist/index.js — a partial or interrupted build leaves the directory present and still produces the resolver noise. - Dropped a `dec.2026-08-01-...` reference from eslint.config.mjs. That ID resolves only in a personal agent codex and was unresolvable to any other contributor. - `--fix` had stripped five unused `eslint-disable no-console` directives and left five whitespace-only lines behind — a non-import change sitting in the commit whose message says there are none. Removed. - Feature file: fixed tag granularity (@Unit for the diff-only scenario, @e2e for the CI-run one), replaced a Background data table no other spec in the corpus uses, and replaced hardcoded pass counts with the relative form the same file already uses elsewhere. - Deduplicated the ScopedSpan OTel-version shim, which was copied verbatim into three probes, into examples/custom-observability/scope-name.ts. - Matched the sibling script's conventions: no shebang, named node:path imports. Falsifiability battery re-run against the new design: 17/17, now including an explicit non-fungibility assertion (an unrelated warn-level violation must leave lint:root GREEN). Suite unchanged at 92 files / 1079 passed / 4 skipped; typecheck:all, lint:root, lint:lib, lint:all all exit 0. Deliberately NOT done here: adding a typecheck script to examples/custom-observability. `typecheck:all` skips it by the same silent-skip mechanism, but adding it surfaces three pre-existing OTel v1/v2 type errors that are out of scope and would land this PR red. Tracked with the exact errors in #867; REQUIRED_SCRIPTS in the guard carries a pointer. Refs #565

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

javascript/scripts/check-lint-preconditions.mjs (1)
71-74: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

execFileSync("pnpm", …) fails on Windows.

execFileSync without shell cannot resolve pnpm.cmd. Windows contributors get ENOENT, and the wrapper reports it as a broken pnpm enumeration. If Windows development is supported, set shell on that platform.
     raw = execFileSync("pnpm", ["list", "-r", "--depth", "-1", "--json"], {
       cwd: packageRoot,
       encoding: "utf8",
+      shell: process.platform === "win32",
     });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@javascript/scripts/check-lint-preconditions.mjs` around lines 71 - 74, Update
the execFileSync call used for pnpm enumeration in the lint precondition script
to enable shell resolution on Windows, allowing pnpm.cmd to be found while
preserving the existing packageRoot, arguments, and UTF-8 output behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@javascript/scripts/check-lint-preconditions.mjs`:
- Around line 56-62: Extend the value-taking flag allowlist in the token loop
before the positional check, updating the condition associated with the visible
token handling so separate values for ESLint flags such as --ignore-path,
--output-file, --max-warnings, --parser, --plugin, --global, --cache-location,
and --report-unused-disable-directives-severity are consumed. Preserve
positional detection for the package-root argument and ensure valid commands
like eslint . --max-warnings 0 do not treat 0 as an additional positional.

---

Nitpick comments:
In `@javascript/scripts/check-lint-preconditions.mjs`:
- Around line 71-74: Update the execFileSync call used for pnpm enumeration in
the lint precondition script to enable shell resolution on Windows, allowing
pnpm.cmd to be found while preserving the existing packageRoot, arguments, and
UTF-8 output behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2b926f18-0799-48f9-b49f-036303c51c76

📥 Commits

Reviewing files that changed from the base of the PR and between dcf5dd7 and c4b0b4d.

📒 Files selected for processing (11)

.github/workflows/javascript-ci.yml
javascript/eslint-suppressions.json
javascript/eslint.config.mjs
javascript/examples/custom-observability/scope-name.ts
javascript/examples/custom-observability/test-config-file.ts
javascript/examples/custom-observability/test-custom-scopes.ts
javascript/examples/custom-observability/test-scenario-only.ts
javascript/package.json
javascript/scripts/check-lint-preconditions.mjs
javascript/src/agents/__tests__/realtime-echo-safe.test.ts
specs/typescript-lint-gate-coverage.feature

💤 Files with no reviewable changes (2)

javascript/src/agents/tests/realtime-echo-safe.test.ts
javascript/eslint.config.mjs

🚧 Files skipped from review as they are similar to previous changes (4)

specs/typescript-lint-gate-coverage.feature
.github/workflows/javascript-ci.yml
javascript/examples/custom-observability/test-custom-scopes.ts
javascript/examples/custom-observability/test-config-file.ts

coderabbitai · 2026-08-01T08:39:43Z

+    if (token.startsWith("-")) {
+      // A value-taking flag written as two tokens consumes the next one.
+      if (!token.includes("=") && /^--(ignore-pattern|format|config|rulesdir|ext|resolve-plugins-relative-to|suppressions-location)$/.test(token)) i++;
+      continue;
+    }
+    positionals.push(token.replace(/^['"]|['"]$/g, ""));
+  }


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

ESLint 9 CLI options that take a separate value argument list

💡 Result:

In ESLint 9, several CLI options are designed to accept multiple values [1][2]. These options allow you to provide these values by either repeating the flag multiple times or by providing a comma-delimited list [3][2]. Common CLI options that support this behavior include: --ext: Specifies additional file extensions to lint [1]. --global: Defines global variables [1][2]. --ignore-pattern: Specifies patterns of files to ignore [1]. --plugin: Specifies plugins to be used [4]. --rule: Specifies rules to be used [4][3]. For example, to define multiple global variables, you can use either of the following approaches [2]: npx eslint --global describe --global it tests/ npx eslint --global describe,it tests/ Note that while most options supporting multiple values accept both methods, there have been historical exceptions; for example, --ignore-pattern has previously had limitations regarding the comma-delimited format [3]. Always refer to the output of eslint --help for the most current information regarding specific flag behavior in your installed version [3].

Citations:

1: https://eslint.org/docs/latest/use/command-line-interface

2: eslint/eslint@7dd402d

3: https://archive.eslint.org/docs/7.0.0/user-guide/command-line-interface

4: https://archive.eslint.org/docs/user-guide/command-line-interface

🏁 Script executed:

#!/bin/bash set -euo pipefail echo "== repo files named check-lint-preconditions ==" fd -a 'check-lint-preconditions\.mjs$' . || true echo "== relevant file excerpt if present ==" for f in $(fd 'check-lint-preconditions\.mjs$' .); do echo "--- $f" wc -l "$f" ast-grep outline "$f" || true sed -n '1,140p' "$f" | cat -n done echo "== usages of lintsWholePackage ==" rg -n "lintsWholePackage|check-lint-preconditions" .

Repository: langwatch/scenario

Length of output: 7942

🌐 Web query:

ESLint CLI help --max-warnings --ignore-path --output-file --parser --plugin --rule --global --cache-location

💡 Result:

The following list details the ESLint CLI options you requested, based on the current official ESLint documentation [1]. --max-warnings Int: Sets the maximum number of warnings allowed. If the number of warnings exceeds this value, ESLint will exit with a non-zero exit code. The default is -1 (no limit) [1][2]. --ignore-path path::String: Specifies the path to a file containing ignore patterns (similar to.gitignore). This allows you to define files and directories that ESLint should ignore during linting [2][3]. -o, --output-file path::String: Specifies a file path to which the linting report will be written. If not provided, the report is typically output to the console [1][2]. --parser String: Specifies the parser to be used for linting (e.g., to handle specific JavaScript versions or non-standard syntax) [1][2]. --plugin [String]: Specifies plugins to be used, which can provide additional rules and configurations to ESLint [1][2]. --rule Object: Explicitly specifies rules to use. This can be used to override configurations defined in your configuration files [1][2]. --global [String]: Defines global variables, allowing you to tell ESLint about variables that should be considered present in the environment [1][2]. --cache-location path::String: Specifies the path to the cache file or directory. This is used in conjunction with the --cache option to store linting results for files that have not changed, improving performance on subsequent runs [1][4][5]. Note that --cache-file is deprecated in favor of this option [1].

Citations:

1: https://eslint.org/docs/latest/use/command-line-interface

2: https://manpages.ubuntu.com/manpages/focal/man1/eslint.1.html

3: https://archive.eslint.org/docs/4.0.0/user-guide/command-line-interface

4: https://github.com/eslint/eslint/blob/f9e54f43/docs/src/use/command-line-interface.md

5: https://mintlify.wiki/eslint/eslint/use/command-line

🌐 Web query:

ESLint 9 --report-unused-disable-directives-severity CLI option

💡 Result:

The --report-unused-disable-directives-severity CLI option in ESLint allows you to specify the severity level for reporting unused eslint-disable and eslint-enable directives [1][2]. Key details regarding this option: Purpose: It provides the same functionality as the --report-unused-disable-directives flag but allows you to set a specific severity level rather than relying on a default [1][2]. You cannot use both options simultaneously; if both are specified, ESLint will throw an error [1][3]. Argument Values: The option accepts a single string argument, which can be one of the following [1][2]: - off (or 0) - warn (or 1) - error (or 2) Default Behavior: If this option is not explicitly provided, ESLint defaults to the value defined in the linterOptions.reportUnusedDisableDirectives configuration setting, which itself defaults to "warn" [1][4]. Relationship with other settings: - This option is available in both the ESLint CLI and is intended to complement the linterOptions.reportUnusedDisableDirectives configuration setting found in flat configuration files (eslint.config.js) [4][5]. - The --report-unused-disable-directives flag remains available as a shorthand for setting the severity to "error" [1][3].

Citations:

1: https://eslint.org/docs/latest/use/command-line-interface

2: https://github.com/eslint/eslint/blob/f9e54f43/docs/src/use/command-line-interface.md

3: Change Request: Merge --report-unused-disable-directives-severity into --report-unused-disable-directives eslint/eslint#18666

4: https://eslint.org/docs/latest/use/configure/configuration-files

5: https://eslint.org/blog/2023/12/eslint-v8.56.0-released/

🌐 Web query:

ESLint 9 --stats CLI option

💡 Result:

The --stats CLI option in ESLint 9 is used to include detailed performance statistics in the linting report [1][2]. When this flag is enabled, ESLint collects performance data—such as parse times, fix times, and lint times per rule—and attaches it to the result objects processed by the formatter [1][3]. Key details about the --stats option: - Purpose: It is primarily designed to allow users and developers to create custom visualizations of ESLint's performance [4]. - Usage: The option takes no arguments [1][2]. It is typically used in conjunction with a formatter that can handle the extra data, such as the built-in json formatter [1][3]. - Example: npx eslint --stats --format json file.js [1][3] - Alternative: You can achieve the same result programmatically by setting the stats: true option in the ESLint constructor when using the Node.js API [3]. Note that not all formatters are designed to output this performance data; it is most compatible with custom formatters or the standard JSON output [1][3]. This feature was introduced as part of the performance improvements in ESLint v9.0.0 [4].

Citations:

1: https://eslint.org/docs/latest/use/command-line-interface

2: https://github.com/eslint/eslint/blob/f9e54f43/docs/src/use/command-line-interface.md

3: https://eslint.org/docs/latest/extend/stats

4: https://eslint.org/blog/2024/04/eslint-v9.0.0-released/

Extend ESLint value-flag handling before the positional check.

lintsWholePackage only consumes the next token for one regex allowlist. ESLint also accepts separate-value flags such as --ignore-path <path>, --output-file <path>, --max-warnings <int>, --parser <name>, --plugin <name>, --global <name>, --cache-location <path>, and --report-unused-disable-directives-severity <level>. A valid script like eslint . --max-warnings 0 leaves 0 as a positional, so the guard fails and reports the script does not cover the whole package. Include the relevant ESLint value-flag allowlist or parse only whether the single remaining positional is the package root.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@javascript/scripts/check-lint-preconditions.mjs` around lines 56 - 62, Extend the value-taking flag allowlist in the token loop before the positional check, updating the condition associated with the visible token handling so separate values for ESLint flags such as --ignore-path, --output-file, --max-warnings, --parser, --plugin, --global, --cache-location, and --report-unused-disable-directives-severity are consumed. Preserve positional detection for the package-root argument and ensure valid commands like eslint . --max-warnings 0 do not treat 0 as an additional positional.

Source: Linters/SAST tools

drewdrewthis · 2026-08-02T10:35:08Z

Lane stopped — this PR is NOT ready for review, correcting my own earlier call

The worker on this branch was stopped today when the owner cut the fleet to one lane. I had recorded this PR as "CI-green, awaiting a human reviewer". That was wrong, and I only caught it by running the tool that owns the bar instead of reading gh pr checks and inferring.

scripts/pr-ready-check.sh langwatch scenario 865 → overall: NOT_READY

PASS  C1: open, non-draft, mergeable (OPEN)
PASS  C2: CI green (15 checks after supersede-collapse; tally={'SUCCESS': 12, 'SKIPPED': 4})
FAIL  C3: no /review verdict comment found
FAIL  C4: 1 unresolved review thread(s); 1 unresolved/orphaned top-level review comment(s)
FAIL  C5: body missing '## Human verification'
FAIL  C6: no live-app visual proof and no backend-only declaration
PASS  C7: no deferral language in body

CI being green was the only part I had right, and it is C2 of eight.

What actually remains

C4 — an unresolved Major finding, and it is substantive. CodeRabbit on javascript/scripts/check-lint-preconditions.mjs:62, tagged Functional Correctness / Major, concerning ESLint 9 CLI options that take a separate value argument. Not a nit; it wants a real answer or a fix, then thread resolution.
C3 — no /review verdict comment. The fan-out has never run on this branch.
C5 — the body has no ## Human verification section. Reformat via /write-pr.
C6 — no live-app proof and no backend-only declaration. This is a lint/CI change with no UI surface, so the honest resolution is almost certainly an explicit backend-only declaration ( or prose), not a screenshot.

State of the work itself

The three commits are sound and fully on origin (c4b0b4d, matching the branch head) — nothing was lost when the session stopped, and there is nothing to re-do. What is missing is the finishing pass, not the implementation.

Worth knowing for whoever resumes: the issue's original premise shifted underneath this work. The worker was briefed to re-baseline because merged PR #755 added a lint:lib step to CI after #565 was filed, so the "~269 problems" figure and AC 1 in the issue body may no longer describe reality. See dec.2026-08-01-scenario-565-lint-ac-set for which ACs govern after measurement flipped two of five.

Board

Moved off Ready for Human Review — that lane asserted a readiness the bar contradicts, which is precisely the failure mode we are trying to stop asserting. It goes back to the queue and gets a lane when one frees; the fleet is capped at one worker today and that lane is on #718.

— orchardist

rogeriochaves

according to @drewdrewthis, not ready for review

github-actions · 2026-08-12T06:42:21Z

Automated low-risk assessment

This PR was evaluated against the repository's Low-Risk Pull Requests procedure and does not qualify as low risk.

This PR modifies files in restricted directories that require manual review per policy.

This PR requires a manual review before merging.

Ubuntu added 2 commits August 1, 2026 08:15

drewdrewthis self-assigned this Aug 1, 2026

drewdrewthis requested a review from rogeriochaves August 1, 2026 08:18

This was referenced Aug 1, 2026

fix(ci): draft PRs bypass every required JavaScript check, and leaving draft never re-triggers them #866

Open

test(ci): examples/custom-observability is type-checked and tested by nothing in CI #867

Closed

coderabbitai Bot reviewed Aug 1, 2026

View reviewed changes

drewdrewthis mentioned this pull request Aug 2, 2026

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

Open

11 tasks

langwatch-agent removed the request for review from rogeriochaves August 3, 2026 06:35

langwatch-agent mentioned this pull request Aug 3, 2026

test(ci): type-check and run the custom-observability probes (#867) #874

Merged

langwatch-agent requested a review from rogeriochaves August 4, 2026 11:22

langwatch-agent added hound-checked Triaged by the pr-hound agent at the current head SHA ci-green Latest run of every check is passing (checks API, not the legacy commit-status index) labels Aug 5, 2026

rogeriochaves requested changes Aug 11, 2026

View reviewed changes

langwatch-agent added the P3 - low Low priority, nice to have label Aug 12, 2026

coderabbitai Bot mentioned this pull request Aug 20, 2026

perf(js): load the ElevenLabs SDK only when voice runs #933

Merged

Navigation Menu

Uh oh!

refactor(lint): gate every root-owned TypeScript file and close two silent-skip holes (#565) - #865

refactor(lint): gate every root-owned TypeScript file and close two silent-skip holes (#565)#865
drewdrewthis wants to merge 3 commits into
mainfrom
issue565/enable-clean-root-package-eslint

drewdrewthis commented Aug 1, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 1, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Aug 1, 2026

Uh oh!

drewdrewthis commented Aug 2, 2026

Uh oh!

rogeriochaves left a comment

Uh oh!

github-actions Bot commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

drewdrewthis commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

For Humans

The real numbers

Two AC corrections worth reading

Two structural findings the issue did not have

How I can prove I was successful

Deliberately out of scope

Review notes

Uh oh!

coderabbitai Bot commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

drewdrewthis commented Aug 2, 2026

Lane stopped — this PR is NOT ready for review, correcting my own earlier call

What actually remains

State of the work itself

Board

Uh oh!

rogeriochaves left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

drewdrewthis commented Aug 1, 2026 •

edited

Loading

coderabbitai Bot commented Aug 1, 2026 •

edited

Loading