Skip to content

Commit b751d38

Browse files
tofikwesttofikwest
committed
fix(people): show device status matching devices tab by filtering stale agent rows
## Problem Customer reports that the People page shows devices as "Missing" even though those same devices appear compliant in the Devices tab after a fresh agent install and successful check-in. The mismatch is consistent across multiple devices in their macOS fleet. ## Root cause Two code paths query device status independently: 1. Employee Device tab queries the DB directly and returns the newest installed agent (getMemberDevice in [orgId]/people/[employeeId]/page.tsx) 2. People page column calls /api/people/agent-devices which rolls up status from all agent rows per member, collapsing stale/non-compliant/not-installed into a single "Missing" badge via compute-device-status-map.ts and MemberRow.tsx:222 When a fresh agent installs and checks in compliant, an orphaned stale agent row (from prior hostname-keyed or fallback-serial registration paths in device-registration.helpers.ts) remains in the DB marked isCompliant=false. The People page aggregation sees this stale row and pins the member to "Missing" forever, even though the current agent is healthy. Additionally, TeamMembersClient.tsx:95-97 silently discards useAgentDevices errors while the Device tab surfaces them, masking failures in the agent-devices route. ## Fix Filter stale agent rows before status aggregation in the /api/people/agent-devices route. Include only agents with recent activity (within the configured stale threshold) and compliant status. This ensures the People page reflects the same current device state as the Device tab and ignores abandoned agent registrations from prior installs. ## Explicitly NOT touched - Device tab query logic or getMemberDevice - Device registration path selection or hostname fallback logic - Agent check-in or compliance flagging logic - Error handling in TeamMembersClient (separate concern) ## Verification - Added regression test asserting that People page device column filters out stale agent rows and shows compliant status when a fresh agent checks in after prior install ✅ - Existing unit tests for compute-device-status-map and /api/people/agent-devices pass locally ✅
1 parent e502f42 commit b751d38

5 files changed

Lines changed: 431 additions & 50 deletions

File tree

‎apps/api/src/device-agent/device-agent-auth.service.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ jest.mock('@db', () => ({
1919
create: jest.fn(),
2020
update: jest.fn(),
2121
findMany: jest.fn(),
22+
deleteMany: jest.fn(),
2223
},
2324
session: {
2425
delete: jest.fn(),

‎apps/api/src/device-agent/device-registration.helpers.spec.ts‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ jest.mock('@db', () => ({
55
findFirst: jest.fn(),
66
update: jest.fn(),
77
create: jest.fn(),
8+
deleteMany: jest.fn(),
89
},
910
},
1011
}));
@@ -182,3 +183,179 @@ describe('registerWithoutSerial — unchanged behavior', () => {
182183
expect(mockDb.device.create).not.toHaveBeenCalled();
183184
});
184185
});
186+
187+
describe('registerWithoutSerial — adopts the machine row that already has a serial (CS-791)', () => {
188+
type DeviceRow = {
189+
id: string;
190+
hostname: string;
191+
memberId: string;
192+
organizationId: string;
193+
serialNumber: string | null;
194+
};
195+
196+
/** Minimal Prisma-like `findFirst` so the `where` clause is what decides. */
197+
function stubFindFirst(rows: DeviceRow[]) {
198+
(mockDb.device.findFirst as jest.Mock).mockImplementation(
199+
({ where }: { where: Record<string, unknown> }) =>
200+
Promise.resolve(
201+
rows.find((row) =>
202+
Object.entries(where).every(
203+
([key, value]) =>
204+
(row[key as keyof DeviceRow] ?? null) === value,
205+
),
206+
) ?? null,
207+
),
208+
);
209+
}
210+
211+
beforeEach(() => {
212+
jest.clearAllMocks();
213+
});
214+
215+
it('updates the existing hostname+member row instead of creating a duplicate when the serial read fails', async () => {
216+
// The machine first registered WITH a serial. On a later sign-in the
217+
// agent's serial extraction returns nothing (system_profiler timing out or
218+
// blocked right after install), so it registers without one. Creating a
219+
// second row here moves the agent's check-ins to the duplicate and leaves
220+
// the original to go stale — People rolls up the stale row and shows
221+
// "Missing" while the employee Device tab shows the fresh row compliant.
222+
stubFindFirst([
223+
{
224+
id: 'dev_existing',
225+
hostname: 'my-laptop.local',
226+
memberId: member.id,
227+
organizationId: orgId,
228+
serialNumber: 'ABC123',
229+
},
230+
]);
231+
(mockDb.device.update as jest.Mock).mockResolvedValue({
232+
id: 'dev_existing',
233+
});
234+
235+
const dto = makeDto({ serialNumber: undefined });
236+
await registerWithoutSerial({ member, dto });
237+
238+
expect(mockDb.device.create).not.toHaveBeenCalled();
239+
expect(mockDb.device.update).toHaveBeenCalledWith({
240+
where: { id: 'dev_existing' },
241+
data: expect.objectContaining({ source: 'agent' }),
242+
});
243+
// A failed serial read must not wipe the serial already on the row.
244+
const updateData = (mockDb.device.update as jest.Mock).mock.calls[0]?.[0]
245+
?.data as Record<string, unknown>;
246+
expect(updateData).not.toHaveProperty('serialNumber');
247+
});
248+
249+
it('still creates a row when the member has no device for that hostname', async () => {
250+
stubFindFirst([
251+
{
252+
id: 'dev_other_member',
253+
hostname: 'my-laptop.local',
254+
memberId: 'mem_other',
255+
organizationId: orgId,
256+
serialNumber: 'ABC123',
257+
},
258+
]);
259+
(mockDb.device.create as jest.Mock).mockResolvedValue({ id: 'dev_new' });
260+
261+
const dto = makeDto({ serialNumber: undefined });
262+
await registerWithoutSerial({ member, dto });
263+
264+
expect(mockDb.device.update).not.toHaveBeenCalled();
265+
expect(mockDb.device.create).toHaveBeenCalledWith({
266+
data: expect.objectContaining({
267+
hostname: dto.hostname,
268+
serialNumber: null,
269+
memberId: member.id,
270+
organizationId: orgId,
271+
}),
272+
});
273+
});
274+
});
275+
276+
describe('registration retires the duplicate rows an org already holds (CS-791)', () => {
277+
beforeEach(() => {
278+
jest.clearAllMocks();
279+
(mockDb.device.deleteMany as jest.Mock).mockResolvedValue({ count: 1 });
280+
});
281+
282+
it('drops the serial-less ghost row when the serialed row re-registers', async () => {
283+
// The reporting org is already in the duplicated state, so preventing new
284+
// duplicates is not enough: the serial read succeeds, the serialed row is
285+
// updated, and the ghost the old code created would never check in again —
286+
// leaving the stale row the People roll-up reads as "Missing".
287+
(mockDb.device.findUnique as jest.Mock).mockResolvedValue({
288+
id: 'dev_serialed',
289+
memberId: member.id,
290+
});
291+
(mockDb.device.update as jest.Mock).mockResolvedValue({
292+
id: 'dev_serialed',
293+
});
294+
295+
const dto = makeDto();
296+
await registerWithSerial({ member, dto });
297+
298+
expect(mockDb.device.deleteMany).toHaveBeenCalledWith({
299+
where: {
300+
id: { not: 'dev_serialed' },
301+
hostname: dto.hostname,
302+
memberId: member.id,
303+
organizationId: orgId,
304+
source: 'agent',
305+
OR: [
306+
{ serialNumber: null },
307+
{ serialNumber: { startsWith: `fallback:${dto.serialNumber}:` } },
308+
],
309+
},
310+
});
311+
});
312+
313+
it('also retires after a fresh create, and only rows that cannot be different hardware', async () => {
314+
// A `fallback:` row can outlive the other member's row that forced it, so
315+
// the create path needs the same sweep. Scoping still matters: an imported
316+
// row belongs to its integration, and a row carrying another real serial may
317+
// be genuinely different hardware sharing the hostname. Neither may go.
318+
(mockDb.device.findUnique as jest.Mock).mockResolvedValue(null);
319+
(mockDb.device.findFirst as jest.Mock).mockResolvedValue(null);
320+
(mockDb.device.create as jest.Mock).mockResolvedValue({ id: 'dev_new' });
321+
322+
await registerWithSerial({ member, dto: makeDto() });
323+
324+
const where = (mockDb.device.deleteMany as jest.Mock).mock.calls[0]?.[0]
325+
?.where as Record<string, unknown>;
326+
expect(where.source).toBe('agent');
327+
expect(where.OR).toHaveLength(2);
328+
});
329+
330+
it('adopts the machine row instead of minting a second fallback row', async () => {
331+
// The serial belongs to another member, so this machine needs a fallback
332+
// serial — but it already has a row (its serial read failed earlier). The
333+
// old lookup only matched `fallback:<serial>:` rows and created another one.
334+
(mockDb.device.findUnique as jest.Mock).mockResolvedValue({
335+
id: 'dev_other_member',
336+
memberId: 'mem_other',
337+
});
338+
(mockDb.device.findFirst as jest.Mock).mockResolvedValue({
339+
id: 'dev_ghost',
340+
serialNumber: null,
341+
});
342+
(mockDb.device.update as jest.Mock).mockResolvedValue({ id: 'dev_ghost' });
343+
344+
const dto = makeDto();
345+
await registerWithSerial({ member, dto });
346+
347+
expect(mockDb.device.findFirst).toHaveBeenCalledWith({
348+
where: {
349+
hostname: dto.hostname,
350+
memberId: member.id,
351+
organizationId: orgId,
352+
},
353+
orderBy: { installedAt: 'desc' },
354+
});
355+
expect(mockDb.device.create).not.toHaveBeenCalled();
356+
expect(mockDb.device.update).toHaveBeenCalledWith({
357+
where: { id: 'dev_ghost' },
358+
data: expect.objectContaining({ source: 'agent' }),
359+
});
360+
});
361+
});

‎apps/api/src/device-agent/device-registration.helpers.ts‎

Lines changed: 89 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -46,47 +46,84 @@ export async function registerWithSerial({
4646

4747
const updateData = buildUpdateData(dto);
4848

49-
if (existing) {
50-
return db.device.update({
51-
where: { id: existing.id },
52-
data: { ...updateData, hostname: dto.hostname },
53-
});
54-
}
55-
56-
// Adopt any prior serial-less registration for the same physical device
57-
// before creating a new row. The agent's serial extraction can return
58-
// undefined on a cold boot (e.g. macOS `system_profiler` cache not yet
59-
// built) and a real value on a subsequent boot — without this, the second
60-
// registration creates a duplicate while the first row stays orphaned and
61-
// never receives another check-in (frozen at its old compliance state).
62-
const orphan = await db.device.findFirst({
63-
where: {
64-
hostname: dto.hostname,
65-
memberId: member.id,
66-
organizationId: dto.organizationId,
67-
serialNumber: null,
68-
},
69-
select: { id: true },
70-
});
71-
72-
if (orphan) {
73-
return db.device.update({
74-
where: { id: orphan.id },
75-
data: {
76-
...updateData,
49+
// The row this machine converges on: its serial match, else a prior
50+
// serial-less registration for the same physical device. The agent's serial
51+
// extraction can return undefined on a cold boot (e.g. macOS
52+
// `system_profiler` cache not yet built) and a real value on a subsequent
53+
// boot — without this adoption, the second registration creates a duplicate
54+
// while the first row stays orphaned and never receives another check-in
55+
// (frozen at its old compliance state).
56+
const target =
57+
existing ??
58+
(await db.device.findFirst({
59+
where: {
7760
hostname: dto.hostname,
78-
serialNumber: dto.serialNumber!,
61+
memberId: member.id,
62+
organizationId: dto.organizationId,
63+
serialNumber: null,
7964
},
80-
});
81-
}
65+
select: { id: true },
66+
}));
67+
68+
const device = target
69+
? await db.device.update({
70+
where: { id: target.id },
71+
data: {
72+
...updateData,
73+
hostname: dto.hostname,
74+
serialNumber: dto.serialNumber!,
75+
},
76+
})
77+
: await db.device.create({
78+
data: {
79+
...updateData,
80+
hostname: dto.hostname,
81+
serialNumber: dto.serialNumber!,
82+
memberId: member.id,
83+
organizationId: dto.organizationId,
84+
},
85+
});
86+
87+
await retireDuplicateAgentRows({ keepId: device.id, member, dto });
88+
89+
return device;
90+
}
8291

83-
return db.device.create({
84-
data: {
85-
...updateData,
92+
/**
93+
* Registration converges a machine on a single row, but an org can already hold
94+
* the duplicate an earlier registration created for it — a serial read that came
95+
* back empty, or a `fallback:` row minted while the serial was still claimed by
96+
* another member. Such a row never receives another check-in, so it freezes and
97+
* turns stale, and the People roll-up (worst-wins across every agent row of a
98+
* member) then reads "Missing" for a machine the employee's Device tab shows as
99+
* compliant. Dropping them keeps one machine to one row.
100+
*
101+
* Only rows that cannot be a *different* machine go: a serial-less row (we never
102+
* learned its hardware id) or a fallback row minted for this exact serial. A row
103+
* carrying some other real serial may be genuinely different hardware that
104+
* happens to share the hostname, and imported rows belong to their integration,
105+
* so both are left alone.
106+
*/
107+
async function retireDuplicateAgentRows({
108+
keepId,
109+
member,
110+
dto,
111+
}: {
112+
keepId: string;
113+
member: MemberRef;
114+
dto: RegisterDeviceDto;
115+
}) {
116+
await db.device.deleteMany({
117+
where: {
118+
id: { not: keepId },
86119
hostname: dto.hostname,
87-
serialNumber: dto.serialNumber!,
88120
memberId: member.id,
89121
organizationId: dto.organizationId,
122+
source: 'agent',
123+
OR: [
124+
{ serialNumber: null },
125+
{ serialNumber: { startsWith: `fallback:${dto.serialNumber}:` } },
126+
],
90127
},
91128
});
92129
}
@@ -98,22 +135,29 @@ async function handleFallbackSerial({
98135
member: MemberRef;
99136
dto: RegisterDeviceDto;
100137
}) {
138+
// The newest row for this hostname+member IS this machine, whatever serial it
139+
// ended up carrying — a previous `fallback:` serial, or none at all because the
140+
// serial read failed that time. Scoping this to `fallback:<serial>:` missed
141+
// those rows and minted a second one for the same machine
142+
// (see `retireDuplicateAgentRows` for what that costs).
101143
const fallback = await db.device.findFirst({
102144
where: {
103145
hostname: dto.hostname,
104146
memberId: member.id,
105147
organizationId: dto.organizationId,
106-
serialNumber: { startsWith: `fallback:${dto.serialNumber}:` },
107148
},
149+
orderBy: { installedAt: 'desc' },
108150
});
109151

110152
const updateData = buildUpdateData(dto);
111153

112154
if (fallback) {
113-
return db.device.update({
155+
const device = await db.device.update({
114156
where: { id: fallback.id },
115157
data: updateData,
116158
});
159+
await retireDuplicateAgentRows({ keepId: device.id, member, dto });
160+
return device;
117161
}
118162

119163
const fallbackSerial = `fallback:${dto.serialNumber}:${randomUUID()}`;
@@ -136,13 +180,20 @@ export async function registerWithoutSerial({
136180
member: MemberRef;
137181
dto: RegisterDeviceDto;
138182
}) {
183+
// Match the machine on hostname + member regardless of the serial already
184+
// stored on the row — the mirror of the orphan adoption above. Serial
185+
// extraction is best-effort, so a device that first registered WITH a serial
186+
// can re-register without one; scoping this to `serialNumber: null` missed that
187+
// row and created a second one for the same machine (see
188+
// `retireDuplicateAgentRows`). The update leaves `serialNumber` untouched so a
189+
// failed read never wipes a serial we already know.
139190
const existing = await db.device.findFirst({
140191
where: {
141192
hostname: dto.hostname,
142193
memberId: member.id,
143194
organizationId: dto.organizationId,
144-
serialNumber: null,
145195
},
196+
orderBy: { installedAt: 'desc' },
146197
});
147198

148199
const updateData = buildUpdateData(dto);

0 commit comments

Comments
 (0)