.convoys/lint-against-cjs-in-esm-scripts.md

@@ -0,0 +1,176 @@
+---
+name: lint-against-cjs-in-esm-scripts
+classification: hygiene
+success_metric: future helper scripts that re-introduce CJS `require()` calls under `package.json` "type": "module" fail at lint time, not at first execution
+status: open
+created: 2026-05-26
+---
+
+# lint-against-cjs-in-esm-scripts (P3 polish — parent-owned)
+
+**Priority:** P3 polish (one-line ESLint rule; no architect required)
+**Convoy owner:** parent
+**Opened:** 2026-05-26
+
+## Background — the recurring bug pattern
+
+Since `bump-next-js` flipped `package.json` to `"type": "module"`,
+any helper script under `scripts/` that uses CJS `require()` throws
+`ReferenceError: require is not defined` on Node 22.x at first run.
+The same bug has now bitten the repo twice in two convoys:
+
+1. **`drop-public-setup` Brief 2** (commit `b63b509`, 2026-05-23):
+   `scripts/setup-neon-db.js` was still CJS post-`bump-next-js`; `npm
+   run setup-db` was silently broken until Brief 2 swept it to ESM
+   imports. The convoy retro called this out as "the seed script
+   silently stopped executing after `bump-next-js`."
+2. **`fix-reset-db-script` Brief 1** (commit `3ab9bf8`, PR #25,
+   2026-05-26): three `require()` calls in `scripts/reset-db.js`
+   (lines 10, 12, 142) — same bug, same blast radius (`npm run
+   reset-db` throws `ReferenceError`), same fix shape (verbatim
+   mirror of post-`drop-public-setup` `setup-neon-db.js`).
+
+Both bugs were caught at first run, not at lint time. A small
+ESLint rule scoped to `scripts/**/*.js` would have caught both at
+PR time and is cheap insurance against a third recurrence.
+
+## Design decision — `scripts/**` only (NOT all `.js`)
+
+Two reasonable scopes:
+
+- **`scripts/**/*.js` (chosen):** matches the actual blast radius —
+  every observed instance of the bug has been in a helper script.
+  Per-file-block override in `eslint.config.mjs` via a second flat-
+  config entry. Zero impact on `pages/api/**` (already correctly
+  ESM-imported throughout) and zero impact on the root `*.config.js`
+  files (which are intentionally CJS-shaped and which the next-config
+  base rules already handle correctly).
+- **All `.js` files at repo root (rejected):** broader-than-necessary
+  blast radius. `pages/api/**` already uses ESM `import` everywhere
+  (`add-rate-limiting`, `cors-tighten`, and `add-route` skill all
+  verified this in the last three months). A repo-wide ban would
+  produce zero true positives outside `scripts/**` today and would
+  risk breaking config-file shapes that legitimately use CJS
+  (`postcss.config.js`, `tailwind.config.js` are flagged by
+  `import/no-anonymous-default-export` today but read CJS-style
+  exports under the hood — see also Gotcha #9 + #10).
+
+The scoped rule is a 7-line flat-config block; the broader rule
+would require explicit allowlist for every config file, which is
+strictly more code and more maintenance.
+
+## The fix
+
+Add a new flat-config block at the end of `eslint.config.mjs` (after
+the existing `globalIgnores(...)` call, NOT inside the root rules
+block) targeting only `scripts/**/*.js`:
+
+```js
+{
+  files: ['scripts/**/*.js'],
+  rules: {
+    'no-restricted-syntax': ['error', {
+      selector: 'CallExpression[callee.name="require"]',
+      message: 'Use ESM `import` syntax. `package.json` has "type": "module"; require() throws ReferenceError at runtime. See .convoys/fix-reset-db-script.md.',
+    }],
+  },
+},
+```
+
+The error message points at `.convoys/fix-reset-db-script.md` so that
+the next agent / contributor who triggers the rule gets a 1-click path
+to the exemplar fix (ESM top-level imports for `dotenv`, `neon`,
+`bcrypt`) instead of having to re-derive it.
+
+`scripts/migrations/**` is already in `globalIgnores` (from
+`pick-a-name` Brief 2's migration script) and stays ignored — the
+rule does not fire there even though the migration script is ESM and
+correctly uses `import` (no need to re-lint files already excluded).
+
+The rule fires on `CallExpression[callee.name="require"]` — the AST
+shape of a plain `require('foo')` call. It does NOT fire on
+`createRequire(import.meta.url)` patterns (which use `Module.createRequire`)
+should one ever be needed; the AST callee is `createRequire`, not
+`require`. If a future helper script legitimately needs CJS interop,
+the right path is `await import('foo')` (ESM dynamic import) — the
+rule will not block that either.
+
+## Verification plan
+
+1. `node --check eslint.config.mjs` → exit 0 (config parses).
+2. `npm run lint` → exit 1 with **128 problems (81 errors, 47
+   warnings)** — verbatim match of the pre-convoy baseline (no
+   regression, no new false positives in the current tree).
+3. **Negative test (apply, run, revert):** prepend
+   `const x = require('fs');` to `scripts/reset-db.js`, run
+   `npm run lint`, confirm exit 1 with the new rule firing at the
+   expected line/column and the documented message, then revert.
+4. `npm run test:run` → 21/21 pass (no test surface touched; runs
+   only to confirm vitest is still green).
+5. `rg "require\(" scripts/ --type js` → 0 hits (sanity check
+   confirming the current tree is clean and the rule has zero
+   positives to silence on day 1).
+
+## Risks
+
+- **False positives if anyone legitimately needs `require()` in
+  `scripts/**`.** None today (verified by step 5 — zero `require(`
+  hits across all helper scripts in the current tree after PR #25
+  and the `drop-public-setup` B2 sweep). If a future script
+  legitimately needs CJS interop (e.g. a dependency that only
+  exports CJS without an ESM wrapper), the fix is `await
+  import('foo')` — ESM dynamic import works in any ESM script and
+  is not flagged by the rule. If that's somehow not viable, the
+  escape hatch is a per-line `// eslint-disable-next-line
+  no-restricted-syntax` with a comment explaining why ESM doesn't
+  work; lint baseline tracking will catch the disable directive in
+  review.
+- **Rule scope drift.** If someone adds a new top-level scripts
+  directory (`tools/`, `cli/`, etc.) the rule won't fire there. Low
+  risk — this repo has consolidated on `scripts/` since inception
+  and there's no signal of a second scripts directory being added.
+  Tracked here so the next refactor that reshapes the helper-script
+  layout knows to extend the `files:` glob.
+- **ESLint v10 bump.** When `bump-eslint-10` lands (currently
+  upstream-blocked per Gotcha #10), re-verify this rule's selector
+  syntax against the v10 AST behavior. `no-restricted-syntax` is a
+  stable core rule going back to ESLint v1; no v10 deprecation is
+  expected, but the smoke check is cheap.
+
+## Acceptance criteria
+
+- `node --check eslint.config.mjs` exit 0
+- `npm run lint` exit 1 with 128 problems (baseline preserved)
+- `npm run test:run` 21/21 pass
+- Negative test passes (rule fires on synthetic `require()` insertion,
+  reverts cleanly to 128 problems after the synthetic edit is
+  removed)
+- Grep: 0 `require(` occurrences in `scripts/**/*.js` (current tree
+  is clean — rule starts with zero positives to silence)
+
+## Out of scope
+
+- Sweeping any other `scripts/**` file — current tree is clean
+  (verified by the grep step above). The rule is preventative,
+  not retroactive.
+- Broadening the rule to all `.js` files at repo root — see
+  § Design decision; `pages/api/**` is already correctly ESM and
+  the config files (`postcss.config.js`, `tailwind.config.js`,
+  `next.config.js`) intentionally use CJS-style exports that the
+  next-config base rules handle correctly.
+- Bumping any deps (ESLint stays at v9 per Gotcha #10; no
+  `typescript-eslint` interaction since the rule is a core rule).
+- The `purge-weak-creds-from-helpers` follow-up (the
+  `scripts/create-test-users.js` portion remains queued; this
+  convoy only adds the lint rule, not the weak-creds sweep).
+
+## Owns
+
+Parent (single-file ESLint config edit; no architect or implementer
+subagent required — proven-pattern follow-up to PR #25).
+
+## As-shipped
+
+_(Stub — doc-writer to fill in post-merge with squash commit SHA,
+PR URL, observed lint baseline before/after, CI gate results, and
+any deviations from the planned shape.)_

eslint.config.mjs

@@ -11,6 +11,15 @@ const eslintConfig = defineConfig([
     'next-env.d.ts',
     'scripts/migrations/**',
   ]),
+  {
+    files: ['scripts/**/*.js'],
+    rules: {
+      'no-restricted-syntax': ['error', {
+        selector: 'CallExpression[callee.name="require"]',
+        message: 'Use ESM `import` syntax. `package.json` has "type": "module"; require() throws ReferenceError at runtime. See .convoys/fix-reset-db-script.md.',
+      }],
+    },
+  },
 ]);
 
 export default eslintConfig;