diff --git a/.agents/skills/README.md b/.agents/skills/README.md index d53facec3b..93f13011ae 100644 --- a/.agents/skills/README.md +++ b/.agents/skills/README.md @@ -6,6 +6,7 @@ This directory contains all Claude Code skills for the web-widgets repository. ``` .agents/skills/ +├── audit-widget/ # Audit one widget into a report (tests, perf, deps, security, smells, behaviours, docs) ├── code-review/ # PR review skill for Mendix widget conventions ├── debug-widget/ # Debug Mendix pluggable widget runtime issues ├── openspec-apply-change/ # Implement tasks from OpenSpec change @@ -26,6 +27,7 @@ This directory contains all Claude Code skills for the web-widgets repository. Skills are automatically loaded by Claude Code. Users can invoke them via slash commands: +- `/audit-widget [widget]` — Audit one widget and write the report to its `AUDIT.md` - `/code-review` — Review current PR - `/debug-widget` — Debug widget runtime issues - `/openspec-*` or `/opsx:*` — OpenSpec workflow commands diff --git a/.agents/skills/audit-widget/SKILL.md b/.agents/skills/audit-widget/SKILL.md new file mode 100644 index 0000000000..e37f34b2ad --- /dev/null +++ b/.agents/skills/audit-widget/SKILL.md @@ -0,0 +1,84 @@ +--- +name: audit-widget +description: Audit one Mendix pluggable widget and write a point-in-time report (AUDIT.md) on unit tests, e2e, performance, dependencies, security, code smells, missing behaviours and mx-docs drift. Report only — it never changes code. +--- + +# Audit Widget + +Writes a health report for **one** widget package. The skill reads code and runs read-only commands. The report is its only output. + +## Invocation + +``` +/audit-widget [] +``` + +`` is a folder name under `packages/pluggableWidgets/` (e.g. `combobox-web`). If it is missing or matches no folder, ask which widget. Do not guess. + +Every run is a fresh snapshot. If `AUDIT.md` already exists, replace it without reading it first: audit from scratch so old findings do not anchor the new ones. + +## Hard rules + +- **The report is the only file you write.** Never edit source, tests, config, `package.json`, docs, or the mx-docs repo. Output that commands write on their own (Jest coverage) is fine. +- **Install nothing.** Never run `pnpm install`, `pnpm add`, `pnpm up` or `pnpm audit --fix`, and never run a script that downloads a tool. They change `package.json` or `pnpm-lock.yaml`, and the repo does not allow lockfile changes. +- **No git writes.** Never `git add`, commit, branch or push. The user decides what to do with the report. +- **Stay in the package.** Read files under `packages/pluggableWidgets//` only. Follow a finding into a shared `@mendix/widget-plugin-*` package only when the widget's own code cannot explain it, and record it in Notes, not as a widget finding. +- **mx-docs is read-only.** Read it from `$MX_DOCS_PATH`, else `~/Projects/mx-docs`. If neither exists, skip `DOCS` and say why in Notes. + +## 1. Orient + +```bash +cd packages/pluggableWidgets/ +git rev-parse --short HEAD # record in the report header +``` + +Read, in this order: + +1. `package.json` — deps, scripts, `widgetName`, `marketplace` +2. `AGENTS.md` if present — the widget's own architecture map, the highest-value context you will get +3. `src/.xml` — the property contract; this is the source of truth for expected behaviour +4. `CHANGELOG.md` — recent behaviour changes +5. `openspec/specs/**` and `openspec/changes/**` — declared requirements and in-flight work +6. Inventory (do not read contents yet): + ```bash + find src -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.scss" \) | sort + find src -path "*__tests__*" -name "*.spec.*" | sort + ls e2e/*.spec.js + ``` +7. The mx-docs page for the widget — locate it with the recipe in the `DOCS` section of [checklists.md](checklists.md) and record its path. Do not read it in full yet. + +## 2. Run the eight checks + +Detection heuristics for each category are in [checklists.md](checklists.md). Read it now — it is the substance of this skill. + +| Category | ID prefix | What it answers | +| ------------------ | --------- | ---------------------------------------------------------------------------- | +| Unit tests | `UT` | Which modules/branches/states have no Jest+RTL coverage? | +| E2E tests | `E2E` | Which user-visible flows have no Playwright spec, or a fragile one? | +| Performance | `PERF` | What causes avoidable renders, work-in-render, or leaks? | +| Dependencies | `DEP` | Which of this widget's own deps are outdated or have known advisories? | +| Security | `SEC` | Where can untrusted data reach HTML, URLs, scripts or other dangerous sinks? | +| Code smells | `SMELL` | Violations of repo conventions, dead code, type escapes | +| Missing behaviours | `BEH` | XML/API/a11y contract the implementation does not honour | +| Public docs | `DOCS` | Where the mx-docs page disagrees with the XML, changelog, or code | + +If your agent supports read-only subagents (for example Claude Code's `Explore`), run one per category with the package path and that category's section of `checklists.md`, then merge the results. Run `DEP` yourself — it needs commands, not a search. + +## 3. Verify before writing + +Every finding must survive this filter, or it does not go in the report: + +- **Evidence**: a `file:line` you actually read, or command output you actually ran. +- **Failure scenario**: concrete inputs/config → wrong, slow or unsafe result. "Could be cleaner" is not a finding. +- **Not already handled**: check the code path once more for the guard you are claiming is absent. +- **Not a shared-package concern** misattributed to the widget. +- **Not handled by a library**: when the finding depends on how a framework or library behaves (for example, a callback captured by `useEditor(..., [])`), read that library's code in `node_modules` before reporting it. `@tiptap/react` forwards callbacks to the latest options, which once made a stale-callback finding a false positive. +- **Critical needs proof**: report a finding as Critical only with proof you produced — a failing existing test, command output, or for a leaked secret the line itself. Otherwise report it as High and add _needs repro_. + +Drop speculative findings silently. A short true report beats a long plausible one. + +## 4. Write the report + +Write `packages/pluggableWidgets//AUDIT.md` using [report-template.md](report-template.md). Then print to the user: the category table, the finding counts per severity, and the 3 findings you would fix first. + +**Stop there.** diff --git a/.agents/skills/audit-widget/checklists.md b/.agents/skills/audit-widget/checklists.md new file mode 100644 index 0000000000..824d96a435 --- /dev/null +++ b/.agents/skills/audit-widget/checklists.md @@ -0,0 +1,275 @@ +# Detection heuristics per category + +One section per audit category. Each finding you report must trace back to a concrete signal here (or a defensible variation of one) plus `file:line` evidence. + +Severity scale used throughout: + +- **Critical** — users hit broken/incorrect behaviour, data loss, or an exploitable security hole +- **High** — a real defect or a gap that lets defects ship undetected +- **Medium** — degraded UX/perf, or a convention violation with maintenance cost +- **Low** — cleanup, nice-to-have coverage + +--- + +## UT — Missing unit tests + +Target: `src/**/__tests__/*.spec.{ts,tsx}` run by `pnpm run test` (Jest + RTL). + +**Build the coverage map first.** For every file under `src/` that exports something, note whether any spec imports it. Optionally run `pnpm run test --coverage` for real numbers; if the toolchain rejects the flag, fall back to import analysis and say the numbers are structural, not measured. + +Then look for these specific gaps: + +| Signal | Severity | +| --------------------------------------------------------------------------------------------------------------------- | -------- | +| Entry component (`src/.tsx`) has no spec at all | High | +| A `src/helpers/**` class or pure function has no spec | High | +| A custom hook in `src/hooks/**` has no spec | High | +| `.editorConfig.ts` `getProperties` / `check` / `getPreview` untested | Medium | +| Branch untested: each mutually exclusive XML enum value (e.g. selection mode, datasource type) that changes rendering | High | +| State untested: loading, unavailable/placeholder, empty/no-results, read-only, validation-message, disabled | High | +| `ActionValue` prop with no test asserting `canExecute === false` blocks `execute()` | High | +| `EditableValue.setValue` path with no test asserting the value written | High | +| Keyboard handler with no test for its keys (Arrow/Enter/Space/Escape/Tab/Home/End) | High | +| ARIA attributes / roles rendered but never asserted | Medium | +| Snapshot exists but no behavioural assertions accompany it | Medium | +| Cleanup untested: listener/timer/observer added in an effect, no unmount assertion | Medium | +| Spec exists but only covers the happy path of a multi-branch function | Medium | +| Test uses real timers/`setTimeout` sleeps instead of fake timers | Low | + +Prefer `@mendix/widget-plugin-test-utils` builders and helpers (`EditableValueBuilder`, `ListValueBuilder`, `actionValue`, `dynamic`, `list`, `obj`) when proposing new tests — check how sibling specs in the package do it and match. + +--- + +## E2E — Missing e2e flows + +Target: `e2e/*.spec.js` run via `@mendix/run-e2e`. Rules: `docs/requirements/e2e-test-guidelines.md`. + +**Enumerate the surface first.** Widget features from the XML + the pages in the test project. Use mxcli only if it is already installed — the script downloads it otherwise. From the repo root: + +```bash +[ -x automation/tools/mxcli ] && bash automation/mxcli/mx-testproject.sh inspect # modules, pages, entities, .mx-name-* labels +``` + +If mxcli is not installed, say so in Notes and infer the page inventory from the selectors in `e2e/*.spec.js`. + +Coverage gaps: + +| Signal | Severity | +| ---------------------------------------------------------------------------------- | -------- | +| A test-project page exercising this widget has no spec referencing it | High | +| A primary interaction (select, filter, sort, open/close, upload, drag) has no spec | High | +| Full keyboard navigation flow untested end-to-end | High | +| No `toHaveScreenshot` coverage for a visually-driven widget | Medium | +| Dark-mode / design-property variants rendered but never screenshotted | Low | +| Error/validation path never driven through the UI | Medium | +| Feature added in a recent `CHANGELOG.md` entry with no matching spec | High | + +Fragility gaps (report these as E2E findings too — a flaky spec is worse than a missing one): + +| Signal | Severity | +| ---------------------------------------------------------------------- | -------- | +| Import from `@playwright/test` instead of `@mendix/run-e2e/fixtures` | High | +| `page.waitForTimeout(...)` | High | +| `waitForLoadState("networkidle")` | High | +| `waitForMendixApp()` called manually right after `page.goto()` | Low | +| `.nth(N)` on an ambiguous selector | Medium | +| Bare `text=` / `page.click("text=…")` locator | Medium | +| `allTextContents()` / `$$eval` / `evaluate` used to build an assertion | Medium | +| Text-content correctness asserted in e2e rather than a unit test | Low | +| `press("Escape")` on a popup page | Medium | +| Manual login / manual logout in `afterEach` | Low | + +--- + +## PERF — Performance issues + +Read the render path top-down from the entry component. Concrete, not vibes: each finding names what re-renders or what work repeats. + +| Signal | Severity | +| ------------------------------------------------------------------------------------------------------------------------- | -------- | +| Object/array/function literal passed as a prop into a `memo`'d child (kills the memo) | High | +| `useEffect`/`useMemo` dependency array containing a value recreated every render | High | +| Class/selector/provider instance constructed in the render body instead of `useMemo`/`useRef`/state initialiser | High | +| Expensive work in render: sort, filter, `match-sorter`, regex build, date formatting over a full list | High | +| Deriving state from props via `useState` + `useEffect` instead of computing during render | Medium | +| MobX-observable state read in a component not wrapped in `observer` (extra renders or missed updates) | High | +| Missing `useCallback` on a handler passed to a large list's items | Medium | +| List of unbounded length rendered without virtualisation or a cap | Medium | +| `key` derived from index on a reorderable/filterable list | Medium | +| Layout read (`getBoundingClientRect`, `offsetWidth`) in an effect that also writes style — forced sync reflow | Medium | +| `ResizeObserver`/`scroll`/`resize` listener without throttle, or without cleanup | High | +| Effect that calls `setValue`/`execute` on every render tick | Critical | +| Timer/interval/listener/observer with no cleanup in the effect return | High | +| Non-tree-shakable import (`import _ from "lodash"`, deep barrel re-export) — repo requires tree-shakable imports | Medium | +| Large asset or icon inlined per render instead of a stable module-level constant | Low | +| SCSS: deeply nested selectors, or animation on a non-composited property (`width`/`top` instead of `transform`/`opacity`) | Low | + +For each PERF finding, state the trigger: "on every keystroke", "on every parent render", "once per row per render". + +--- + +## DEP — Dependencies + +**Only this widget's `package.json`.** Report only: never run `pnpm up`, `pnpm add`, `pnpm install` or `pnpm audit --fix` — they rewrite `package.json` or `pnpm-lock.yaml`. + +Outdated versions, from the package root: + +```bash +pnpm outdated +``` + +Known advisories. `pnpm audit` always scans the whole workspace, so keep only the paths that start at this widget: + +```bash +pnpm audit --prod --json 2>/dev/null | node -e ' +const w = process.argv[1], d = JSON.parse(require("fs").readFileSync(0, "utf8")); +for (const a of Object.values(d.advisories ?? {})) { + const paths = a.findings.flatMap(f => f.paths).filter(p => p.startsWith(`packages__pluggableWidgets__${w}>`)); + if (paths.length) console.log(a.severity, a.module_name, a.github_advisory_id, a.patched_versions, paths[0]); +}' +``` + +Classify each outdated result against the **range declared in this widget's `package.json`**: + +- **patch** — `x.y.Z`; safe, batch them together — Low +- **minor** — `x.Y.z`; read the changelog for behaviour changes — Low +- **major** — `X.y.z`; breaking; propose on its own, never bundled with other work — Low, or Medium when the installed major no longer gets fixes + +Each advisory is its own row with kind `advisory`. Its severity follows the advisory (critical → Critical, high → High, moderate → Medium, low → Low); lower it one level when the vulnerable code is not reachable from the widget, and say why in Risk. + +Rules: + +- Skip every `workspace:*` / `workspace:^*` dependency — those move with the monorepo, not with this widget. Advisories reached only through a shared package go in Notes. +- `react` / `react-dom` are supplied by the Mendix client and pinned repo-wide to `>=18.0.0 <19.0.0`. Never propose a React 19 bump, and flag any dep whose new version requires React 19. +- `@mendix/pluggable-widgets-tools` is `*` — do not propose pinning it. +- Check the new version still supports the widget's `marketplace.minimumMXVersion` browser baseline; flag ESM-only majors, since the widget is bundled by Rollup for the Mendix client. +- Renovate manages the repo (`.github/renovate.json`). If a bump is already in an open Renovate PR, say so in Risk. + +--- + +## SEC — Security + +Trace untrusted data to dangerous sinks. Untrusted means anything a user or app data can control: attribute values, text templates, datasource items, pasted or dropped content, URL parameters, `postMessage` events. For each finding, name the source, the sink, and an input that exploits it (for example `` or `javascript:alert(1)`). + +| Signal | Severity | +| ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------- | +| Untrusted HTML reaches `dangerouslySetInnerHTML`, `innerHTML`, `outerHTML`, `insertAdjacentHTML` or `document.write` without a sanitizer (DOMPurify or equivalent) | High | +| Sanitizer present but configured to allow `