Skip to content

feat(devices): one device name everywhere — nickname first (CU-86ake4xz3) - #386

Open
bohdan-blip wants to merge 11 commits into
mainfrom
feat/device-name-nickname-ssot
Open

bohdan-blip wants to merge 11 commits into
mainfrom
feat/device-name-nickname-ssot

Conversation

@bohdan-blip

Copy link
Copy Markdown
Contributor

Summary

A device renamed by the user (nickname, labelled Display Name in the UI) still showed its hostname on most screens, and could not be found by typing that name. On test-3020 the only device is hostname: "Mac", displayName: null, nickname: "MyMacTest1": the Devices page said MyMacTest1, the Run Script picker said Mac, the Logs SOURCE column said Mac, and searching the picker for the device's own name returned "No devices found".

Every screen that shows or searches a device name now resolves it through the existing SSOT getDeviceName() (nickname → displayName → hostname), and every client-side search predicate shares one primitive. 25 hand-written displayName || hostname chains removed.

ClickUp: 86ake4xz3. Backend: openframe-oss-lib#2088, openframe-saas-tenant#3020.

Changes

  • Fragment laddernickname moves from deviceFields_machine (step 3) down to deviceRowFields_machine (step 1), selected whole at one step per the ladder's rule. This is what makes the schedule picker and the Assigned Devices tab resolve the same name as the Devices page; before, getDeviceName there silently fell through to displayName. The raw-POST assignments union query is type-checked against the generated row shape and had to start selecting the field too.
  • Select Device / pickersdevice-selector.tsx name cell and test-script-modal.tsx callback → getDeviceName. One cell fixes eight pickers.
  • Search predicates — new matchesDeviceName(device, search) beside getDeviceName: the rendered name or the hostname, case-insensitive substring. Hostname stays a match after a nickname is set on purpose: the server's devices(search:) matches hostname regardless of nickname (alongside ip, serial, manufacturer, model — CustomMachineRepositoryImpl.buildDeviceQuery), and a list filtering what it already loaded must not be stricter than the page it stands in for. Before, the three client predicates had three different rules. Used by the pickers, DevicesAssignedTable, and the monitoring Query "Assigned Devices" table.
  • Display sitesmachineLabel (all Execution History tables + execution details), schedule assign/unassign toasts, shared DeviceInfoSection and LogDrawer device cards (also drop the description/machineId fallbacks device-name.ts forbids), monitoring query/policy row builders, live-query device dropdown, ticket device autocomplete, assignments type-ahead label.
  • Device sub-pages — remote shell / remote desktop / file manager had the priority inverted (hostname || displayName). Their hostname variable only ever fed a page title and the Mingo "open view" label, so it is renamed deviceName; the FileManagerContainer prop follows, since a nickname now travels through it.
  • LogsLogEvent.nickname / LogDetails.nickname (new on the backend) selected in the Relay fragment and in GetLogs; the synthetic device the log row builds stops inventing displayName out of the hostname and carries the real field. GetLogDetails deliberately does not select it: that page's device card fetches the live device, and a snapshot name there would be wrong.
  • schema.graphql — the two nickname: String fields hand-added exactly as #2088 adds them to log.graphqls; verified field-for-field (including position) by introspecting test-3020.
  • Tests — first coverage for getDeviceName (7) and matchesDeviceName (7), including the empty-string fall-through that Pinot's schema default for the logs column produces.

Testing

  • npm run relay, npm run type-check, npm run lint:ci, Prettier — green. npm run build green before the rebase onto current main.
  • Vitest: 206/206 on the branch before the rebase. The one full run after rebasing onto main (+2 unrelated commits) reported 1 failure in 206; every subset re-run since (src/lib 61, src/graphql/notifications 21, mobile-auth-return 8) passes, and the full suite was not re-run to keep the machine free. npm test is not a CI gate. To be pinned down before merge.
  • Live against test-3020 (API): Machine.nickname, LogEvent.nickname, LogDetails.nickname present; devices(search:) finds the device by nickname, by substring, lowercase, and by hostname; logs(search:"MyMac") → 3 RMM events carrying nickname; scriptExecutions[].machine.nickname populated; all four raw-POST documents in this PR (AssignmentsAssignedItems, AssignmentsDevicesSearch, GetLogs, GetLogDetails) executed verbatim against the live schema — no validation errors.
  • Live "before" on the deployed frontend: Devices → MyMac (already via getDeviceName); Logs search "MyMac" → 3 rows, SOURCE column "Mac"; Run Script picker → "Mac", search "MyMac" → "No devices found"; Execution History → "Mac" while the API returns nickname: "MyMacTest1" for the same rows.
  • Not verifiable on test-3020: schedule picker / Assigned Devices tab (no schedules) and tickets (none). Those paths are covered by the compile-time ladder guarantee only.
  • relay/unused-fields (editor-only backlog, off in eslint.ci.mjs): +2 net, all inside the fragment definition files; logs-table.tsx stays clean because node.nickname is read directly.

Backend gaps — deliberately not worked around here

  • Tickets keep hostname. /chat/graphql (saas-ai-agent) Machine exposes only id, machineId, hostname, organizationId — verified by introspecting all 160 types. A client-side join through /api/graphql's device(machineId:) is possible but rejected: it would be a string-built document (raw-POST against /api/graphql, which CLAUDE.md forbids), a cross-service join on the client, and would mix a live nickname with the snapshotted Ticket.deviceHostname. Fix is Ticket.deviceNickname next to deviceHostname.
  • Log search by hostname returns nothing. logs(search:"Mac") → 0 for events whose hostname is "Mac"; control logs(search:"executed") → 1. Blocks the fallback case: a device without nickname shows its hostname in Logs but cannot be searched by it.
  • FLEET log events are not attributed to a device. 29 of 32 events on the stand have hostname as the literal string "null", no deviceId, no nickname. The existing 'null' → "System" guard in the logs table still applies unchanged.

Notes

  • Merge after the backend. The Relay logs fragment is validated against the local schema.graphql, not the runtime. Deployed to an environment without LogEvent.nickname, the whole logs query errors and the Logs page breaks. #3020 currently has conflicts and no review.
  • schema.graphql is hand-edited. Re-run npm run fetch-schema once #2088 reaches a normal environment; running it against one without the field will silently drop nickname and break the build on the logs fragment.
  • Log nicknames are snapshots. Renaming a device does not touch existing log events: after MyMac → MyMacTest1 on the stand, the 3 old events still read MyMac while every live surface (Devices, picker, Execution History, the device card in the log drawer) reads MyMacTest1. Expected, not a bug — worth knowing before QA files it as one.
  • npm run dev does not run relay-compiler and src/__generated__ is gitignored: after checking out this branch run npm run relay, or the stale artifacts will not select nickname and the old names come back.
  • Out of scope, flagged for follow-ups: the pickers' 100-device client-side ceiling (DEVICE_LIST_LIMIT, pre-existing, affects hostname search equally); MachineLike in execution-helpers.ts is the hand-written structural type CLAUDE.md warns about and wants a small @inline machine fragment; logs-table.tsx / logging-step.tsx build /log-details?… by hand outside routes.ts.

🤖 Generated with Claude Code

…xz3)

Every screen that shows or searches a device name now resolves it through
`getDeviceName` (nickname -> displayName -> hostname). 25 call sites built
that name with their own `displayName || hostname` chain, so a device
rendered under its nickname could not be found by typing it.

- `nickname` moves DOWN the fragment ladder, from `deviceFields_machine`
  (step 3) to `deviceRowFields_machine` (step 1). Selected whole at one
  step, per the ladder's rule. This is what makes the schedule picker and
  the Assigned Devices tab resolve the same name as the Devices page. The
  raw-POST assignments path is type-checked against the generated row
  shape, so it had to start selecting the field too.
- Client-side search predicates share one primitive, `matchesDeviceName`:
  the rendered name OR the hostname, case-insensitive substring. Hostname
  stays a match after a nickname is set on purpose — the server's
  `devices(search:)` matches hostname regardless of nickname (alongside
  ip, serial, manufacturer, model; openframe-oss-lib#1658), and a list
  filtering what it already loaded must not be stricter than the page it
  stands in for. Before, the three predicates had three different rules.
- Logs carry `nickname` as of openframe-oss-lib#2088 / openframe-saas-
  tenant#3020. `LogEvent`/`LogDetails` have no `displayName`, so the
  synthetic device the log row builds stops inventing one out of the
  hostname and carries the real field instead. Log details keep fetching
  the live device for their card, so the snapshot field is not selected
  there.
- `remote-shell` / `remote-desktop` / `file-manager` had the priority
  inverted (hostname first); their `hostname` variable only ever fed a
  page title and a Mingo label, so it is renamed to `deviceName`.
- First test coverage for `getDeviceName` and `matchesDeviceName`,
  including the empty-string fall-through that Pinot's schema default for
  the logs column produces.

`schema.graphql` is hand-edited with the two `nickname: String` fields
exactly as openframe-oss-lib#2088 adds them to `log.graphqls`; verified
field-for-field against test-3020. Re-run `npm run fetch-schema` once that
PR is deployed to a normal environment.

Tickets keep their own name chain: they read machines over `/chat/graphql`
(saas-ai-agent), whose `Machine` exposes only id, machineId, hostname and
organizationId — verified by introspecting all 160 types on test-3020.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@bohdan-blip
bohdan-blip requested review from a team as code owners September 11, 2026 15:59
@michaelassraf

Copy link
Copy Markdown
Contributor

aliaska-varieva and others added 4 commits September 14, 2026 14:32
…iceName (CU-86ake4xz3)

The tickets list Source column and the ticket details header/Device field were the
last device-name surfaces still on hostname: their machine comes from /chat/graphql,
whose Machine type had no nickname until openframe-saas-tenant#3020 added it
(resolved live from the device registry, nullable).

Select `nickname` in the three ClientTicketOwner.machine selections and route both
surfaces through one helper, getTicketDeviceName = getDeviceName({ nickname,
hostname: deviceHostname || machine.hostname }), so tickets follow the same
nickname -> displayName -> hostname rule as every other screen. Drop the
Machine.displayName field the types carried: the chat schema never had it and no
query selected it, so both fallbacks that read it were dead.

Depends on the ai-agent change being deployed — a tenant whose ai-agent predates it
fails the whole tickets query on the unknown field.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fleet audit events (User logged in, Created saved query) arrive with the string
"null" — not null — in hostname, organizationName and organizationId. The Source
cell already mapped that hostname to "System" but rendered the organization as-is,
so every system row read "System / null". Map the organization the same way, on
the logs page and the onboarding logging step.

The backend is being fixed at ingest, but log rows are immutable, so the rows
already written keep the literal — the guard stays for them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread src/app/(app)/tickets/utils/ticket-device-name.ts
Comment thread src/graphql/devices/device-fields.ts
bohdan-blip and others added 5 commits September 15, 2026 16:09
…name-ssot

# Conflicts:
#	src/app/(app)/devices/details/file-manager/page.tsx
#	src/app/(app)/devices/details/remote-desktop/page.tsx
…on machineId presence

Both owner kinds carry the `type` discriminant, so check that instead of whether a
`machineId` field happens to exist — an ADMIN owner that ever grows one would no
longer be misread as a CLIENT owner with a missing machine. Review follow-up on #386.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… (CU-86ake4xz3)

getDeviceName accepted a shape whose keys were all optional, so an operation that
forgot to select `nickname` compiled and quietly rendered the hostname — the
silent-fallback trap the fragment ladder in src/graphql/devices/ exists to close
(its own header describes the all-optional MachineLike it replaced). Close it at
the sink too:

- `DeviceNameSource` requires the `nickname` key (null/undefined allowed, silence
  not); `Device.nickname` and `DeviceGraphQlNode.nickname` become required keys,
  and `PartialNamedDevice` is the Partial<Device> that still carries it (log rows,
  DeviceInfoSection). A test pins the contract with @ts-expect-error so loosening
  it fails tsc.
- execution-helpers: `MachineLike`, the hand-written all-optional interface,
  becomes `ExecutionMachine`, typed off the generated executionFields machine.
- monitoring rows: the field named `displayName` held the resolved name, and the
  query table searched it through matchesDeviceName on a row that is a Fleet host,
  not a device. Rename it `name` and search name/hostname explicitly.
- LogEntry.nickname is `string | null`, as the query returns it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e selector search once

The remote-desktop and remote-shell pages kept a useMemo around a plain string;
useTrackOpenView already depends on the primitive fields, and the compiler
memoizes the rest. The device selector trimmed the term for the name clause but
not for the OS clause — one needle for both. The useTrackOpenView example named
the device by hostname; it names it like every other screen now.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rals

The logs page and the onboarding logging step each mapped the literal "null" a
system event carries — first the name to "System", then (since the previous
commit) the organization to empty — in their own cell renderer. One helper,
tested, used by both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

🦩 Flamingo Code Review

3 finding(s) — 0 action required · 3 recommended · 0 informational

Mode: advisory · 3 defect(s) outside any rule

Inline comments: 2 new

Findings without an inline anchor in this diff

  • 🟠 [warn/recommended] src/app/(app)/tickets/queries/ticket-queries.ts:120 — New nickname field added to GraphQL queries but ticket-service.ts type change may outrun backend rollout
    The nickname field was added to machine selections in GET_TICKET_QUERY, GET_TICKETS_QUERY, and boardCardTicketFragment (ticket-queries.ts), and the corresponding TicketNode type in ticket-service.ts was updated to include nickname?: string | null. A comment already present nearby in ticket-queries.ts (// Unflagged, so it must not outrun the backend) documents that this codebase has a known failure mode where the frontend GraphQL selection outruns backend schema support, causing empty results across board columns, tickets table, and ticket picker. If the backend service schema does not yet expose machine.nickname on the Ticket/ClientTicketOwner type, these queries will fail entirely (GraphQL validation error) rather than degrading gracefully, breaking ticket loading. Verify the backend schema in openframe-oss-lib/openframe-oss-tenant already ships nickname on the relevant machine type before merging, or gate this field similarly to the documented pattern.
              machine {
                id
                machineId
                hostname
                nickname
                organizationId
              }
    

Need another pass? Commits pushed after this review are not reviewed automatically.

  • Review the new commits — the commits added since this review
  • Review the whole diff again — ignoring what was already reviewed

Prefer typing? Comment @flamingo-review, or @flamingo-review full. To review every push on this pull request, add the flamingo-review-always label.

React 👍/👎 on inline comments to teach the reviewer.

Started 2026-09-15 15:01 UTC · updated 2026-09-15 15:02 UTC · workflow run

Comment thread src/app/components/shared/device-selector/use-device-selector.ts
Comment thread src/app/components/shared/device-info-section.tsx
Like unreadNotificationCount, the field rides no feature flag, so the ai-agent
that added it (openframe-saas-tenant#3020) has to ship before this frontend —
say so next to the selection, where the constraint for the other field lives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants