Adopt @exadev/eslint-config - #117
Merged
Merged
Conversation
Adds @exadev/eslint-config as an exact-pinned dev dependency, replaces the hand-rolled flat config with the shared array plus a siblings-mode barrel-policy override and this repo's own overrides (unused-vars, restricted-syntax, prettier), bumps the pinned package manager to pnpm 12.4.1, and decides allowBuilds for the four lifecycle-script packages pnpm 12 now gates on (esbuild and koffi need their native-binary install step, @google/genai and protobufjs don't). Also fixes the three Husky hooks, which invoked their tools as `node node_modules/.bin/<tool>` -- pnpm 12 changed .bin shims to POSIX shell scripts, so forcing them through node's CommonJS loader now crashes with a syntax error on the shim's own shebang line. Switches all three to `pnpm exec <tool>`, which resolves and runs the shim correctly regardless of its underlying format.
Fixes lint violations across the harness bridge entry points and the user CLI/controller: named constants for magic numbers, Readonly<> wraps on flat object/array parameters, explicit nullable-string/number checks in place of implicit truthiness, method-signature-style conversions, and a Promise.resolve() return in pi/extension.ts's comms-url command handler (it never awaits anything, so it doesn't need to be async -- the earlier eslint-disable-next-line comment suppressing require-await had no effect anyway, since this config sets noInlineConfig).
…rver Fixes lint violations across the web dashboard's Preact frontend and its HTTP/WebSocket server: widens rooms/agents/messages prop and state types to readonly arrays throughout the whole App -> Sidebar/ ChatArea -> MessageList -> project-tree prop-drilling chain (nothing in that chain ever mutates them), converts ServiceWorkerGlobalScope's overloaded addEventListener interface member to an intersection of call signatures to satisfy method-signature-style while preserving its per-event-type overload typing, and applies the same named- constant/readonly-param/explicit-nullable-check patterns used elsewhere in this pass.
Fixes lint violations across the web dashboard's e2e/unit/integration tests, and removes fixtures.ts's plain re-export of Playwright's own expect (a barrel-policy violation outside an index file) -- each e2e test now imports expect directly from @playwright/test alongside test from ./fixtures.js, matching the barrel/re-export rules the rest of this pass already applies everywhere else.
… fixes Several fixes here changed runtime behaviour, not just syntax, so they get their own commit distinct from the purely mechanical sweep: - tool.ts: CommsTool's mesh/room-admission action handlers extracted each optional MeshOnlyFeatures method into a local const before calling it inside an async closure (e.g. `const acceptConnection = this.store.acceptConnection; ... acceptConnection(id)`), which detaches the method from its `this` binding -- every one of these calls threw "Cannot read properties of undefined" against MeshStore's own private fields at runtime. Fixed by binding each method to this.store explicitly (`.bind(this.store)`) right after the existence guard, which both preserves this binding and keeps the narrowed, non-optional call signature inside the closure. - mesh-store-shared.ts: mergeMessageHistories was refactored from an in-place array mutation to a pure function returning a new merged array (satisfying exadev/prefer-readonly-array-param without leaving the parameter falsely readonly). delivery-engine.ts's two call sites needed updating to consume the return value, or merged message history would silently stop being applied. - room-router.ts: routeLegacyMessage's switch gained an explicit case for every MeshMessage union member to satisfy switch-exhaustiveness-check, which meant dropping the old unconditional default case entirely. A wire message naming a method outside the closed union (e.g. from a peer on a newer build) no longer matched anything and silently went unrouted. Restored a default case that forwards to events.onMessage, matching the original catch-all behaviour, while keeping the explicit cases for compile-time exhaustiveness. - discovery-tailscale.ts: the hand-written execFileAsync replacement for promisify(execFile) used NodeJS.ErrnoException for its callback error parameter, which doesn't match any of execFile's actual overload signatures and broke type inference for the whole call. Switched to ExecException (execFile's real error type; its deprecated ExecFileException alias was tried first and rejected by @typescript-eslint/no-deprecated), and guards the promise rejection with an instanceof check per prefer-promise-reject-errors. federation.ts's own switch over MeshMessage gained the same explicit per-case coverage, but its already-existing catch-all behaviour was "ignore silently" both before and after (no dedicated default logic was ever lost), so it's included here as a same-file, same-cause fix rather than a behavioural regression.
Mechanical, root-cause fixes across the rest of the mesh core: named constants for magic numbers (including a hand-written ASN.1/DER/X.509 certificate encoder in identity.ts with ~40 of them), Readonly<> wraps on flat object/array parameters, method-signature-style conversions on interface members, explicit undefined/empty-string checks in place of implicit truthiness, async on every promise-returning method, prefer-readonly on constructor-only class fields, and TSDoc escaping. Also removes push-manager.ts's redundant re-export of PushSubscription/PushPayload, now that core/index.ts re-exports them directly from their real source, web-push.ts.
Mechanical, root-cause fixes across the mesh core's integration and unit tests: named timing/count constants replacing repeated magic numbers, readonly params, braced void-return arrow callbacks, async on promise-returning helpers (sleep/findFreePort and similar), and explicit undefined/empty-string checks. room-router.test.ts's own "routes an otherwise-well-formed FRAME_VERB payload with no recognised case to the onMessage catch-all" test is what caught the missing-default regression fixed in the earlier core commit -- it still exercises the same behaviour, unchanged.
Fixes cli.ts's argv-parsing helpers (parseNameArg's rest field widened to readonly string[], since it's only ever read downstream, never mutated) and named constants / readonly params in scripts/publish-mcp-registry.ts.
The suite's setup() called createWebServer(0) with no coordinatorPort override, so its MeshStore always bound the real default coordinator port (19876) -- the same well-known port any other running agent-comms bridge instance on the same machine already listens on. On a machine actively running one (a normal state for this project's own contributors, who dogfood agent-comms as part of their own development tooling), every test in this suite hung for the full 30-second test timeout instead of ever completing, since the store was joining a real, unrelated, already-established mesh rather than starting its own isolated one. Adds a findFreePort() helper (matching the identical pattern already used by the mesh core's own integration tests) and passes its result as an explicit coordinatorPort, giving each test run its own private mesh regardless of what else is running locally.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
🎉 This PR is included in version 2.21.7 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@exadev/eslint-configas an exact-pinned dependency, replacing the hand-rolled flat config with the shared array plus a siblings-modebarrel-policyoverride.allowBuildsfor the packages pnpm 12 now gates lifecycle scripts on.node node_modules/.bin/<tool>— pnpm 12 changed.binshims to POSIX shell scripts, so forcing them through node's CommonJS loader crashed on the shim's own shebang line. Switched topnpm exec <tool>.index.tsfiles (bridges/index.ts,bridges/mcp/index.ts,bridges/pi/index.ts) thatbarrel-policyin siblings mode forces to be pure re-export barrels — renamed to descriptive names (registry.ts,server.ts,extension.ts) and updated every reference, includingpackage.json'spi.extensionsentry.mesh-store.ts,push-manager.ts, the Playwright e2efixtures.ts), redirecting their consumers to import directly from the real source.Readonly<>wraps, method-signature-style conversions, explicit nullable checks, TSDoc escaping, and more.tool.ts: 11 call sites extracted an optionalMeshOnlyFeaturesmethod into a local const before calling it inside a closure, detaching it fromthisand throwing at runtime. Fixed with.bind(this.store).mesh-store-shared.ts/delivery-engine.ts:mergeMessageHistoriesbecame a pure function instead of mutating in place; its two call sites needed updating to consume the return value.room-router.ts: an exhaustive switch lost its catch-alldefault, silently dropping messages from a wire method outside the closed union (e.g. a peer on a newer build). Restored thedefaultalongside the explicit cases.Test plan
pnpm lintcleanpnpm exec tsc --noEmitclean (verified with a fresh tsbuildinfo)pnpm buildsucceedspnpm test— 367 tests passing