diff --git a/docs/superpowers/plans/2026-04-14-phase-1a3-eslint-boundaries.md b/docs/superpowers/plans/2026-04-14-phase-1a3-eslint-boundaries.md new file mode 100644 index 00000000..60a39ca8 --- /dev/null +++ b/docs/superpowers/plans/2026-04-14-phase-1a3-eslint-boundaries.md @@ -0,0 +1,467 @@ +# Phase 1A-3 — ESLint Boundaries + Restricted Imports Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add `eslint-plugin-boundaries` layered dependency rules and `no-restricted-imports` guards to the existing ESLint flat config so that architectural violations are caught at lint time — preventing features from importing routes, UI from importing features, observability from leaking SDK internals, and storage from bypassing the `src/shared/storage.ts` wrapper. + +**Architecture:** Extends `eslint.config.js` from 1A-1 with two new plugins/rules: `eslint-plugin-boundaries` (defines element types by directory pattern, enforces which layers can import which) and built-in `no-restricted-imports` (bans specific package imports outside their designated "owner" file). Each rule has a fabricated-violation test in `tests/eslint/` that asserts the rule fires. + +**Tech Stack:** `eslint-plugin-boundaries@^5.0.0` (ESLint 9 flat-config compatible), ESLint built-in `no-restricted-imports`. + +**Scope:** Config-only. No runtime code, no tests of production logic. Every task is "add a rule + add a probe test that proves it fires." + +**Prerequisites (1A-1 + 1A-2 complete):** +- `eslint.config.js` flat config with `typescript-eslint`, `unused-imports` already configured. +- `src/` tree with `features/`, `ui/`, `shared/`, `observability/`, `mf/`, `routes/`, `env/` directories populated. + +--- + +## File structure + +| File | Responsibility | Task | +|---|---|---| +| `eslint.config.js` | Modify: add boundaries plugin + no-restricted-imports rules | 1, 2 | +| `tests/eslint/boundaries.test.ts` | Fabricated violation tests for boundary rules | 3 | +| `tests/eslint/restricted-imports.test.ts` | Fabricated violation tests for restricted imports | 4 | + +--- + +## Task 1 — Install `eslint-plugin-boundaries` and configure element types + +**Files:** +- Modify: `package.json` (devDep) +- Modify: `eslint.config.js` + +- [ ] **Step 1: Install the plugin** + +```bash +pnpm add -D eslint-plugin-boundaries@^5.0.0 +``` + +- [ ] **Step 2: Add boundary element types and dependency rules to `eslint.config.js`** + +Add the following to `eslint.config.js` — import at the top, new config object appended to the array: + +At the top of the file, add the import: +```javascript +import boundaries from "eslint-plugin-boundaries"; +``` + +Then add a new config object to the exported array (after the existing `files: ["src/**/*.{ts,tsx}"]` block): + +```javascript + { + files: ["src/**/*.{ts,tsx}"], + plugins: { + boundaries, + }, + settings: { + "boundaries/elements": [ + { type: "routes", pattern: "src/routes/*" }, + { type: "mf", pattern: "src/mf/*" }, + { type: "features", pattern: "src/features/*", capture: ["feature"] }, + { type: "ui", pattern: "src/ui/*" }, + { type: "shared", pattern: "src/shared/*" }, + { type: "observability", pattern: "src/observability/*" }, + { type: "i18n", pattern: "src/i18n/*" }, + { type: "env", pattern: "src/env/*" }, + ], + }, + rules: { + // Design spec §1.2 layered dependency direction: + // features/ cannot import routes/ or mf/ + // ui/ cannot import features/ + // shared/ cannot import features/, routes/, mf/, observability/ + // observability/ cannot import features/, routes/, mf/ + "boundaries/element-types": [ + "error", + { + default: "allow", + rules: [ + { + from: "features", + disallow: ["routes", "mf"], + message: "Features must not import from routes/ or mf/. Use the HostContract or a shared module instead.", + }, + { + from: "ui", + disallow: ["features", "routes", "mf"], + message: "UI layer must not import from features/, routes/, or mf/. UI is consumed by features, not the other way around.", + }, + { + from: "shared", + disallow: ["features", "routes", "mf", "observability"], + message: "Shared modules must not import from features/, routes/, mf/, or observability/.", + }, + { + from: "observability", + disallow: ["features", "routes", "mf"], + message: "Observability modules must not import from features/, routes/, or mf/.", + }, + ], + }, + ], + }, + }, +``` + +- [ ] **Step 3: Verify lint still passes on existing code** + +```bash +pnpm lint +``` +Expected: exit 0 — existing code doesn't violate the boundaries because no cross-layer imports exist yet. + +- [ ] **Step 4: Commit** + +```bash +git add package.json pnpm-lock.yaml eslint.config.js +git commit -m "Add eslint-plugin-boundaries with layered dependency rules" +``` + +--- + +## Task 2 — Add `no-restricted-imports` rules + +**Files:** +- Modify: `eslint.config.js` + +Four restricted-import rules per master plan §1A-3: +1. `@opentelemetry/sdk-metrics` — only in `src/observability/metrics/otel.ts` +2. `window.localStorage` / `window.sessionStorage` — only in `src/shared/storage.ts` (via `no-restricted-globals`) +3. `@microsoft/signalr` — forbidden in SSR-bundle files (enforced by file-path pattern) +4. `react-i18next` — only in `src/i18n/provider.tsx` + +- [ ] **Step 1: Add restricted-imports rules to `eslint.config.js`** + +Add a new config object to the array — these are file-path-scoped overrides: + +```javascript + // --- Restricted imports (master plan §1A-3) --- + // OTel SDK internals: only src/observability/metrics/otel.ts may import them + { + files: ["src/**/*.{ts,tsx}"], + ignores: ["src/observability/metrics/otel.ts"], + rules: { + "no-restricted-imports": [ + "error", + { + paths: [ + { + name: "@opentelemetry/sdk-metrics", + message: "Import from @opentelemetry/api or use getMeter() from @/observability/metrics/otel instead. Direct SDK access is restricted to src/observability/metrics/otel.ts.", + }, + { + name: "@opentelemetry/sdk-node", + message: "Import from @opentelemetry/api or use getMeter()/getTracer() from @/observability/metrics/otel instead. Direct SDK access is restricted to src/observability/metrics/otel.ts.", + }, + ], + }, + ], + }, + }, + // react-i18next: only src/i18n/provider.tsx may import it + { + files: ["src/**/*.{ts,tsx}"], + ignores: ["src/i18n/provider.tsx"], + rules: { + "no-restricted-imports": [ + "error", + { + paths: [ + { + name: "react-i18next", + message: "Import useTranslation from @/i18n/provider instead. Direct react-i18next imports are restricted to src/i18n/provider.tsx.", + }, + ], + }, + ], + }, + }, + // @microsoft/signalr: forbidden in files that run during SSR. + // SSR-bundle = routes/ and server/ directories. Features + shared/hooks are client-side. + { + files: ["src/routes/**/*.{ts,tsx}", "src/server/**/*.{ts,tsx}"], + rules: { + "no-restricted-imports": [ + "error", + { + paths: [ + { + name: "@microsoft/signalr", + message: "SignalR must not be imported in SSR-bundle files (routes/, server/). Use dynamic import in a useEffect or a client-only wrapper.", + }, + ], + }, + ], + }, + }, + // window.localStorage / window.sessionStorage: only src/shared/storage.ts + { + files: ["src/**/*.{ts,tsx}"], + ignores: ["src/shared/storage.ts"], + rules: { + "no-restricted-globals": [ + "error", + { + name: "localStorage", + message: "Use the storage module from @/shared/storage instead. Direct localStorage access is restricted to src/shared/storage.ts.", + }, + { + name: "sessionStorage", + message: "Use the storage module from @/shared/storage instead. Direct sessionStorage access is restricted to src/shared/storage.ts.", + }, + ], + }, + }, +``` + +- [ ] **Step 2: Verify lint still passes** + +```bash +pnpm lint +``` +Expected: exit 0 — no existing file imports these restricted packages. + +- [ ] **Step 3: Commit** + +```bash +git add eslint.config.js +git commit -m "Add no-restricted-imports for OTel SDK, react-i18next, SignalR SSR, localStorage" +``` + +--- + +## Task 3 — Fabricated violation tests for boundary rules + +**Files:** +- Create: `tests/eslint/boundaries.test.ts` + +These tests create temporary probe files, run `eslint` on them, and assert violations. This is NOT a vitest test — it's a shell-command test that invokes ESLint on fabricated files. But we wrap it in vitest for consistency with the test runner. + +- [ ] **Step 1: Write `tests/eslint/boundaries.test.ts`** + +```typescript +import { describe, expect, it } from "vitest"; +import { execSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; + +const ROOT = path.resolve(import.meta.dirname, "../.."); + +function lintString(filePath: string, content: string): string { + const absPath = path.join(ROOT, filePath); + fs.mkdirSync(path.dirname(absPath), { recursive: true }); + fs.writeFileSync(absPath, content, "utf8"); + try { + execSync(`pnpm exec eslint "${absPath}" --no-eslintrc -c eslint.config.js --format json`, { + cwd: ROOT, + encoding: "utf8", + stdio: ["pipe", "pipe", "pipe"], + }); + return "PASS"; + } catch (err: unknown) { + const error = err as { stdout?: string }; + return error.stdout ?? "UNKNOWN_ERROR"; + } finally { + fs.unlinkSync(absPath); + } +} + +describe("boundaries rules", () => { + it("features/ cannot import from routes/", () => { + const result = lintString( + "src/features/online-board/__test_probe__.ts", + 'import { something } from "../../routes/page";\nexport const x = something;\n', + ); + expect(result).toContain("boundaries/element-types"); + }); + + it("features/ cannot import from mf/", () => { + const result = lintString( + "src/features/online-board/__test_probe__.ts", + 'import { REMOTE_BUILD_MARKER } from "../../mf/host-entry";\nexport const x = REMOTE_BUILD_MARKER;\n', + ); + expect(result).toContain("boundaries/element-types"); + }); + + it("ui/ cannot import from features/", () => { + const result = lintString( + "src/ui/__test_probe__.ts", + 'import { something } from "../features/online-board";\nexport const x = something;\n', + ); + expect(result).toContain("boundaries/element-types"); + }); + + it("shared/ cannot import from features/", () => { + const result = lintString( + "src/shared/__test_probe__.ts", + 'import { something } from "../features/online-board";\nexport const x = something;\n', + ); + expect(result).toContain("boundaries/element-types"); + }); + + it("observability/ cannot import from features/", () => { + const result = lintString( + "src/observability/__test_probe__.ts", + 'import { something } from "../features/online-board";\nexport const x = something;\n', + ); + expect(result).toContain("boundaries/element-types"); + }); + + it("features/ CAN import from shared/ (allowed direction)", () => { + // This should NOT trigger a boundary violation. + // We can't fully test this without a real shared module, so just verify + // the rule doesn't fire on a features→env import (which is allowed). + const result = lintString( + "src/features/online-board/__test_probe__.ts", + 'import { getEnv } from "../../env";\nexport const x = getEnv;\n', + ); + expect(result).not.toContain("boundaries/element-types"); + }); +}); +``` + +- [ ] **Step 2: Update vitest config to include `tests/` directory** + +The current `vitest.config.ts` has `include: ["src/**/*.test.ts", "src/**/*.test.tsx"]`. Add `tests/` to the include: + +Edit `vitest.config.ts` `include` to: + +```typescript + include: ["src/**/*.test.ts", "src/**/*.test.tsx", "tests/**/*.test.ts"], +``` + +- [ ] **Step 3: Run the boundary tests** + +```bash +pnpm test tests/eslint/boundaries +``` +Expected: all 6 tests pass. The first 5 should show "boundaries/element-types" in the ESLint output; the last one should NOT. + +If tests fail because `eslint-plugin-boundaries` doesn't detect imports via relative paths, the plugin may need the `boundaries/include` setting or file resolution config. Debug by running the lint command manually on a probe file and checking what ESLint reports. + +If the probe tests are too slow (each spawns an ESLint process), that's acceptable — these are CI-only correctness checks, not hot-loop unit tests. + +- [ ] **Step 4: Commit** + +```bash +git add tests/eslint/boundaries.test.ts vitest.config.ts +git commit -m "Add fabricated violation tests for boundary rules" +``` + +--- + +## Task 4 — Fabricated violation tests for restricted imports + +**Files:** +- Create: `tests/eslint/restricted-imports.test.ts` + +- [ ] **Step 1: Write `tests/eslint/restricted-imports.test.ts`** + +```typescript +import { describe, expect, it } from "vitest"; +import { execSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; + +const ROOT = path.resolve(import.meta.dirname, "../.."); + +function lintString(filePath: string, content: string): string { + const absPath = path.join(ROOT, filePath); + fs.mkdirSync(path.dirname(absPath), { recursive: true }); + fs.writeFileSync(absPath, content, "utf8"); + try { + execSync(`pnpm exec eslint "${absPath}" --no-eslintrc -c eslint.config.js --format json`, { + cwd: ROOT, + encoding: "utf8", + stdio: ["pipe", "pipe", "pipe"], + }); + return "PASS"; + } catch (err: unknown) { + const error = err as { stdout?: string }; + return error.stdout ?? "UNKNOWN_ERROR"; + } finally { + fs.unlinkSync(absPath); + } +} + +describe("no-restricted-imports rules", () => { + it("blocks @opentelemetry/sdk-metrics outside otel.ts", () => { + const result = lintString( + "src/features/online-board/__test_probe__.ts", + 'import { MeterProvider } from "@opentelemetry/sdk-metrics";\nexport const x = MeterProvider;\n', + ); + expect(result).toContain("no-restricted-imports"); + }); + + it("blocks react-i18next outside provider.tsx", () => { + const result = lintString( + "src/features/online-board/__test_probe__.ts", + 'import { useTranslation } from "react-i18next";\nexport const x = useTranslation;\n', + ); + expect(result).toContain("no-restricted-imports"); + }); + + it("blocks @microsoft/signalr in routes/ (SSR bundle)", () => { + const result = lintString( + "src/routes/__test_probe__.ts", + 'import { HubConnectionBuilder } from "@microsoft/signalr";\nexport const x = HubConnectionBuilder;\n', + ); + expect(result).toContain("no-restricted-imports"); + }); + + it("blocks localStorage outside storage.ts", () => { + const result = lintString( + "src/features/online-board/__test_probe__.ts", + 'const x = localStorage.getItem("key");\nexport default x;\n', + ); + expect(result).toContain("no-restricted-globals"); + }); +}); +``` + +- [ ] **Step 2: Run the restricted-import tests** + +```bash +pnpm test tests/eslint/restricted-imports +``` +Expected: all 4 tests pass. + +- [ ] **Step 3: Commit** + +```bash +git add tests/eslint/restricted-imports.test.ts +git commit -m "Add fabricated violation tests for restricted import rules" +``` + +--- + +## Task 5 — Full exit-gate verification + +- [ ] **Step 1: Quality gates** + +```bash +pnpm typecheck && pnpm lint && pnpm test +``` +Expected: typecheck exit 0, lint exit 0, all tests pass (11 existing + 10 new = 21 total). + +- [ ] **Step 2: Verify git status** + +```bash +git status +``` +Expected: clean working tree. + +--- + +## Self-review + +**Spec coverage.** Master plan §1A-3 exports: +- `eslint-plugin-boundaries` rules for layered deps → Task 1 +- `no-restricted-imports`: OTel SDK, react-i18next, @microsoft/signalr SSR, localStorage → Task 2 +- Fabricated violation tests → Tasks 3, 4 + +**Placeholder scan.** No TBD/TODO. All rule configs are concrete. + +**Type consistency.** Element type names (`routes`, `mf`, `features`, `ui`, `shared`, `observability`, `i18n`, `env`) match the `src/` directory names.