chore(audit): unblock workspace lint pipeline + 13 mechanical fixes
The first `pnpm -r exec eslint .` run was bailing at the very first
package (design-tokens), hiding any lint state in the rest of the 69
workspace packages. This commit fixes the structural blockers so the
pipeline runs end-to-end, then sweeps the small, low-risk lint errors
in the next 4 packages it surfaces. Real lint debt that remains
(85 errors, mostly @typescript-eslint/no-unused-vars across many
unrelated packages) is cataloged in docs/AUDIT_PLATFORM.md for follow-
up by package owners.
Structural fixes (eslint config):
- eslint.config.js (root):
• New flat-config block for **/*.cjs and **/scripts/**/*.{js,cjs}
with Node globals (process, console, require, module, __dirname)
and no-console disabled. CLI scripts legitimately print to
stdout. This alone clears the 45 errors in design-tokens'
validate-tokens.cjs.
• Added XMLHttpRequest + ProgressEvent to browser globals so
feedback-client compiles.
- packages/ui/eslint.config.js:
• Added @typescript-eslint/parser — the package-local override
replaced (didn't merge with) the root config, so TS syntax was
being parsed by espree and erroring on every `interface` /
type import.
• Added ignores for dist/** (root's ignores aren't inherited).
• Extended the files glob to .storybook/**/*.{ts,tsx}.
Mechanical lint fixes (no behaviour change):
- design-tokens/scripts/{validate,token-coverage}.cjs: empty catch
binding (catch (e) → catch).
- feedback-client/src/index.ts:
• captureScreen(): preserve caught error via `{ cause: err }`
on the rethrown Error (preserve-caught-error rule, real bug —
previous chain dropped the original stack).
• captureElement(): rename unused parity params mimeType/quality
to _mimeType/_quality and document why they exist.
- logger/__tests__/logger.test.ts: drop unused `LoggerConfig` import.
- extraction-service/{lib/circuit-breaker,modules/extract/{sidecar-
monitor,usage}}.test.ts: drop 3 unused vitest/type imports.
- tracker-web/__tests__/tracker-proxy.test.ts: rename unused local
`url` → `_url`.
New: docs/AUDIT_PLATFORM.md
Tooling-backed audit summary (pnpm install / typecheck / test / lint
results), classification of remaining lint debt by rule, and an
ordered hand-off plan for package owners to clear the rest with
`pnpm --filter <pkg> lint:fix` followed by an eyeball review.
Verified before commit:
- `pnpm typecheck` → pass (all 69 packages compile)
- `pnpm test` → pass (~2,200 tests across 18+ suites)
- `pnpm lint` → 85 pre-existing errors surfaced (none introduced
by this commit; all in unrelated packages — see AUDIT_PLATFORM.md
section P).
Out of scope (left untouched in working tree):
- In-progress nomgap-on-Vercel migration: docker-compose.ecosystem.yml,
products/nomgap/product.json, services/platform-service/src/
modules/flags/seed.ts.
- pnpm-lock.yaml: my `pnpm install -r` regenerated it (+2.9k/-8.5k
lines) — not part of the audit, owner should commit deliberately.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
46a16f06bc
commit
8f541c9f87
97
docs/AUDIT_PLATFORM.md
Normal file
97
docs/AUDIT_PLATFORM.md
Normal file
@ -0,0 +1,97 @@
|
|||||||
|
# Platform — Systematic Audit (cross-workspace)
|
||||||
|
|
||||||
|
Date: 2026-05-04
|
||||||
|
Tooling-backed audit (with `GITEA_NPM_TOKEN` available): full `pnpm install`,
|
||||||
|
typecheck, test, and lint run successfully across all 69 workspace packages
|
||||||
|
(`packages/`, `services/`, `dashboards/`).
|
||||||
|
|
||||||
|
Legend: 🔴 critical · 🟠 high · 🟡 medium · 🟢 low ·
|
||||||
|
⬜ open · 🟦 in PR · ✅ fixed (commit hash on the right).
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 0. Health snapshot
|
||||||
|
|
||||||
|
| Check | Result | Notes |
|
||||||
|
| ----------------- | ---------- | ------------------------------------------------------------------------------------------------------ |
|
||||||
|
| `pnpm install -r` | ✅ pass | 4 peer warnings — `@azure/cosmos` wants `@azure/core-client@^1.10.0`. Cosmetic. |
|
||||||
|
| `pnpm typecheck` | ✅ pass | All TS sources compile (`tsc --noEmit`). |
|
||||||
|
| `pnpm test` | ✅ pass | ~2,200 tests across 18+ test suites, all green. |
|
||||||
|
| `pnpm lint` | 🟡 partial | After this audit's structural fixes: still 85 pre-existing errors in code, 96 warnings. See section P. |
|
||||||
|
|
||||||
|
## A. Lint pipeline blockers (fixed by this audit)
|
||||||
|
|
||||||
|
| # | Issue | Severity | Status | Fix |
|
||||||
|
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :------: | :----: | ----------- |
|
||||||
|
| A1 | `packages/design-tokens/scripts/validate-tokens.cjs` — 45 `no-undef` errors for `process` / `console`. Root eslint config didn't declare a Node-script env, and inline `/* eslint-env node */` is ignored by flat-config. | 🟠 | ✅ | this commit |
|
||||||
|
| A2 | `packages/design-tokens/scripts/token-coverage.cjs` — same root cause: 1 unused `e` in catch. | 🟢 | ✅ | this commit |
|
||||||
|
| A3 | `packages/ui/eslint.config.js` was a _complete override_ (flat config doesn't merge with the root). It declared no parser, so 38 parsing errors fired on `interface`, `import {…}`, and other TS syntax in `src/index.ts` and `src/components/*`. | 🟠 | ✅ | this commit |
|
||||||
|
| A4 | `packages/ui` lint also re-included `dist/**/*.d.ts` because the root's `ignores: ['dist/**']` isn't inherited by the package-local override. | 🟡 | ✅ | this commit |
|
||||||
|
| A5 | `packages/ui/.storybook/preview.ts` not covered by any TS-parser block. | 🟡 | ✅ | this commit |
|
||||||
|
| A6 | `packages/feedback-client/src/index.ts` — 2 `no-undef` for browser globals `XMLHttpRequest` / `ProgressEvent` (not in root globals list). | 🟢 | ✅ | this commit |
|
||||||
|
| A7 | `packages/feedback-client/src/index.ts` — `preserve-caught-error` violation in `captureScreen()` (re-throwing without `cause`). | 🟡 | ✅ | this commit |
|
||||||
|
| A8 | `packages/feedback-client/src/index.ts` — `captureElement()` declares unused params `mimeType`, `quality`. Renamed with `_` prefix and documented why. | 🟢 | ✅ | this commit |
|
||||||
|
| A9 | `packages/logger/src/__tests__/logger.test.ts` — unused type import `LoggerConfig`. | 🟢 | ✅ | this commit |
|
||||||
|
| A10 | `services/extraction-service/src/lib/circuit-breaker.test.ts` — unused vitest import `afterEach`. | 🟢 | ✅ | this commit |
|
||||||
|
| A11 | `services/extraction-service/src/modules/extract/sidecar-monitor.test.ts` — unused type import `HealthCheck`. | 🟢 | ✅ | this commit |
|
||||||
|
| A12 | `services/extraction-service/src/modules/extract/usage.test.ts` — unused vitest import `beforeEach`. | 🟢 | ✅ | this commit |
|
||||||
|
| A13 | `dashboards/tracker-web/src/__tests__/tracker-proxy.test.ts` — unused local `url` (renamed to `_url`). | 🟢 | ✅ | this commit |
|
||||||
|
|
||||||
|
These all matter because `pnpm -r exec eslint` bails on the first package
|
||||||
|
failure, so the 45-error design-tokens issue was hiding everything below it.
|
||||||
|
Now the pipeline runs to completion and **surfaces the real lint debt** (next
|
||||||
|
section).
|
||||||
|
|
||||||
|
## P. Pre-existing lint debt now visible
|
||||||
|
|
||||||
|
After section A's structural unblocks the workspace-wide lint reports:
|
||||||
|
|
||||||
|
```
|
||||||
|
✖ 181 problems (85 errors, 96 warnings)
|
||||||
|
```
|
||||||
|
|
||||||
|
Errors break down by rule (top 4):
|
||||||
|
|
||||||
|
| Rule | Count | Notes |
|
||||||
|
| ----------------------------------- | ----: | ------------------------------------------------------------------ |
|
||||||
|
| `@typescript-eslint/no-unused-vars` | 67 | Mostly unused imports / unused destructured params in tests + code |
|
||||||
|
| `prefer-const` | 7 | `let` declarations never reassigned |
|
||||||
|
| `no-redeclare` | 7 | Likely identifiers shadowing globals / re-imports |
|
||||||
|
| `no-useless-escape` | 4 | Regex / string escapes that are no-ops |
|
||||||
|
|
||||||
|
All of these are autofixable for the most part (`pnpm lint:fix`) **except**
|
||||||
|
where renaming an unused var changes a public API surface. They span multiple
|
||||||
|
packages I don't have working knowledge of (auth, llm, cosmos, billing-service,
|
||||||
|
tracker-service, growth-service, etc.). I'm not fixing them blind in this pass
|
||||||
|
— each one is a one-liner but a wrong rename can break a downstream consumer.
|
||||||
|
|
||||||
|
**Recommendation**: a follow-up sweep where the package owner runs
|
||||||
|
`pnpm --filter <pkg> lint:fix`, eyeballs the diff, and commits per package.
|
||||||
|
Should be 6–10 small commits.
|
||||||
|
|
||||||
|
## W. Pre-existing lint warnings (96)
|
||||||
|
|
||||||
|
Mostly `no-console` in CLI tools (`create-app`, `keyvault`, `sidecar-monitor`,
|
||||||
|
`generate.ts`, `roadmap/page.tsx`) and one `no-explicit-any` in `api-client`.
|
||||||
|
These are intentional in CLI scripts (we already disabled `no-console` for
|
||||||
|
`**/scripts/**` in this audit's eslint config change), and the rest are case-by-
|
||||||
|
case judgment calls. Not blocking.
|
||||||
|
|
||||||
|
## R. Repo-state observations (not fixed)
|
||||||
|
|
||||||
|
| # | Observation | Severity | Status |
|
||||||
|
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | :------: | :----------------: |
|
||||||
|
| R1 | Working tree had 3 uncommitted edits when the audit started: `docker-compose.ecosystem.yml` (removes `nomgap-web` from Docker), `products/nomgap/product.json` (replaces flag set + adds containers), `services/platform-service/src/modules/flags/seed.ts` (+14 lines of flags). These look like an in-progress nomgap-on-Vercel migration. **Not touched** — out of audit scope and missing context. | — | ⬜ |
|
||||||
|
| R2 | Local `main` was 17 commits behind `origin/main` at the start of the session. Backup branch `backup/main-20260504-062733` was taken from `origin/main` (the source of truth) — local stale main was _not_ backed up. | 🟢 | ✅ (backup exists) |
|
||||||
|
| R3 | `.npmrc` references `${GITEA_NPM_TOKEN}` — pnpm prints a noisy WARN if the env var is unset. Cosmetic; the install still resolves the public packages it can. | 🟢 | ⬜ |
|
||||||
|
| R4 | `pnpm install -r` reports 4 missing peer warnings for `@azure/core-client@^1.10.0`. Adding it as an explicit dep on the two services that use `@azure/cosmos` would silence the warning. | 🟢 | ⬜ |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Ordering of fixes
|
||||||
|
|
||||||
|
1. **Section A** (this commit) — structural unblocks so `pnpm lint` runs end-to-end again.
|
||||||
|
2. **Section P** — per-package `lint:fix` sweeps owned by each package's
|
||||||
|
maintainer. Each package = one commit. ~6–10 commits.
|
||||||
|
3. **Section W** — case-by-case `no-console` review (defer; warnings only).
|
||||||
|
4. **Section R** — chore-level housekeeping (peer deps, .npmrc).
|
||||||
@ -106,6 +106,8 @@ export default [
|
|||||||
NodeList: 'readonly',
|
NodeList: 'readonly',
|
||||||
DOMRect: 'readonly',
|
DOMRect: 'readonly',
|
||||||
SVGElement: 'readonly',
|
SVGElement: 'readonly',
|
||||||
|
XMLHttpRequest: 'readonly',
|
||||||
|
ProgressEvent: 'readonly',
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
plugins: {
|
plugins: {
|
||||||
@ -137,4 +139,32 @@ export default [
|
|||||||
// 'sort-imports': ['error', { ignoreDeclarationSort: true }],
|
// 'sort-imports': ['error', { ignoreDeclarationSort: true }],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
|
// Node CJS scripts (build helpers, validators) and `**/scripts/**/*.js` —
|
||||||
|
// these run on Node directly and need the Node globals (process, console,
|
||||||
|
// require, module, __dirname). Without this block they trip 40+ no-undef
|
||||||
|
// errors and break `pnpm lint` for the whole workspace.
|
||||||
|
{
|
||||||
|
files: ['**/*.cjs', '**/scripts/**/*.js', '**/scripts/**/*.cjs'],
|
||||||
|
languageOptions: {
|
||||||
|
ecmaVersion: 2022,
|
||||||
|
sourceType: 'commonjs',
|
||||||
|
globals: {
|
||||||
|
process: 'readonly',
|
||||||
|
Buffer: 'readonly',
|
||||||
|
console: 'readonly',
|
||||||
|
require: 'readonly',
|
||||||
|
module: 'readonly',
|
||||||
|
exports: 'readonly',
|
||||||
|
__dirname: 'readonly',
|
||||||
|
__filename: 'readonly',
|
||||||
|
setTimeout: 'readonly',
|
||||||
|
clearTimeout: 'readonly',
|
||||||
|
setInterval: 'readonly',
|
||||||
|
clearInterval: 'readonly',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
rules: {
|
||||||
|
'no-console': 'off', // CLI scripts legitimately print to stdout
|
||||||
|
},
|
||||||
|
},
|
||||||
];
|
];
|
||||||
|
|||||||
@ -39,7 +39,7 @@ function findFiles(dir, files = []) {
|
|||||||
files.push(fullPath);
|
files.push(fullPath);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (e) {
|
} catch {
|
||||||
// Directory might not exist
|
// Directory might not exist
|
||||||
}
|
}
|
||||||
return files;
|
return files;
|
||||||
|
|||||||
@ -27,7 +27,7 @@ function findFiles(dir, files = []) {
|
|||||||
files.push(fullPath);
|
files.push(fullPath);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (e) {
|
} catch {
|
||||||
// Directory might not exist or be accessible
|
// Directory might not exist or be accessible
|
||||||
}
|
}
|
||||||
return files;
|
return files;
|
||||||
|
|||||||
@ -106,11 +106,7 @@ export class FeedbackClient {
|
|||||||
if (params.screenshot) {
|
if (params.screenshot) {
|
||||||
const sas = await this.generateSasUrl(params.screenshot.contentType);
|
const sas = await this.generateSasUrl(params.screenshot.contentType);
|
||||||
|
|
||||||
await this.uploadScreenshot(
|
await this.uploadScreenshot(sas.uploadUrl, params.screenshot.blob, onProgress);
|
||||||
sas.uploadUrl,
|
|
||||||
params.screenshot.blob,
|
|
||||||
onProgress
|
|
||||||
);
|
|
||||||
|
|
||||||
screenshotMeta = {
|
screenshotMeta = {
|
||||||
blobPath: sas.blobPath,
|
blobPath: sas.blobPath,
|
||||||
@ -132,7 +128,10 @@ export class FeedbackClient {
|
|||||||
appVersion: params.appVersion,
|
appVersion: params.appVersion,
|
||||||
platform: params.platform,
|
platform: params.platform,
|
||||||
screenshotBlobPath: screenshotMeta?.blobPath,
|
screenshotBlobPath: screenshotMeta?.blobPath,
|
||||||
screenshotContentType: screenshotMeta?.contentType as 'image/png' | 'image/jpeg' | 'image/webp',
|
screenshotContentType: screenshotMeta?.contentType as
|
||||||
|
| 'image/png'
|
||||||
|
| 'image/jpeg'
|
||||||
|
| 'image/webp',
|
||||||
screenshotSizeBytes: screenshotMeta?.sizeBytes,
|
screenshotSizeBytes: screenshotMeta?.sizeBytes,
|
||||||
deviceContext: params.deviceContext,
|
deviceContext: params.deviceContext,
|
||||||
}),
|
}),
|
||||||
@ -232,7 +231,8 @@ export class FeedbackClient {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const format = options.format || 'png';
|
const format = options.format || 'png';
|
||||||
const mimeType = format === 'png' ? 'image/png' : format === 'jpeg' ? 'image/jpeg' : 'image/webp';
|
const mimeType =
|
||||||
|
format === 'png' ? 'image/png' : format === 'jpeg' ? 'image/jpeg' : 'image/webp';
|
||||||
|
|
||||||
// If selector provided, capture specific element
|
// If selector provided, capture specific element
|
||||||
if (options.selector) {
|
if (options.selector) {
|
||||||
@ -285,7 +285,7 @@ export class FeedbackClient {
|
|||||||
const quality = mimeType === 'image/jpeg' ? 0.9 : undefined;
|
const quality = mimeType === 'image/jpeg' ? 0.9 : undefined;
|
||||||
const blob = await new Promise<Blob>((resolve, reject) => {
|
const blob = await new Promise<Blob>((resolve, reject) => {
|
||||||
canvas.toBlob(
|
canvas.toBlob(
|
||||||
(b) => b ? resolve(b) : reject(new Error('Canvas toBlob failed')),
|
b => (b ? resolve(b) : reject(new Error('Canvas toBlob failed'))),
|
||||||
mimeType,
|
mimeType,
|
||||||
quality
|
quality
|
||||||
);
|
);
|
||||||
@ -298,17 +298,22 @@ export class FeedbackClient {
|
|||||||
height: canvas.height,
|
height: canvas.height,
|
||||||
};
|
};
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
throw new Error(`Screen capture failed: ${err instanceof Error ? err.message : String(err)}`);
|
throw new Error(
|
||||||
|
`Screen capture failed: ${err instanceof Error ? err.message : String(err)}`,
|
||||||
|
{ cause: err }
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Capture specific DOM element using html-to-image approach
|
* Capture specific DOM element using html-to-image approach.
|
||||||
|
* `mimeType` and `quality` are accepted for API parity with captureScreen()
|
||||||
|
* but are not yet honoured here (this path always returns the fallback PNG).
|
||||||
*/
|
*/
|
||||||
private async captureElement(
|
private async captureElement(
|
||||||
selector: string,
|
selector: string,
|
||||||
mimeType: string,
|
_mimeType: string,
|
||||||
quality?: number
|
_quality?: number
|
||||||
): Promise<CaptureResult> {
|
): Promise<CaptureResult> {
|
||||||
const element = document.querySelector(selector);
|
const element = document.querySelector(selector);
|
||||||
if (!element) {
|
if (!element) {
|
||||||
@ -330,8 +335,8 @@ export class FeedbackClient {
|
|||||||
// This is a simplified implementation
|
// This is a simplified implementation
|
||||||
throw new Error(
|
throw new Error(
|
||||||
'Element capture requires html2canvas library. ' +
|
'Element capture requires html2canvas library. ' +
|
||||||
'Please install: npm install html2canvas ' +
|
'Please install: npm install html2canvas ' +
|
||||||
'Then use: html2canvas(element).then(canvas => canvas.toBlob(...))'
|
'Then use: html2canvas(element).then(canvas => canvas.toBlob(...))'
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -365,12 +370,15 @@ export class FeedbackClient {
|
|||||||
const capture = await this.captureScreenshot(screenshotOptions);
|
const capture = await this.captureScreenshot(screenshotOptions);
|
||||||
|
|
||||||
// Submit with captured screenshot
|
// Submit with captured screenshot
|
||||||
return this.submitWithScreenshot({
|
return this.submitWithScreenshot(
|
||||||
...params,
|
{
|
||||||
screenshot: {
|
...params,
|
||||||
blob: capture.blob,
|
screenshot: {
|
||||||
contentType: capture.contentType,
|
blob: capture.blob,
|
||||||
|
contentType: capture.contentType,
|
||||||
|
},
|
||||||
},
|
},
|
||||||
}, onProgress);
|
onProgress
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -4,7 +4,7 @@
|
|||||||
|
|
||||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||||
import { createLogger } from '../logger.js';
|
import { createLogger } from '../logger.js';
|
||||||
import type { Logger, LoggerConfig } from '../types.js';
|
import type { Logger } from '../types.js';
|
||||||
|
|
||||||
describe('createLogger', () => {
|
describe('createLogger', () => {
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
|||||||
@ -1,7 +1,23 @@
|
|||||||
|
// Package-local override for @bytelyst/ui.
|
||||||
|
// Flat config does NOT inherit the root file when ESLint discovers a config
|
||||||
|
// here, so the parser must be declared explicitly or ESLint falls back to
|
||||||
|
// the espree default which can't parse TypeScript syntax (`interface`,
|
||||||
|
// type-only imports, etc.) and emits 38 parse errors.
|
||||||
|
import tsparser from '@typescript-eslint/parser';
|
||||||
|
|
||||||
export default [
|
export default [
|
||||||
|
// Ignore build artifacts (root config's ignores aren't inherited).
|
||||||
{
|
{
|
||||||
files: ['src/**/*.{ts,tsx}'],
|
ignores: ['dist/**', 'node_modules/**', 'coverage/**'],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
files: ['src/**/*.{ts,tsx}', '.storybook/**/*.{ts,tsx}'],
|
||||||
languageOptions: {
|
languageOptions: {
|
||||||
|
parser: tsparser,
|
||||||
|
parserOptions: {
|
||||||
|
ecmaVersion: 2022,
|
||||||
|
sourceType: 'module',
|
||||||
|
},
|
||||||
globals: {
|
globals: {
|
||||||
HTMLButtonElement: 'readonly',
|
HTMLButtonElement: 'readonly',
|
||||||
HTMLSpanElement: 'readonly',
|
HTMLSpanElement: 'readonly',
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user