Coding conventions
Style and structural rules the codebase follows. Frontend rules are extracted from review feedback on PR series #3591–#3596; backend rules are extracted from the provider/repository/service layering used across server/src/service/infrastructure/network/* and the patterns enforced in code review. These are non-negotiable for new code. They exist to keep the codebase greppable, maintainable, and consistent.
If you're touching a file in
client/srcthe frontend rules apply. If you're touching a file inserver/srcthe backend rules apply. Rule 0 applies everywhere.
0. Look at existing implementations and follow the established pattern
Before writing new code, find one or two files that already do something similar — a peer provider for a new monitor type, a peer page for a new admin screen, a peer repository method for a new query — and follow the same shape. Match imports, helper-function placement, naming, error handling, mock-construction style, and assertion idioms. If you're tempted to do it differently, the burden is on you to justify the divergence.
Why: consistency is more valuable than locally-optimal cleverness. A reader who has internalized the pattern of one peer file should be able to read the new one with no extra mental load. New shapes also make refactors more expensive — when someone wants to change the convention later, every variant has to be visited and reasoned about separately.
How to apply: before writing a new component, page, provider, repository, or test file:
- Grep for the closest existing peers (e.g.
WebSocketProvider.tsif you're writing a new monitor provider). - Read at least two of them end-to-end.
- Mirror their structure: same constants at the top (
SERVICE_NAME,TIMEOUT_MS), same import order, same DI shape, same try/catch envelope, same test-file location and helper imports. - If your new code needs a shape no peer has, name it explicitly in the PR description and link to the peer you diverged from. Don't sneak novelty in.
Concrete check before opening a PR: can a reader open a peer file side-by-side with yours and trace each section in the same order? If not, restructure.
Part 1 — Frontend (client/src)
1. Prefer MUI native (system) props over sx
MUI components accept layout, spacing, color, border, typography and sizing values as top-level props. Use those. Reach for sx only when no native prop exists for the value you need (e.g. textDecoration, pseudo-selectors, complex selectors, conditional CSS).
Why: native props are easier to read in JSX, easier to grep, easier to refactor, and don't force the reader to mentally parse a nested object literal for things that are conceptually one attribute. A sx block also can't be split-line-edited cleanly when most of its keys are simple primitives.
How to apply: any time you write sx={{ … }}, ask whether each key has a native equivalent. If yes, hoist it.
Canonical mappings
sx key |
Native prop |
|---|---|
color |
color |
backgroundColor |
bgcolor |
borderRadius |
borderRadius |
border, borderTop, … |
border, borderTop, … |
width, height, minHeight, maxWidth |
same names as native |
p, pt, pb, px, py |
same names as native |
m, mt, mb, mx, my |
same names as native |
display |
display |
textAlign |
textAlign |
fontWeight, fontSize, lineHeight |
same names as native |
gap (on Stack) |
gap |
Examples
// BAD — everything inside sx
<Box
sx={{
width: "100%",
p: theme.spacing(LAYOUT.MD),
borderRadius: 1,
border: `1px solid ${alpha(tone, 0.45)}`,
backgroundColor: alpha(tone, 0.08),
textAlign: "left",
}}
/>
// GOOD — every key that has a native prop is hoisted
<Box
width="100%"
p={theme.spacing(LAYOUT.MD)}
borderRadius={theme.shape.borderRadius}
border={`1px solid ${alpha(tone, 0.45)}`}
bgcolor={alpha(tone, 0.08)}
textAlign="left"
/>
// BAD
<Typography sx={{ color: theme.palette.text.primary, lineHeight: 1.55, fontSize: 13 }}>
// GOOD — hoist what you can; drop fontSize:13 because it's the body default
<Typography
color={theme.palette.text.primary}
lineHeight={1.55}
>
When sx is still correct
Keep sx for things that don't have a native prop:
<Link
component={RouterLink}
to="/settings"
color={theme.palette.primary.main}
fontWeight={600}
sx={{ textDecoration: "underline" }} // OK — no native prop for textDecoration
/>
Pseudo-selectors ("&:hover"), transition, cursor, outline, overflow (when not on Box/Stack), media queries, and conditional spread objects all stay in sx.
2. Use the full theme path for colors, never the shorthand string
Prop forms like color="text.secondary", color="primary.main", bgcolor="background.paper" work, but they are forbidden in this codebase.
Why: the shorthand is invisible to grep. If a future change needs to find every place that consumes theme.palette.text.secondary — to retheme, to swap, to audit — the shorthand strings are missed. That is a maintenance trap. It's worth a few extra characters at every callsite to keep the codebase searchable.
How to apply: always destructure or reference the theme value explicitly.
// BAD
<Typography color="text.secondary">…</Typography>
<Typography color="primary.main">…</Typography>
// GOOD
const theme = useTheme();
<Typography color={theme.palette.text.secondary}>…</Typography>
<Typography color={theme.palette.primary.main}>…</Typography>
This applies to every theme-aware string prop (color, bgcolor, borderColor, etc.), and inside sx if you really need it there.
3. No hardcoded numeric or string literals — use tokens
The theme exposes tokens for every kind of magic value. Use them. Reserve raw numbers for values that are genuinely one-off (and even then, push back on yourself first).
Why: tokens are the single source of truth. A literal 2 in a mt prop is invisible — it could mean LAYOUT.XXS, a rounding correction, or a thinko. The reader can't tell. With a token, intent is in the name.
How to apply: before writing a literal, check whether one of these tokens covers it. If it nearly does but not exactly, extend the token rather than hardcoding (Alex did exactly this — he added LAYOUT.XXS = 2 rather than letting mt: "2px" stand).
Token sources
- Spacing / layout —
client/src/Utils/Theme/constants.tsLAYOUT.XXS = 2·LAYOUT.XS = 4·LAYOUT.SM = 6·LAYOUT.MD = 8·LAYOUT.LG = 10·LAYOUT.XL = 12·LAYOUT.XXL = 16SPACINGfortheme.spacing(...)multipliers
- Typography sizes —
client/src/Utils/Theme/Palette.ts→typographyLevelsxs / s / m / l / xl / xxl(rem values)
- Hover / interaction —
HOVER.DARKENetc. - Shape —
theme.shape.borderRadius(don't writeborderRadius: 1orborderRadius: 4) - Colors —
theme.palette.*(rule 2 above)
Examples
// BAD
<Box sx={{ mt: "2px" }} />
<Stack spacing={theme.spacing(2)} />
<Box sx={{ borderRadius: 1 }} />
<Typography fontSize={18} />
<Typography fontSize={13} /> // 13 is the body default — drop the prop entirely
// GOOD
<Box mt={LAYOUT.XXS} />
<Stack spacing={theme.spacing(LAYOUT.XXS)} />
<Box borderRadius={theme.shape.borderRadius} />
<Typography fontSize={typographyLevels.xl} />
<Typography /> // inherits 13px from theme
Token naming gotcha
typographyLevels.xl was historically used for both 18px and 23px callsites. Don't reuse a token for two different purposes — split it (xl = 18, xxl = 23) and update every callsite. If you find an ambiguous token while editing, fix it the same way.
4. Use useTheme() inside components, don't import the theme module
The theme object exported from @/Utils/Theme/Theme is the factory output, not a live, mode-aware instance. Importing it directly couples a component to the wrong reference and breaks dark-mode reactivity. It also breaks the build at import time in some module-graph configurations (this happened in PR #3596 and required a follow-up commit).
Why: themes can change at runtime (mode toggle), and the hook is the only correct way to read the active theme. Imports also create circular-graph risk because Theme.ts itself depends on Palette.ts.
How to apply: inside any component or component-helper that needs theme values:
// BAD
import { theme } from "@/Utils/Theme/Theme";
const SectionHeading = ({ children }) => (
<Typography color={theme.palette.text.secondary}>{children}</Typography>
);
// GOOD
import { useTheme } from "@mui/material/styles";
const SectionHeading = ({ children }) => {
const theme = useTheme();
return <Typography color={theme.palette.text.secondary}>{children}</Typography>;
};
Module-level utility files that don't render React (pure helpers) should accept theme as a parameter rather than importing it.
5. Discriminated unions: pair the runtime list with the type
When you have a closed set of string literals that's used both as a type and as runtime data (severities, statuses, kinds), declare the runtime tuple first and derive the type from it. Don't declare them as two parallel literals that can drift.
Why: if the list and the type aren't tied to each other, adding a new value to one and forgetting the other compiles fine and breaks at runtime. Deriving the type from the tuple makes them inseparable.
// BAD
type Severity = "info" | "warning" | "error";
// GOOD
export const Severities = ["info", "warning", "error"] as const;
export type Severity = (typeof Severities)[number];
Export both when callers need to iterate (e.g. for a Storybook stories matrix or a select dropdown).
Part 2 — Backend (server/src)
6. Layering: Controller → Service → Repository → Mongoose
The backend enforces a strict three-layer separation between HTTP handling, business logic, and data access. Every new feature follows this path, no shortcuts.
Request → Controller → Service → Repository → MongoDB (Mongoose)
Why: this is what makes each layer independently testable. Mongoose calls in services force services to mock the database in tests; raw DB queries in controllers couple HTTP shape to schema shape and turn every refactor into a multi-file rewrite.
How to apply: when adding a new feature, the pattern is — add a repository method for any new DB query, call it from a service, expose it via a controller. If a service constructs a MonitorModel.findOne({...}), you've broken the rule; the query belongs in the repository.
- Controllers (
server/src/api/controllers/) handle HTTP concerns only. Each method is annotatedHandler(fromcontrollerUtils.ts), parses the request with a Zod schema, calls the service with the parse result, and writesres.json({ success, msg, data }).Handlertypes the response body as the envelope, so any other shape is a compile error. No wrapper: Express 5 forwards a rejected promise tohandleErrors. No business logic. - Services (
server/src/service/business/) contain all business logic. They throwAppErroron validation/auth failures; they do not write directly to MongoDB. - Repositories (
server/src/repositories/) are the sole layer that talks to MongoDB. They expose clean query methods (findByMonitorId,createCheck,updateStatusWindowAndChecks) and return entities — never Mongoose documents — via atoEntityprivate helper.
7. Provider conventions for service/infrastructure/network/*
Every monitor-type provider implements IStatusProvider<TPayload> and follows the same skeleton.
Why: the registry in services.ts builds providers identically (new XxxProvider(deps)) and NetworkService invokes them identically (provider.handle(monitor)). If your provider's shape diverges, the registry/dispatch becomes asymmetric and NetworkService grows special cases.
How to apply: copy the structure of a recent peer (WebSocketProvider, GrpcProvider, PortProvider). The skeleton:
const SERVICE_NAME = "XxxProvider";
const TIMEOUT_MS = 10000;
type ClientCtor = typeof SomeNodeStdlibClass;
export class XxxProvider implements IStatusProvider<XxxStatusPayload> {
readonly type = "xxx";
constructor(private Client: ClientCtor) {}
supports(type: MonitorType): boolean {
return type === "xxx";
}
async handle(monitor: Monitor): Promise<MonitorStatusResponse<XxxStatusPayload>> {
try {
// 1. validate inputs → throw AppError(400) on bad input
// 2. set up the client (use this.Client, never imported singleton)
// 3. perform the call inside timeRequest(...), with a setTimeout race
// in the Promise body to enforce TIMEOUT_MS
// 4. return MonitorStatusResponse with payload
} catch (err: unknown) {
if (err instanceof AppError) throw err;
const originalMessage = err instanceof Error ? err.message : String(err);
throw new AppError(internalError, {
message: originalMessage || "Error performing xxx check",
service: SERVICE_NAME,
details: { /* monitor-shaped context */ },
});
}
}
}
Hard rules
- DI for all external dependencies.
typeof WebSocket,typeof net— pass the constructor in via the provider's constructor. Neverimport { Foo } from "some/library"and call it insidehandle(). This is the single thing that makes the provider unit-testable withoutjest.unstable_mockModule. SERVICE_NAMEandTIMEOUT_MSas module-level constants near the top, not magic strings/numbers in the body.- Use the shared
timeRequest()helper fromservice/infrastructure/network/utils.tsto measure latency. Don't roll your ownprocess.hrtime(). - Enforce a hard timeout. Every peer provider inlines a
setTimeoutrace in the Promise body around the underlying client call (seeWebSocketProvider.ts); sockets, DNS resolvers, and external clients can hang indefinitely without it. There is no sharedwithTimeouthelper today — match the inline pattern. - Outer try/catch that converts unknown errors to
new AppError(internalError, { service, details })while letting your ownAppErrorpass through.
8. Mongoose schema fields with closed value sets
If a field's TypeScript type is a closed union (status, type, record type), the Mongoose schema must declare the same set as enum.
Why: the validator and the schema are independent gates. A service-layer Zod check can be bypassed (imports, migrations, direct repo writes). A schema enum is the last line of defense and surfaces invalid data as a Mongoose validation error rather than silently accepting "badtype" and breaking downstream consumers.
How to apply: when you add a string field whose values are constrained by a const tuple in types/, reuse the same const for the schema enum:
// types/monitor.ts (existing)
export const MonitorMatchMethods = ["equal", "include", "regex"] as const;
// db/models/Monitor.ts
{
matchMethod: {
type: String,
enum: MonitorMatchMethods,
},
}
The same applies to MonitorTypes, MonitorStatuses, and any new closed-set field.
9. Test conventions for server/test/unit/providers/network/*
Each provider gets a unit test file at server/test/unit/providers/network/<providerName>.test.ts. Follow the structure used by webSocketProvider.test.ts, grpcProvider.test.ts, portProvider.test.ts.
Why: consistency lets reviewers diff a new test file against any peer and only see the actual logic differences, not framework/import noise. It also lets the testStatusProviderContract helper guarantee every provider satisfies the IStatusProvider contract.
How to apply:
-
Imports:
from "@jest/globals"(not the globaljest), and.tsextensions on every relative import. -
Contract test: call
testStatusProviderContract("XxxProvider", { create, supportedType, unsupportedType, makeMonitor })before the per-testdescribeblock. -
Mock construction: build the constructor mock as
jest.fn().mockImplementation(() => mock) as unknown as typeof Client(typed cast, notas any). -
makeMonitorfactory: takesPartial<Monitor>overrides, spreads onto a baseline, returnsas Monitor. -
Inline
setup()per test rather thanbeforeEachshared state. Each test should construct its own mock + provider, so a single test can be read in isolation:const setup = () => { const mock = createMockClient(); const Ctor = jest.fn().mockImplementation(() => mock) as unknown as typeof Client; const provider = new XxxProvider(Ctor); return { mock, provider }; }; it("does X", async () => { const { mock, provider } = setup(); // ... }); -
Assertion style: individual
expect(result.field).toBe(...)lines orexpect(result).toEqual(expect.objectContaining({...})). Both are used; pick what reads cleanest for the case. -
Test naming: lowercase imperative ("returns success when connection opens", "uses the configured timeout", "throws AppError when host is missing").
10. Validation: centralize, don't inline
Whenever the same string-set or shape constraint appears in two or more validators, lift it into types/ as a const tuple and import it into both validators. Don't inline z.enum(["A", "AAAA", ...]) in three places.
Why: drift. Three inline copies will eventually disagree — someone adds a new value to one and forgets the other two. The validator silently accepts it on POST /monitors and rejects it on PATCH, leading to bug reports the author can't reproduce.
How to apply: keep validation source-of-truth in server/src/types/*.ts as paired const tuples + derived types (rule 5). The validators in server/src/validation/*.ts and the Mongoose schemas in server/src/db/models/*.ts import from types/ and reuse the same tuple.
11. OpenAPI: entity schemas live in the domain
Each entity's shape is one Zod schema in domain/<entity>/<entity>.schema.ts. The TS type is z.infer of it in <entity>.type.ts, the route table references it as the response, and the OpenAPI spec is generated from it. Entities with a secret field (User, Proxy, Monitor) also export a *ResponseSchema that is .omit() of the entity schema and carries the component id. Shapes a route composes (lists, pages, envelopes) live in api/validation/<entity>Validation.ts.
Why: the spec is auto-generated. If the schema doesn't list a field, SDK consumers don't see it, even though the server returns it. One definition means the type, the mapper and the spec cannot drift from each other.
How to apply: when you add an entity field, update two files in the same PR:
domain/<entity>/<entity>.schema.ts— add the field. Required or optional follows what the repository'stoEntitywrites. If the field is a secret, add it to the response schema's.omit().domain/<entity>/<entity>.model.ts— add the Mongoose schema entry (withenumif the value set is closed; rule 8).
The repository's toEntity must write the new field; the build fails until it does. Zod outside api/validation/ is for response shapes only: request parsing stays in the validators, and services never call .parse on a response schema.
12. Every AppError is built from a definition
Client errors are { status, description } literals in domain/<entity>/<entity>.errors.ts (middleware errors in api/middleware/middleware.errors.ts, email errors in service/email.errors.ts), thrown as new AppError(<entity>Errors.<name>, { service, message?, details? }) and listed by reference in the route table's errors. Server errors throw internalError from utils/AppError.ts. The OpenAPI spec is generated from the definitions, and npm run test:route-errors fails when a route omits a definition its handler can throw, or lists one it cannot.
Why: the status and the spec description come from the same value, so they cannot drift. A reviewer sees every error a route can return by reading its table entry.
How to apply: when you add a throw:
- Add the definition to the entity's catalog if none fits. Reuse an existing one when the meaning is the same;
monitorErrors.notFoundis thrown from several repositories. - Throw it with
serviceandmethod. Passmessageonly when it adds something the description cannot, such as an id. Otherwise the description is the message. - Add it to the
errorsarray of every route entry whose handler can reach it, including entries in other domains' tables. - Run
npm run test:route-errors.
Guard errors (validation 400, auth 401, role 403, upload 413 and 415, rate limit 429, and 500) are derived from the entry's fields by registerRoutes and are never listed.
Pre-PR checklist
Before opening a PR, grep your diff for the patterns below. Each item describes the anti-pattern to find and fix; if your diff has none, that line passes. Frontend section applies to client/src; backend to server/src.
Universal
Frontend (client/src)
Backend (server/src)
A clean PR on these axes is a PR that ships without a normalize follow-up.