fix(rack): keep a rack height edit from orphaning its mounts

The number input's min/max are hints the browser does not enforce on a
typed value, and updateRack wrote the patch straight through. Two ways
out of a usable canvas:

- Shrinking below a mounted device left the plate drawn above the
  chassis, outside the React Flow node, with nothing in the UI able to
  drag it back. freeUnits only counts 1..uHeight, so the modal's "used
  of" label under-counted it in silence.
- A height over 100 made RackSave reject every save with a 422, so both
  the explicit Save and each autosave tick reported "Save failed" with
  no cause.

updateRack now clamps to [MIN_RACK_U, MAX_RACK_U] and relocates the
mounts a shrink pushes past the top rail, one at a time so two never
land on the same slot. It returns false — changing nothing — when one
has nowhere to go, and the modal says so.

Two server-side guards that would have contained it: RackSaveRequest
cross-checks every mount against the rack it names, and RackDeviceSave
checks col_start + col_span against the grid, which each field passing
its own bounds never caught.

Also normalizes the MAC on POST /scan/pending. Every other write path
canonicalizes it and dedup compares by equality, so a hand-typed
AA-BB-CC-11-22-33 never matched the scanned aa:bb:cc:11:22:33 and
approve built a duplicate node.

ha-relevant: maybe
This commit is contained in:
Pouzor
2026-08-09 20:35:23 +02:00
committed by Pouzor - Rémy Jardient
parent 4d2128c494
commit cd6402a680
9 changed files with 272 additions and 10 deletions
+5 -1
View File
@@ -15,6 +15,7 @@ from app.db.database import AsyncSessionLocal, get_db
from app.db.models import Design, Edge, Node, PendingDevice, PendingDeviceLink, ScanRun
from app.schemas.nodes import NodeCreate
from app.schemas.scan import PendingDeviceCreate, PendingDeviceResponse, ScanRunResponse
from app.services.mac_utils import normalize_mac
from app.services.node_dedupe import dedupe_nodes_by_ieee, find_duplicate_node
from app.services.scanner import DeepScanOptions, _valid_port_range, request_cancel, run_scan
from app.services.zigbee_service import (
@@ -335,7 +336,10 @@ async def create_pending(
device = PendingDevice(
hostname=body.hostname,
ip=body.ip,
mac=body.mac,
# Canonical form, like every other write path: dedup compares MACs by
# equality, so a hand-typed "AA-BB-CC-11-22-33" would never match the
# scanned "aa:bb:cc:11:22:33" and approve would build a duplicate node.
mac=normalize_mac(body.mac),
suggested_type=body.suggested_type,
model=body.model,
vendor=body.vendor,
+38 -1
View File
@@ -1,6 +1,6 @@
from typing import Any
from pydantic import BaseModel, field_validator
from pydantic import BaseModel, field_validator, model_validator
# Kept in sync with the frontend rack model (frontend/src/types).
WIDTH_STANDARDS = {"19", "10"}
@@ -79,6 +79,20 @@ class RackDeviceSave(BaseModel):
raise ValueError(f"col_span must be between 1 and {RACK_COLUMNS}")
return v
@model_validator(mode="after")
def _fits_the_column_grid(self) -> "RackDeviceSave":
"""`col_start` and `col_span` are legal apart and still illegal together.
11 + 12 spans to column 23 of a 12-column grid — each field passes its
own check, and the frontend serializer clamps them independently, so
nothing caught it before this.
"""
if self.col_start + self.col_span > RACK_COLUMNS:
raise ValueError(
f"col_start + col_span must not exceed the {RACK_COLUMNS}-column grid"
)
return self
class RackCableSave(BaseModel):
id: str
@@ -123,6 +137,29 @@ class RackSaveRequest(BaseModel):
# Pan/zoom, stored on the design's shared CanvasState row like the logical canvas.
viewport: dict[str, Any] = {}
@model_validator(mode="after")
def _devices_fit_their_rack(self) -> "RackSaveRequest":
"""A mount must fit inside the rack it names.
`RackDeviceSave` validates each field in isolation and never sees the
rack, so `u_start: 40` in a 12U rack passed. A mount above the top rail
draws outside the chassis and nothing in the UI can drag it back, so the
server refuses to store it. Rack membership itself is checked in the
route, which knows what is already persisted.
"""
heights = {r.id: r.u_height for r in self.racks}
for device in self.devices:
u_height = heights.get(device.rack_id)
if u_height is None:
continue # unknown rack — the route reports that with a 400
if device.u_start + device.u_height - 1 > u_height:
raise ValueError(
f"Device {device.id} does not fit in rack {device.rack_id}: "
f"U {device.u_start}–{device.u_start + device.u_height - 1} "
f"of {u_height}U"
)
return self
class RackResponse(BaseModel):
id: str
+29
View File
@@ -260,3 +260,32 @@ async def test_update_scan_config_persists_deep_scan(client: AsyncClient, header
)
assert res.status_code == 200
assert saved == {"http_ranges": ["9000-9100"], "probe": True}
@pytest.mark.asyncio
async def test_create_pending_normalizes_the_mac(client: AsyncClient, headers):
# Dedup compares MACs by equality, so a hand-typed entry in any other
# notation would never match the scanned row and approve would build a
# duplicate node.
res = await client.post(
"/api/v1/scan/pending",
json={
"hostname": "printer",
"mac": "AA-BB-CC-11-22-33",
"discovery_source": "manual",
},
headers=headers,
)
assert res.status_code == 201, res.text
assert res.json()["mac"] == "aa:bb:cc:11:22:33"
@pytest.mark.asyncio
async def test_create_pending_keeps_a_missing_mac_null(client: AsyncClient, headers):
res = await client.post(
"/api/v1/scan/pending",
json={"hostname": "no-mac", "discovery_source": "manual"},
headers=headers,
)
assert res.status_code == 201, res.text
assert res.json()["mac"] is None
+46
View File
@@ -226,6 +226,52 @@ class TestSaveAndLoad:
res = await client.post("/api/v1/racks/save", json=payload, headers=headers)
assert res.status_code == 422
async def test_rejects_a_device_taller_than_its_rack(self, client: AsyncClient, headers):
# Each field is legal on its own; only the rack says otherwise. A mount
# above the top rail draws outside the chassis with no way to drag it back.
design_id = await _design(client, headers)
payload = _state(design_id)
payload["devices"][0]["u_start"] = 40 # rack is 12U
res = await client.post("/api/v1/racks/save", json=payload, headers=headers)
assert res.status_code == 422
async def test_rejects_a_device_whose_height_overruns_the_rack(
self, client: AsyncClient, headers
):
design_id = await _design(client, headers)
payload = _state(design_id)
payload["devices"][0].update({"u_start": 11, "u_height": 4}) # 11..14 of 12U
res = await client.post("/api/v1/racks/save", json=payload, headers=headers)
assert res.status_code == 422
async def test_accepts_a_device_that_ends_on_the_top_rail(
self, client: AsyncClient, headers
):
design_id = await _design(client, headers)
payload = _state(design_id)
payload["devices"][0].update({"u_start": 11, "u_height": 2}) # 11..12 of 12U
res = await client.post("/api/v1/racks/save", json=payload, headers=headers)
assert res.status_code == 200, res.text
async def test_rejects_a_span_that_overruns_the_column_grid(
self, client: AsyncClient, headers
):
design_id = await _design(client, headers)
payload = _state(design_id)
payload["devices"][0].update({"col_start": 11, "col_span": 12})
res = await client.post("/api/v1/racks/save", json=payload, headers=headers)
assert res.status_code == 422
async def test_accepts_a_half_width_pair_sharing_one_u(
self, client: AsyncClient, headers
):
design_id = await _design(client, headers)
payload = _state(design_id)
payload["devices"][0].update({"col_start": 0, "col_span": 6})
payload["devices"][1].update({"col_start": 6, "col_span": 6})
res = await client.post("/api/v1/racks/save", json=payload, headers=headers)
assert res.status_code == 200, res.text
async def test_rejects_an_unknown_cable_type(self, client: AsyncClient, headers):
design_id = await _design(client, headers)
payload = _state(design_id)
+1
View File
@@ -55,6 +55,7 @@ types in `@/types/rack`, narrowing every enum on the way in.
- Double-click a plate → the same modal in edit mode: label, faceplate, U/height/column/width, status, colour, port list, Unmount. Single click only selects.
- Double-click empty rack chrome → `RackSettingsModal` (name, location, U height, 19"/10", numbering direction, frame/rail/interior colours, U numbers, enclosed, delete).
- Growing a device — by hand or by picking a taller plate — relocates it to the nearest slot that takes the new size. Only a rack with no such slot rejects the edit, and says so.
- Shrinking a **rack** follows the same rule: `updateRack` clamps `uHeight` to `[MIN_RACK_U, MAX_RACK_U]` (1–48; the backend tolerates 100) and relocates every mount the new height would push above the top rail, one at a time so two never land on the same slot. It returns false and changes nothing when one has nowhere to go. The number input's `min`/`max` are hints the browser does not enforce on typed input — a raw 999 used to make every save fail the backend's `1..100` check with no cause shown, and a shrink left plates drawn outside the chassis that nothing could drag back.
- **Patch mode**: drag from port A to port B to cable them — a dashed rubber band follows the pointer, exactly like dragging an edge on the logical canvas. Clicking A then B still works; Escape drops a half-drawn patch. Rack dragging is disabled while in patch mode.
- **Click a cable** — in patch mode or out of it — to select it: it gets an accent halo and opens `RackCablePanel` on the right (endpoints, type, colour, label, properties, Unplug). Delete/Backspace unplugs the selection; Escape or a pane click deselects. Selecting a cable drops any mount/rack selection and vice versa — one rail, one occupant.
- **Import links**: reads the physical edges (ethernet/fibre/vlan/cluster) of every non-rack design and matches them on `nodeId`. Idempotent — a device pair already cabled is skipped, whichever ports carry it, so a re-run after racking more gear adds only what is missing. There is no "done" flag: one lived in memory only, and a reload re-armed the import onto the next free ports.
+78
View File
@@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach } from 'vitest'
import { useRackStore } from '../store'
import { getFaceplate } from '../faceplates'
import { RACK_COLUMNS } from '@/types'
import { MAX_RACK_U, MIN_RACK_U } from '../rackDefaults'
import { demoNetworkLinks } from '../demoData'
const store = () => useRackStore.getState()
@@ -36,6 +37,83 @@ describe('racks', () => {
expect(style.showNumbers).toBe(false)
expect(style.frame).toBeTruthy()
})
/** An empty rack, so a clamp test is not also a relocation test. */
const emptyRack = () => {
const id = store().addRack()
return { id, height: () => store().racks.find((r) => r.id === id)!.uHeight }
}
it('clamps the height to the supported range', () => {
// The number input's min/max do not survive a typed value, and the backend
// rejects anything over 100 U with a 422 the user cannot read.
const rack = emptyRack()
expect(store().updateRack(rack.id, { uHeight: 999 })).toBe(true)
expect(rack.height()).toBe(MAX_RACK_U)
store().updateRack(rack.id, { uHeight: 0 })
expect(rack.height()).toBe(MIN_RACK_U)
store().updateRack(rack.id, { uHeight: -5 })
expect(rack.height()).toBe(MIN_RACK_U)
})
it('rounds a fractional height', () => {
const rack = emptyRack()
store().updateRack(rack.id, { uHeight: 12.6 })
expect(rack.height()).toBe(13)
})
it('relocates the mounts a shrink would push above the top rail', () => {
const rack = store().racks[0]
const tallest = store().devices
.filter((d) => d.rackId === rack.id)
.reduce((max, d) => (d.uStart + d.uHeight - 1 > max.uStart + max.uHeight - 1 ? d : max))
const next = tallest.uStart - 1
const mountedBefore = store().devices.filter((d) => d.rackId === rack.id).length
expect(store().updateRack(rack.id, { uHeight: next })).toBe(true)
expect(store().racks[0].uHeight).toBe(next)
// Every mount is inside the chassis, and none was dropped on the way.
const after = store().devices.filter((d) => d.rackId === rack.id)
expect(after).toHaveLength(mountedBefore)
for (const d of after) expect(d.uStart + d.uHeight - 1).toBeLessThanOrEqual(next)
})
it('never stacks two relocated mounts on the same slot', () => {
const rack = store().racks[0]
store().updateRack(rack.id, { uHeight: 12 })
const mounted = store().devices.filter((d) => d.rackId === rack.id)
for (const a of mounted) {
for (const b of mounted) {
if (a.id === b.id) continue
const uOverlap = a.uStart < b.uStart + b.uHeight && b.uStart < a.uStart + a.uHeight
const colOverlap =
a.colStart < b.colStart + b.colSpan && b.colStart < a.colStart + a.colSpan
expect(uOverlap && colOverlap).toBe(false)
}
}
})
it('refuses a shrink that leaves a mount nowhere to go, changing nothing', () => {
const rack = store().racks[0]
const before = store().devices.map((d) => ({ ...d }))
expect(store().updateRack(rack.id, { uHeight: 1 })).toBe(false)
expect(store().racks[0].uHeight).toBe(rack.uHeight)
expect(store().devices).toEqual(before)
})
it('leaves the mounts alone when the rack grows', () => {
const before = store().devices.map((d) => ({ ...d }))
expect(store().updateRack('rack-main', { uHeight: MAX_RACK_U })).toBe(true)
expect(store().devices).toEqual(before)
})
it('returns false for a rack that does not exist', () => {
expect(store().updateRack('nope', { name: 'Ghost' })).toBe(false)
})
})
describe('mounting', () => {
@@ -4,11 +4,13 @@
* Same reasoning as `RackDeviceModal`: the rack canvas has no right rail, so the
* settings live in a dialog. Opened by double-clicking a rack's chassis.
*/
import { toast } from 'sonner'
import { Dialog, DialogContent, DialogHeader, DialogTitle } from '@/components/ui/dialog'
import { Button } from '@/components/ui/button'
import { Label } from '@/components/ui/label'
import { useRackStore } from '../store'
import { freeUnits } from '../layout'
import { MAX_RACK_U, MIN_RACK_U } from '../rackDefaults'
import type { RackNumbering, RackWidthStandard } from '@/types'
const inputClass =
@@ -62,12 +64,18 @@ export function RackSettingsModal() {
<Field label={`Height — ${used}U used of ${rack.uHeight}U`}>
<input
type="number"
min={1}
max={48}
min={MIN_RACK_U}
max={MAX_RACK_U}
className={inputClass}
aria-label="Rack height"
value={rack.uHeight}
onChange={(e) => updateRack(rack.id, { uHeight: Number(e.target.value) || 1 })}
onChange={(e) => {
// The store clamps and relocates; it only refuses when a mount
// the shrink pushes out has nowhere left to go.
if (!updateRack(rack.id, { uHeight: Number(e.target.value) || MIN_RACK_U })) {
toast.error('Not enough room to shrink the rack — unmount something first')
}
}}
/>
</Field>
+8
View File
@@ -9,6 +9,14 @@ export const DEFAULT_RACK_STYLE: RackStyle = {
enclosed: false,
}
/**
* Capacity a rack may be set to. The backend accepts up to 100 U; the UI stops
* at 48 — taller than any cabinet a homelab owns, and the number input's
* `min`/`max` are only hints, so the store clamps to these for real.
*/
export const MIN_RACK_U = 1
export const MAX_RACK_U = 48
export const CABLE_COLORS: Record<CableType, string> = {
ethernet: '#39d353',
fiber: '#f0a500',
+56 -5
View File
@@ -21,7 +21,7 @@ import {
toRackDevice,
} from '@/utils/rackSerializer'
import { getFaceplate, suggestFaceplate } from './faceplates'
import { canPlace, findSlot, type Placement } from './layout'
import { canPlace, clamp, findSlot, type Placement } from './layout'
import {
RACK_COLUMNS,
type Cable,
@@ -34,7 +34,13 @@ import {
type RackStyle,
} from '@/types'
import { demoCables, demoDevices, demoInventory, demoRacks } from './demoData'
import { CABLE_COLORS, DEFAULT_RACK_STYLE, PORT_CABLE_TYPE } from './rackDefaults'
import {
CABLE_COLORS,
DEFAULT_RACK_STYLE,
MAX_RACK_U,
MIN_RACK_U,
PORT_CABLE_TYPE,
} from './rackDefaults'
export { CABLE_COLORS, DEFAULT_RACK_STYLE }
@@ -130,7 +136,12 @@ interface RackState {
// Racks
addRack: (partial?: Partial<Rack>) => string
updateRack: (id: string, patch: Partial<Omit<Rack, 'id'>>) => void
/**
* Edit a rack. `uHeight` is clamped to `[MIN_RACK_U, MAX_RACK_U]`, and a
* shrink relocates the mounts it would push above the top rail. Returns false
* — leaving the rack untouched — when one of them has nowhere left to go.
*/
updateRack: (id: string, patch: Partial<Omit<Rack, 'id'>>) => boolean
updateRackStyle: (id: string, patch: Partial<RackStyle>) => void
moveRack: (id: string, position: { x: number; y: number }) => void
/** Removes the rack and every device mounted in it (inventory untouched). */
@@ -388,8 +399,48 @@ export const useRackStore = create<RackState>((set, get) => {
return id
},
updateRack: (id, patch) =>
edit((s) => ({ racks: s.racks.map((r) => (r.id === id ? { ...r, ...patch } : r)) })),
updateRack: (id, patch) => {
const { racks, devices } = get()
const rack = racks.find((r) => r.id === id)
if (!rack) return false
const next: Rack = { ...rack, ...patch }
if (patch.uHeight !== undefined) {
// The modal's min/max are hints the browser does not enforce on typed
// input, and the height reached the store raw: 999 made every save fail
// the backend's 1..100 check with no cause shown, and a shrink left
// mounts drawn above the chassis with no way to drag them back.
next.uHeight = clamp(Math.round(patch.uHeight) || MIN_RACK_U, MIN_RACK_U, MAX_RACK_U)
}
let relocated = devices
if (next.uHeight < rack.uHeight) {
const mounted = devices.filter((d) => d.rackId === id)
const pushedOut = mounted.filter((d) => d.uStart + d.uHeight - 1 > next.uHeight)
if (pushedOut.length > 0) {
const moved = new Map<string, RackDevice>()
// Place them one at a time against the layout decided so far, so two
// relocated mounts cannot be handed the same slot.
let settled = mounted.filter((d) => !pushedOut.includes(d))
for (const device of pushedOut) {
const slot = findSlot(next, settled, { ...device, uStart: next.uHeight }, device.id)
// Nowhere left to put it: refuse the whole edit rather than drop a
// mount off the rack, the same way a device resize refuses.
if (!slot) return false
const placed = { ...device, ...slot }
settled = [...settled, placed]
moved.set(device.id, placed)
}
relocated = devices.map((d) => moved.get(d.id) ?? d)
}
}
edit(() => ({
racks: racks.map((r) => (r.id === id ? next : r)),
devices: relocated,
}))
return true
},
updateRackStyle: (id, patch) =>
edit((s) => ({