Copilot review instructions
August 2, 2026 · View on GitHub
AGENTS.mdis the source of truth. This file is a review-focused distillation of it, kept in sync by hand — see "Keep documentation files up to date" inAGENTS.md. Where the two disagree,AGENTS.mdwins, and the drift is a bug worth flagging in review.
The Inspector ships as one package with three clients (Web, CLI, TUI) over a shared core/, consumed via the @inspector/core build-time alias. v2 is not an npm workspace: the root and each clients/* keep their own package.json and node_modules.
TypeScript
- Never use
any. Not in types, not in casts, not in generics. - Never suppress errors to satisfy the linter or compiler — no disabling
no-unused-vars/no-explicit-anyin config, and no// @ts-nocheckor// @ts-ignore(@typescript-eslint/ban-ts-commentrejects these across every surface). - Avoid double casts (
as unknown as T). They erase all type safety and usually mean the real type is being worked around. Prefer a type guard, a narrower single cast, or fixing the underlying type. If genuinely unavoidable (a documented gap in a third-party type, or bridging structurally-identical shapes TS can't relate), it must carry an inline comment justifying why it's safe and why nothing better exists. An unjustifiedas unknown asis not acceptable in review. - Prefer inference, type guards, and precise annotations over assertions.
- An
_prefix is the intentionally-unused marker (argsIgnorePattern/varsIgnorePattern/caughtErrorsIgnorePattern).
React and UI (web client)
The web client is built from presentational ("dumb") components — they take data and callbacks as props and hold display logic only. No data fetching or client state inside them; state comes from the @inspector/core hooks wired near the top of the tree. A component that reaches for a store or fetches directly is a review finding.
Styling is Mantine-first, in this strict order of preference: component props → theme variants → CSS classes (last resort).
- Never use inline styles.
- Never use raw color literals — no hex (
#ddd), norgba(). Use the--inspector-*CSS custom properties fromApp.css :root(e.g.c: 'var(--inspector-text-primary)'). If no token fits, add one to:rootfirst. - Avoid
divand bare HTML for layout. Use MantineBox,Group,Stack,Flex,Paper. - Never add a CSS class when the styles can be component props or a theme variant. Flat CSS properties (margin, padding, background, border, color, font-size) belong in the theme (
src/theme/<Component>.ts, viaComponent.extend()).App.cssmay contain only what the theme cannot express:@keyframes, pseudo-selectors (:hover,:focus), cross-component hover relationships, nested child selectors for third-party HTML output, and styles for native elements (img,iframe). - When a theme variant needs a class for nested/pseudo selectors, assign it via
classNamesin the theme extension — never a manualclassNamein JSX for theme-styled components.
The .withProps() rule
Declare a named subcomponent constant via .withProps() whenever an inline Mantine element carries two or more static props. This applies to single-use elements too — "it's only used once" is not an exemption.
- Static = a literal configuring styling, layout, or behavior:
size="sm",c="dimmed",fw={500},gap="xs",justify="space-between",variant="light",withBorder,readOnly,striped. - Not counted: dynamic props (
value,on*,children,key,ref, anything whose value is a variable) — pass these at the call site; and per-instance content/accessibility literals (label,description,placeholder,title,aria-label,role) — these never by themselves trigger extraction.
const CardContent = Group.withProps({
flex: 1,
align: "flex-start",
justify: "space-between",
wrap: "nowrap",
});
Legitimate exceptions (each stays inline, with a one-line comment saying why):
Box— does not support.withProps(). UseGroup/Stack/Flex/Text/Paper/UnstyledButton/Imageinstead, chosen by purpose. ABoxthat genuinely needs a non-flex primitive (component="iframe",display="grid") stays inline.Accordion— a compound,multiple-discriminated generic;.withProps()loses its JSX call signature and fails to type.- Headless, non-
factory()components such asTransition— no Styles API, so no.withPropsstatic at all. data-*attributes — not part of a component's typed props, so excess-property-checked out of awithPropsliteral. Pass at the call site.- Anything that isn't a Mantine factory component — a
react-iconsglyph, another library's component, or a first-party plainexport function.
State and effects
- Never reset or re-sync local state from a prop inside a
useEffect.useEffect(() => setX(prop), [prop])paints the stale value first and renders twice; it is an error underreact-hooks/set-state-in-effect. - Use
useValueChange(value, onChange)(src/hooks/useValueChange.ts) — React's documented "adjusting state during render" pattern. It does not fire on the first render; seed the state withuseState. The comparison isObject.is, so pass a referentially stable value — a primitive key (id/name/URI) or a memoized one, never a fresh object literal. - The
onChangeruns during render, so it must be pure —setStateand nothing else. No fetches, DOM writes, logging, ref mutation, or parent callbacks; a render can be replayed or abandoned. - Effects remain correct for real external-system synchronization (DOM measurement, rAF, subscriptions, timers).
Theme files vs. element components
Both exist and do different jobs. Theme files (src/theme/<Component>.ts) customize a Mantine primitive app-wide. Element components (src/components/elements/) add domain semantics on top of primitives.
- Element components import from
@mantine/core, not fromsrc/theme/— the theme layer is applied transparently by the provider. - Never push domain-specific variant logic into theme files (annotation types, transport types, …). Domain variants belong to the element component that owns those semantics.
Where code goes (web client)
utils = functions that compute; lib = things that instantiate, adapt, or touch the environment. If it does I/O or wraps a subsystem it's lib; if it's a pure transform it's utils.
src/utils/— pure, side-effect-free. Also: pure shared domain types and their constructors; diagnosticconsole.warn/errordoes not count as a side effect; type-only imports from@inspector/core, and re-exporting pure functions/constants from core, are both fine.src/lib/— infrastructure, integration, stateful adapters: composes subsystems, wraps the core runtime, touches DOM /window/sessionStorage, or produces side effects.- Cross-directory imports go one way:
lib → utils, never the reverse. src/types/is only for ambient.d.tsmodule stubs — not a home for new domain types.- ⚠️ The coverage
includeis a whitelist namingcomponents/hooks/theme/lib/utils/server(plus thecore/*runtime). A module placed outside those directories silently falls out of the ≥90% gate. Flag new top-level files or new grab-bag directories.
Tests and the coverage gate
- All new or modified code needs tests. The per-file gate is ≥ 90% on all four dimensions — lines, statements, functions, and branches — enforced in CI for
clients/{web,cli,tui,launcher}and the gatedcore/runtime. - Never lower the gate to accommodate an unreachable branch. Annotate at the source with a justified
/* v8 ignore … -- <reason> */. Acceptable reasons: happy-dom-inherent paths (Mantine portal mounts,useMediaQueryfallbacks,typeof windowSSR guards), React StrictMode effect-replay blocks, and provably-dead defensive guards. Anything else is a missing test. - Suppress expected error output from the console in tests that exercise error paths.
Test placement
- Web: side-by-side by default —
<Name>.test.tsxnext to the source. A web-owned test undersrc/test/instead of beside its source is a bug.src/test/is only for what can't be co-located: tests of the repo-rootcore/package (src/test/core/…, mirroring core's layout), theintegrationproject (src/test/integration/…— placement is the manifest), and shared test infrastructure. - CLI / TUI / launcher: all tests live in a top-level
__tests__/directory. A co-locatedsrc/**/*.test.*lands in no tsconfig project and failsverify:typecheck-coverage.
Rendering components in tests
- Always render through
renderWithMantinefromsrc/test/renderWithMantine.tsx. Never hand-roll a bareMantineProvider— that reintroduces a real failure class where aTransition/Modaltimer fires after happy-dom tears downwindowand fails the entire run. - For a forced color scheme, pass the option —
renderWithMantine(ui, { colorScheme: "dark" })— rather than a hand-rolleddefaultColorSchemeprovider. - Only when asserting mid-flight transition state, use
renderWithMantineTransitions, passingsettleMsderived from the component's real animation duration. Do not combine it withvi.useFakeTimers(), and use theunmount()it returns if the test unmounts the tree itself.
Gates and PR hygiene
npm run formatbefore committing;npm run cibefore pushing (validate→coverage→verify:build-gate→smoke→ Storybook).npm run validateis the fast inner-loop check and is not a substitute — it runstest, nottest:coverage, so it does zero coverage gating.- Every PR references an issue, first body line
Closes #<ISSUE_NUMBER>. - Every PR carries exactly one version label,
v1orv2, matching its base branch. - Update the relevant
README.md/AGENTS.mdwhen a change adds, removes, renames, or repurposes a file or folder, changes the structure or tech stack, or introduces a command, dependency, or architectural pattern.
What to prioritize in review
- Correctness and security — this backend spawns local processes and proxies outbound requests, so anything touching auth, origin validation, host binding, or the proxy's SSRF controls deserves close reading.
- Type-safety violations (
any, suppressions, unjustified double casts). - Missing or thin tests against the ≥90% four-dimension gate, and modules placed outside the gated directories.
- Mantine convention violations — inline styles, raw colors, unnecessary CSS classes, missing
.withProps()extraction. - Docs that contradict the change.