PR #16234 - onyx-dot-app/onyx

Description

This is the bottom of a stack of six PRs. Together they make coding agents (and humans) reuse Opal and refresh-components, not design UI from scratch.

#16234 (this PR): enforces the design-system and layout rules with oxlint, and adds an agent hook that makes agents migrate a file to Opal when they edit it. The hook works in Claude Code, Codex and Cursor.
#16235: adds a component catalog and a skill, so agents know which components exist before they build.
#16240, #16241, #16242, #16243: fix the existing violations (icons, raw elements, replaced refresh-components and refresh Text).

Problem

When agents do frontend work in web/, they often skip the existing components. They hand-write buttons, cards, chips, rows and empty states that Opal already has. They import lucide-react or react-icons. They also keep using refresh-components that Opal has replaced, such as the deprecated boolean-flag Text.

The rules are already written down in web/AGENTS.md ("Buttons: Button from @opal/components. No raw <button>", "Icons: only @opal/icons", "Never import from web/src/components/"). But nothing enforced them:

web/.oxlintrc.json had no no-restricted-imports and no react/forbid-elements config.
A raw <button> or a lucide-react import passed lint, the pre-commit hooks and CI.
Rules written as prose in an agents file are advisory. Agents follow them some of the time, but not reliably.

Current violation counts on main, before this PR:

Violation	Files
Raw `<button>`, `<input>`, `<select>`, `<textarea>` (outside Opal)	17 app files, plus 16 in `lib/opal/src` (expected there)
`lucide-react` / `react-icons` imports	20
`@/components/*` (legacy) imports	119
Imports of refresh-components that Opal replaces	155 import sites

What this PR changes

1. New oxlint rules (`web/.oxlintrc.json`)

Rule	Level	What it catches	Message points to
`react/forbid-elements`	error	`<button>`, `<input>`, `<select>`, `<textarea>`	`Button`, `InputTypeIn` / `InputCheckbox`, `InputSingleSelect`, `InputTextArea` from `@opal/components`
`no-restricted-imports`	error	`lucide-react`, `react-icons`, `react-icons/*`	`@opal/icons`
`no-restricted-imports`	error	`@/components/*`, except `@/components/icons/icons` (`createLogoIcon`, the exception that `web/AGENTS.md` names)	`@opal/`, then `@/refresh-components/`, then `@/sections/*`
`opal/no-replaced-refresh-import` (new)	warn	Imports of refresh-components that Opal replaces	The Opal replacement

Scope:

lib/opal/src/**: react/forbid-elements is off, because Opal implements these primitives and must render the raw elements. The import rules stay on, so Opal itself cannot import lucide-react or @/components/*.
src/components/**: both rules are off. The folder is legacy and is being deleted, so a lint pass there would be churn.
Tests, specs, __tests__, __mocks__, tests/** and stories: both rules are off. They often need raw elements or fixtures.

2. A warn-only baseline for existing violations

94 files outside the exempt paths already broke the error rules. I did not migrate them in this PR. Instead, a "design-system baseline" override in .oxlintrc.json lists them, and there the two rules only warn. This is the same per-path ratchet idea that the file already uses for i18n/no-raw-jsx-text, the other way round:

New code gets errors at once.
Existing code keeps building.
A file comes off the list when it is clean. No file gets added (the override comment says so).

Two oxlint details a reviewer should know:

The override repeats the full rule options, not only "warn". An override that only sets a severity ("no-restricted-imports": "warn") resets the rule's options to the default, which is empty. The rule then restricts nothing, so the baseline files were silently not checked at all. I found this in testing. Now the baseline files show their real violations as warnings.
Next.js dynamic-route segments are escaped ([id] becomes [[]id]), because oxlint override globs treat [...] as a character class. Without the escape, the bracketed paths did not match, and 14 errors came back.

3. New oxlint plugin: `web/tools/oxlint/opal/`

It has one rule, no-replaced-refresh-import, built the same way as the existing anti-slop and i18n plugins (eslintCompatPlugin + defineRule, package.json with exports: ./index.ts). It holds a map from each refresh-components module to its Opal replacement:

refresh-components module	Opal replacement	Import sites today
`@/refresh-components/texts/Text`	`Text` (string-enum `font` and `color` props)	130
`@/refresh-components/texts/Truncated`	`OverflowText`	11
`@/refresh-components/cards/Card`	`Card`	7
`@/refresh-components/buttons/LineItem`	`LineItemButton`	4
`@/refresh-components/Divider`	`Divider`	1
`@/refresh-components/buttons/SelectButton`	`SelectButton`	1

How I picked the mapping: I compared the refresh-components file list with the @opal/components exports, and then read both sides (JSDoc, README and props) to confirm that the Opal component does the same job. Text and Card are already marked @deprecated in favour of Opal.

Left out on purpose: @/refresh-components/buttons/Tag. It is a display or editable chip with a different API, while Opal Tag is a colored status label, so a mechanical migration would change the UI. To add a row later, edit the map in rules/no-replaced-refresh-import.ts. #16235 reads the same map, so the catalog follows.

This rule only warns, because 155 import sites cannot all move in one PR. The hook below is what drives the migration.

4. Agent hook: `.agents/hooks/opal_migrate_on_edit.py`

The hook is for every agent that works in this repo:

Agent	Config	Event and matcher	How it gets the edited files	Feedback field the model reads
Claude Code	`.claude/settings.json`	`PostToolUse`, `Edit\|MultiEdit\|Write`	`tool_input.file_path`	`hookSpecificOutput.additionalContext`
Codex	`.codex/hooks.json`	`PostToolUse`, `apply_patch`	The `* Add/Update File:` and `* Move to:` lines in the patch text (`tool_input.command`)	`hookSpecificOutput.additionalContext`
Cursor	`.claude/settings.json` (Cursor imports it by default; the repo's existing Stop hook already relies on this)	`postToolUse`, `Write`	`file_path` (top level or in `tool_input`)	`additional_context`

The per-agent parts are in the new shared module .agents/hooks/agent_hook_io.py:

edited_files() finds the edited paths in any of the three payload shapes.
give_feedback() writes the output shape that each agent reads.
repo_root() takes CLAUDE_PROJECT_DIR, then CURSOR_PROJECT_DIR, then Cursor's workspace_roots, then the payload cwd.

The hook gives feedback as context with exit code 0, so it never blocks or undoes an edit.

I did not add a .cursor/hooks.json. With Cursor's default import on, it would run each hook twice.

What the hook does:

It reads the edited file_path from the hook payload. It returns 0 at once for anything that is not a .ts/.tsx file under web/, or when web/node_modules/.bin/oxlint does not exist (for example, before bun install).
It runs oxlint -f json <file> from web/. This uses the real repo config, so the hook and pre-commit cannot disagree. It takes about 0.2s per file.
It keeps the diagnostics from the three design-system rules:
- errors, always;
- warnings from opal/no-replaced-refresh-import, react/forbid-elements and the icon import restrictions;
- but not the @/components/* warnings in baseline files. Many legacy components have no replacement yet, and forcing an agent to port one in the middle of an unrelated edit would derail the task.
If it found any, it gives them to the agent with the line number and the lint help text, plus a pointer to the Opal READMEs and "Do not change behavior". The agent acts on them at once.
If the file is in a baseline (design-system or layout) and has no diagnostics left for that baseline's rules, it tells the agent to remove the file from that baseline. So the ratchet tightens by itself.
It also reports errors from the two layout rules (section 5). It does not report layout-baseline warnings when an agent touches a baseline file. Turning a flex div into a Section changes its defaults (centered, full width and height, gap={4}), so it is not a mechanical swap. Forcing it on every edit to 327 files would derail unrelated work. To turn it on, delete one if in _is_reported.

This puts the "migrate on touch" convention into practice: an agent cannot edit a file and leave the replaced refresh imports behind.

Why a hook in addition to the lint:

The existing Stop hook (pre_commit_on_stop.py) already runs oxlint at the end of a turn, but only for errors. Warnings do not fail pre-commit, so the migration warnings would never reach the agent.
At the end of a turn, the agent has already written a whole feature the wrong way. Feedback after each edit is cheaper to act on.

web/AGENTS.md gets one paragraph on the rules, the baseline and the hook.

5. Layout rules in the `opal` plugin

Two more rules come from a local hook that has run since the summer. They are the lint version of the "everything inline, vertically and horizontally" layout principle. Raw <button> and <input> are the obvious violations, but most layout drift comes from hand-rolled flex divs, and those break the inline alignment.

Layout rule 1: `opal/no-layout-div` (error)

The rule flags a <div> or <span> whose className makes it a layout container: flex, inline-flex, grid, inline-grid, items-*, justify-*, gap-* or gap-x/y-*. The message points to Section from @opal/layouts, then to Content, ContentAction, Card, InputHorizontal/InputVertical and Divider.

Section takes className, a ref and event handlers, so there is no reason to drop to a raw element for a container.
Class names are read from string literals, template literal text, cn(...)/clsx(...) arguments, conditionals, logical expressions, arrays and object keys (web/tools/oxlint/opal/shared/class-names.ts).
Variant prefixes are stripped before matching, so md:flex counts.
A flex item class (flex-1, flex-col alone, shrink-0) does not count. Only container classes do.
Exempt: lib/opal/src/** (Opal builds the primitives from raw elements), src/components/** (legacy) and tests and stories.

Baseline: 1,035 sites in 327 files break the rule on this branch. A second "layout baseline" override in .oxlintrc.json lists those files, and there the rule only warns. As with the design-system baseline (section 2), a file comes off the list when it is clean, and no file gets added. New files get the error.

Layout rule 2: `opal/no-section-utility-classes` (error)

It flags classes on Section that its props own. It detects Section from all three import paths: @opal/layouts, @opal/layouts/general/components and @/layouts/general-layouts, which re-exports the Opal Section.

Classes	Why it flags them	Message points to
`p-`, `px-`, `py-`, `pt-`, `pr-`, `pb-`, `pl-`, `ps-`, `pe-`, `gap-`, `gap-x-`, `gap-y-` (with or without a variant prefix)	Dead. `Section` always sets `gap` and `padding` as inline styles (`spacingToRem` returns `"0rem"` for 0), and an inline style beats any class	`gap={n}`, `padding={n}` (N / 4 rem), or margin classes for asymmetric spacing
`w-`, `h-`, `size-*` with no variant prefix	Not dead, but they bypass the props. `cn` uses `tailwind-merge` and puts `className` last, so a `w-40` does override the default `w-full`	`width` / `height` (`"auto" \| "fit" \| "full"` or a number in rem)

What it allows on purpose:

min-* and max-* classes. Section has no props for them, and min-w-0/min-h-0 (30 sites today) are the standard fix for flex shrinking.
Responsive size classes such as sm:w-auto. The props cannot set a width per breakpoint, and Opal's own Modal footer uses them.
Margin classes, for asymmetric spacing.

A correction to the rule's original idea: size classes on Section do not "fight" its defaults. tailwind-merge resolves them, and the class wins. That is why rule 2 has a separate message for size classes.

Silent bugs the layout rules found

Rule 2 found 12 dead padding classes in 7 files. None of these classes ever applied, so these Sections render with zero padding in production today. I deleted the classes, so the rendered output stays exactly the same:

File	Dead class(es)
`src/ee/views/admin/HooksPage/HookFormModal.tsx`	`px-0.5`
`src/ee/views/admin/HooksPage/HookLogsModal.tsx`	`py-1`, `py-2`, `py-6`
`src/views/admin/connectors/AddConnectorPage/components/CredentialChecksCard.tsx`	`pt-1`
`src/views/admin/GroupsPage/SharedGroupResources/index.tsx`	`p-0.5` (`shrink-0` kept)
`src/views/admin/connectors/IndexingStatusPage/ConnectorRow.tsx`	`expanded ? "px-2 pt-2 pb-1" : "p-2"`
`src/views/admin/connectors/ConnectorDetailPage/RunsCard.tsx`	`px-2 pb-2`

The authors meant to add this padding. Reviewers, especially for the indexing pages: if you want the spacing that was intended, set it with the padding prop, or with margins on the children for asymmetric values. That is a visible change, so I did not make it here.

Known gaps

Grid: the rule flags grid as the spec asked, but Opal has no grid layout. 38 baseline sites use grid. A new grid site has no Opal replacement until Opal adds a grid primitive.
Inline spans: Section renders a div, which cannot sit inside a <p>. An inline-flex span inside text has no Opal replacement yet.

Options I rejected

Prose rules only: this is the state today. It does not work reliably.
A // oxlint-disable comment in each violating file: this edits about 100 files, and it hides the violations from the hook. The baseline list keeps the debt visible in one place.
Making no-replaced-refresh-import an error with its own baseline: 155 sites in about 130 files would mean a second huge list for little gain. A warning plus the hook gives the same result on touched files.
Hand-rolled grep checks in the hook: they would drift from the lint config. Running oxlint keeps one source of truth.

Risks and trade-offs

Bigger diffs for small edits. 130 files import refresh Text. Any agent edit to one of them now also migrates every Text use in that file. This is intentional, but reviewers will see migration noise in otherwise small PRs while the debt goes down.
Humans do not get the hook. For humans, oxlint warnings show in the editor. Errors block commits through the existing oxlint pre-commit hook and in CI.
Cursor was not tested live. The Cursor CLI is not installed on my machine. I tested the Cursor payload and output shapes with sample stdin only. Cursor's docs do not list the field names of tool_input for Write, so edited_files() tries file_path, path, target_file and filePath.
Codex hook trust. Codex runs project hooks only after the user trusts them, and the trust is tied to each hook's hash. After this PR merges, Codex users accept the new hook once.
A loop the agent cannot fix. If an agent cannot migrate a file (for example, a needed Opal prop is missing), it will see the same message after each edit. The message is advisory and the agent can explain and move on. If this happens often, we can add a stop_hook_active-style escape like the one in pre_commit_on_stop.py.

How to review

web/tools/oxlint/opal/rules/no-replaced-refresh-import.ts: is each mapping a correct replacement?
web/.oxlintrc.json, top-level rules: the rule configs and messages.
web/.oxlintrc.json, overrides: the exemptions (tests, Opal, src/components), then the baseline. The baseline is a generated list, so you can skim it.
.agents/hooks/opal_migrate_on_edit.py: the filtering in _is_reported and the baseline-cleanup branch.
.agents/hooks/agent_hook_io.py: the payload parsing for each agent.
.claude/settings.json and .codex/hooks.json: one new PostToolUse entry in each.
web/tools/oxlint/opal/rules/no-layout-div.ts, no-section-utility-classes.ts and shared/class-names.ts: the layout rules (section 5). Then the layout baseline at the end of .oxlintrc.json (generated; skim it) and the 7 files where dead Section classes were removed.

Follow-ups (not in this PR)

Add a grid layout primitive to Opal, so opal/no-layout-div has a replacement for grid (38 sites).
Decide on the intended padding for the 7 files with dead Section classes (section 5).
Burn down the baseline and the 155 refresh imports. The hook does this file by file, but a dedicated sweep of Text would go faster.
Add @deprecated JSDoc to Truncated, Divider, SelectButton and LineItem, so editors also strike them through.

How Has This Been Tested?

Full lint run: oxlint on all of web/ gives 0 errors. Design-system warnings: 155 opal(no-replaced-refresh-import), 78 no-restricted-imports and 58 react(forbid-elements), all in baseline files. A script confirmed that every file in the baseline still has at least one hit, so there are no dead entries.
Probe file: I created src/sections/__LintProbe.tsx with lucide-react, react-icons/fi, @/components/ErrorCallout, createLogoIcon from @/components/icons/icons, refresh Text and a raw <button>. I got 4 errors with the right help messages and the refresh-Text warning. createLogoIcon was allowed. Then I deleted the file.
Baseline options bug: before the fix, oxlint src/app/admin/agents/CollapsibleSection.tsx reported nothing for its react-icons/fi import. After the fix, it reports a warning with "Use icons from @opal/icons."
Hook, with sample stdin:
- src/app/craft/components/BuildWelcome.tsx (imports refresh Text): exit 2, and the message names Text from @opal/components.
- src/app/admin/agents/CollapsibleSection.tsx (baseline file, react-icons): exit 2 with the icon message.
- lib/opal/src/utils.ts (clean): exit 0, no output.
- backend/onyx/main.py: exit 0 (out of scope).
Live runs in the real CLIs, on a probe file that imports refresh Text:
- claude -p (Claude Code 2.1.303): after an Edit, the model quoted the hook's additionalContext with the Opal Text replacement.
- codex exec (Codex 0.163.0): after an apply_patch, the model quoted the same message.
Sample stdin for all three payload shapes: Claude Code tool_input.file_path, the Codex apply_patch patch text and the Cursor conversation_id + workspace_roots payload. Each one gives the right output field. Invalid JSON exits 0 with no output.
Layout rules probe file with:
- a flex div;
- a Section with px-2 w-40 sm:w-auto min-w-0 mt-2;
- a span with cn("md:inline-flex", "text-03");
- a div with flex-1 p-2.
Result: errors on the flex div, the md:inline-flex span, px-2 and w-40. No errors on sm:w-auto, min-w-0, mt-2 or the flex-1 p-2 flex item. The hook reported the same 4 problems.
Hook on a layout-baseline file with only layout warnings: no message, as designed.
Full oxlint run on this branch, rebased on the latest main: 0 errors. 1,035 opal(no-layout-div) warnings, all in layout-baseline files. 0 opal(no-section-utility-classes) diagnostics.
Every branch above this one (#16235, #16240 to #16243) also has 0 lint errors after the rebase. bun run types:check at the top of the stack reports no type errors (99.04% coverage).
pre-commit passes on all changed files (oxlint, oxfmt, ruff, feature-map integrity, typescript-check).

Additional Options

[Optional] Please cherry-pick this PR to the latest release version.

Summary by CodeRabbit

Developer Tooling
- Added automated checks for layout patterns, component spacing, legacy imports, and restricted elements.
- Editing web files can surface selected code-quality findings and identify files that are clean of configured baseline rules.
- Added these checks to supported editing workflows, with status messages describing the checks being run.
- Documented layout and component-spacing standards for web development.
Style
- Updated spacing in several admin pages to use default section spacing.