From f8a01ef2fa527639bbd53508c461fc921b724b34 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 03:41:37 +0300 Subject: [PATCH 01/19] frontend: say what the identities console actually knows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The console showed values without the facts that make them mean something. Four of those, each with a way for an operator to be wrong: - an automatic decision rendered an EMPTY badge. The journal stores no reason for the resolver's own rows as an empty string, not as null, so the nullish fallback never fired — and the mock sent null, the one shape the wire never carries, so no test could catch it. Machine versus human is the only fact that badge holds. - a binding minted during a sign-in showed its raw `login-bootstrap` code. - a person the journal holds no attributes for — exactly what first-login provisioning creates until the resolver attaches the roster's name — was titled with its own id, stating it twice and as if it were a name. - the person id was nowhere. A conflict is normally two records of one human, so name and address are precisely the fields that fail to tell them apart; the id never does, and it is what gets pasted into a search or a ticket. It is now on every card, selectable, with a copy control — which is why the queue row stopped being a + ); +} diff --git a/src/frontend/src/components/metric-evidence-table.tsx b/src/frontend/src/components/metric-evidence-table.tsx index 82c7890a8..29a3e1f5a 100644 --- a/src/frontend/src/components/metric-evidence-table.tsx +++ b/src/frontend/src/components/metric-evidence-table.tsx @@ -1,20 +1,18 @@ -import { Fragment, useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { Fragment, useCallback, useEffect, useMemo, useState } from "react"; import { useVirtualizer } from "@tanstack/react-virtual"; import { ArrowDown, ArrowUp, - Check, ChevronDown, ChevronRight, ChevronsUpDown, - Copy, } from "lucide-react"; -import { toast } from "sonner"; import type { MetricEvidenceColumn, MetricEvidenceRow, } from "@/api/metric-drilldown-client"; +import { CopyValueButton } from "@/components/copy-value-button"; import { Button } from "@/components/ui/button"; import { Spinner } from "@/components/ui/spinner"; import { @@ -45,47 +43,6 @@ function columnLayout(column: MetricEvidenceColumn) { const EXPANDER_REM = 2.25; -function CopyValueButton({ value }: { value: string }) { - const [copied, setCopied] = useState(false); - const resetTimer = useRef(null); - - useEffect( - () => () => { - if (resetTimer.current != null) window.clearTimeout(resetTimer.current); - }, - [] - ); - - async function copyValue(): Promise { - try { - await navigator.clipboard.writeText(value); - setCopied(true); - if (resetTimer.current != null) window.clearTimeout(resetTimer.current); - resetTimer.current = window.setTimeout(() => setCopied(false), 1500); - } catch { - setCopied(false); - toast.error("Unable to copy ref"); - } - } - - return ( - - ); -} - function SortIcon({ state }: { state: "asc" | "desc" | null }) { if (state === "asc") return ; if (state === "desc") return ; @@ -303,7 +260,11 @@ export function MetricEvidenceTable({ {column.key === "ref" && value != null ? (
{line} - +
) : ( line diff --git a/src/frontend/src/components/portal/account-detail.test.tsx b/src/frontend/src/components/portal/account-detail.test.tsx index 819669de9..29c8889bb 100644 --- a/src/frontend/src/components/portal/account-detail.test.tsx +++ b/src/frontend/src/components/portal/account-detail.test.tsx @@ -125,6 +125,46 @@ describe("AccountDetail", () => { expect(screen.getByText(/1 Aug 2026/)).toBeInTheDocument(); }); + // The resolver writes no reason at all — as an empty string, which is not + // null, so a nullish fallback left the badge blank on every automatic entry. + // Machine decision versus human decision is the one thing the badge carries. + it("says a reasonless entry was automatic instead of rendering a blank badge", () => { + binding.q.data = bound({ + history: [ + { + person_id: BOB.person_id, + author_person_id: "00000000-0000-0000-0000-000000000000", + by_operator: false, + reason: "", + recorded_at: "2026-08-01T10:15:00.000000", + }, + ], + }); + render(); + + expect(screen.getByText(/automatic/i)).toBeInTheDocument(); + }); + + // First-login provisioning mints the binding during the sign-in itself; an + // operator meeting a raw `login-bootstrap` learns nothing from it. + it("names the first-sign-in provisioning reason", () => { + binding.q.data = bound({ + history: [ + { + person_id: BOB.person_id, + author_person_id: "00000000-0000-0000-0000-000000000000", + by_operator: false, + reason: "login-bootstrap", + recorded_at: "2026-08-01T10:15:00.000000", + }, + ], + }); + render(); + + expect(screen.getByText(/first sign-in/i)).toBeInTheDocument(); + expect(screen.queryByText("login-bootstrap")).not.toBeInTheDocument(); + }); + it("reads an off-queue empty journal as a stale link, offering no verbs", () => { binding.q.data = bound({ person_id: null, history: [] }); render(); diff --git a/src/frontend/src/components/portal/account-detail.tsx b/src/frontend/src/components/portal/account-detail.tsx index 740b4d5ad..63de277bb 100644 --- a/src/frontend/src/components/portal/account-detail.tsx +++ b/src/frontend/src/components/portal/account-detail.tsx @@ -31,12 +31,13 @@ import { personDisplayName } from "@/lib/identities/person-display"; import { formatUtcInstant } from "@/lib/format"; import { useAccountBinding } from "@/queries/identity-resolution"; -/** Known verb codes → i18n keys; anything else renders as-is (open vocabulary). */ +/** Known reason codes → i18n keys; anything else renders as-is (open vocabulary). */ const VERB_KEYS: Record = { "operator-bind": "identities.history.bind", "operator-merge": "identities.history.merge", "operator-detach": "identities.history.detach", "operator-exclude": "identities.history.exclude", + "login-bootstrap": "identities.history.login_bootstrap", }; export function AccountDetail({ @@ -152,13 +153,17 @@ function HistoryRow({ candidates: PersonSummary[]; }) { const { t } = useTranslation(); - const verbKey = entry.reason ? VERB_KEYS[entry.reason] : undefined; + // The resolver stores no reason for its own rows — as an empty string, not + // as null, so a nullish fallback leaves the badge blank on every automatic + // entry, which is most of them. + const reason = entry.reason?.trim() || undefined; + const verbKey = reason ? VERB_KEYS[reason] : undefined; const target = candidates.find((c) => c.person_id === entry.person_id); return (
  • - {verbKey ? t(verbKey) : (entry.reason ?? t("identities.history.automatic"))} + {verbKey ? t(verbKey) : (reason ?? t("identities.history.automatic"))} {formatUtcInstant(entry.recorded_at, "d MMM yyyy, HH:mm")} diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index 0a6b998f2..3d29f31c5 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -159,7 +159,7 @@ describe("IdentitiesView", () => { const contested = screen.getByText(/contested/i).closest("[data-slot=card]"); expect(within(contested as HTMLElement).getByText("2")).toBeInTheDocument(); - expect(screen.getByText(/nothing links the account/i)).toBeInTheDocument(); + expect(screen.getByText(/no address to match on/i)).toBeInTheDocument(); // Unknown kind lands in the catch-all group rather than vanishing. expect(screen.getByText("q-1")).toBeInTheDocument(); }); @@ -197,6 +197,51 @@ describe("IdentitiesView", () => { expect(portalRouter.search.acct).toBeUndefined(); }); + // The tiles count binding states; only the queue is work. A tile promising + // "review" for accounts the resolver binds by itself sent an operator + // looking for something they cannot do. + it("leads with the number the operator can act on — the queue's own size", () => { + attention.q.data = { items: [item({}), item({ account_id: "a2" })], rates: RATES }; + render(); + + const tile = screen + .getByText(/needs a decision/i) + .closest("div")?.parentElement; + expect(within(tile as HTMLElement).getByText("2")).toBeInTheDocument(); + // The state tiles say what they count, never "review". + expect(screen.getByText(/unbound · has an address/i)).toBeInTheDocument(); + expect(screen.queryByText(/pending review/i)).not.toBeInTheDocument(); + }); + + it("marks the decision count as a floor when the server cut the list", () => { + attention.q.data = { items: [item({})], rates: RATES, items_truncated: true }; + render(); + + expect(screen.getByText("1+")).toBeInTheDocument(); + }); + + // The row carries the values an operator copies out, so it cannot be a + //
    ); } -function RatesStrip({ rates }: { rates: ResolutionRates }) { +function RatesStrip({ + rates, + decisions, + decisionsCapped, +}: { + rates: ResolutionRates; + /** Cases in the queue — the only figure here that is the operator's work. */ + decisions: number; + /** The server cut the list, so the queue size is a floor, not the total. */ + decisionsCapped: boolean; +}) { const { t } = useTranslation(); return ( -
    - {RATE_TILES.map(({ key, status }) => ( -
    -
    {rates[key]}
    - +
    + + {RATE_TILES.map(({ key, status }) => ( + + ))} +
    + + ); +} + +function Tile({ + figure, + label, + hint, + status, +}: { + figure: string; + label: string; + hint: string; + status: Status; +}) { + return ( +
    +
    {figure}
    + + + {label} + + + } + aria-label={label} > - {t(`identities.rates.${key}`)} - -
    - ))} + + + {hint} + +
    ); } @@ -223,13 +297,26 @@ function QueueGroup({ const key = itemKey(item); const selected = key === selectedKey; return ( -
    {item.candidates.length > 0 ? ( -
    +
    {item.candidates.map((candidate) => ( ))}
    ) : null} - +
    ); })} diff --git a/src/frontend/src/components/portal/person-cell.test.tsx b/src/frontend/src/components/portal/person-cell.test.tsx index 9d1a68536..525ebab89 100644 --- a/src/frontend/src/components/portal/person-cell.test.tsx +++ b/src/frontend/src/components/portal/person-cell.test.tsx @@ -43,6 +43,33 @@ describe("PersonCell", () => { expect(screen.queryAllByText(/a@example\.com/)).toHaveLength(1); }); + // Two records of one human is the normal shape of a conflict, so name and + // address are exactly the fields that fail to tell them apart. + it("always shows the person id, and offers it for copying", () => { + render(); + + expect( + screen.getByText("01900000-0000-7000-8000-000000000001"), + ).toBeInTheDocument(); + expect( + screen.getByRole("button", { + name: /copy 01900000-0000-7000-8000-000000000001/i, + }), + ).toBeInTheDocument(); + }); + + // A person minted at first sign-in carries no attributes until the resolver + // attaches the roster's; printing its id as a name would state the id twice + // and imply the journal knows something it does not. + it("reads an attribute-less person as unnamed rather than naming it by its id", () => { + render(); + + expect(screen.getByText(/unnamed person/i)).toBeInTheDocument(); + expect( + screen.getAllByText("01900000-0000-7000-8000-000000000001"), + ).toHaveLength(1); + }); + it("marks a terminated person", () => { render( - {getInitials(name)} + {named ? getInitials(resolved) : "?"}
    - {name} + + {name} + {person.status?.trim().toLowerCase() === TERMINATED ? ( {detail}
    ) : null} +
    + + {person.person_id} + + +
    ); diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index 5384749c6..ab96fd0db 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -84,7 +84,10 @@ "team_summary": "Team summary", "attention": "Attention needed", "members": "Members" - } + }, + "copy": "Copy", + "copied": "Copied", + "copy_failed": "Unable to copy" }, "theme": { "light": "Light", @@ -402,16 +405,23 @@ "title": "Identities", "subtitle": "Accounts the resolver could not decide — review and resolve them.", "rates": { + "decisions": "Needs a decision", + "decisions_hint": "Cases waiting for you. This is the queue below — the only number here you can act on.", "observed": "Observed", - "bound": "Resolved", - "pending": "Pending review", - "no_evidence": "No evidence", - "excluded": "Excluded" + "observed_hint": "Every account a connector currently reports, closed ones excluded.", + "bound": "Bound to a person", + "bound_hint": "The account belongs to a person. Its activity counts towards them.", + "pending": "Unbound · has an address", + "pending_hint": "Waiting for the resolver, not for you: an address is present, so the next run binds it. Only the contested ones reach the queue.", + "no_evidence": "Unbound · nothing to match on", + "no_evidence_hint": "No address, so automation has no key to match on. Only a person can bind these — they are in the queue.", + "excluded": "Excluded", + "excluded_hint": "Declared not a person — a bot, CI or service account. Dropped from every person metric." }, "kind": { "contested": "Contested — more than one person claims the account", "binding_conflict": "Binding conflicts — evidence disagrees with the current binding", - "no_evidence": "No evidence — nothing links the account to anyone", + "no_evidence": "No address to match on — only a person can bind these", "other": "Needs review" }, "queue": { @@ -433,7 +443,9 @@ "no_history": "No decisions recorded yet." }, "person": { - "terminated": "terminated" + "terminated": "terminated", + "unnamed": "Unnamed person", + "copy_id": "Copy the person id" }, "history": { "bind": "Bound", @@ -442,7 +454,8 @@ "exclude": "Excluded", "automatic": "Automatic", "to": "to", - "by_operator": "by an operator" + "by_operator": "by an operator", + "login_bootstrap": "First sign-in" }, "actions": { "bind": "Bind", diff --git a/src/frontend/src/mocks/handlers.ts b/src/frontend/src/mocks/handlers.ts index 393f27870..0b1f7bea8 100644 --- a/src/frontend/src/mocks/handlers.ts +++ b/src/frontend/src/mocks/handlers.ts @@ -421,9 +421,18 @@ export const handlers = [ person_id: carol?.person_id, author_person_id: "00000000-0000-0000-0000-000000000000", by_operator: false, - reason: null, + // The resolver's own rows carry an EMPTY reason, never null — + // mirroring the real column, which a nullish fallback misses. + reason: "", recorded_at: "2026-07-15T08:00:00.000000", }, + { + person_id: carol?.person_id, + author_person_id: "00000000-0000-0000-0000-000000000000", + by_operator: false, + reason: "login-bootstrap", + recorded_at: "2026-07-01T06:30:00.000000", + }, ], }); }, From c4e9003f4104a4e72ae02c278077749cceb0b8d6 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 03:49:48 +0300 Subject: [PATCH 02/19] frontend: decide a case in a window, not in a column MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The panel was where every correction was taken, in the narrowest thing on the page. Addresses truncated to an ellipsis in the bound person, in both candidates, and in the person picker — on a surface whose whole job is telling two records of one human apart, and where the address is the field that does it. The verbs wrapped onto their own rows underneath. It is a dialog now, opened by the `?acct=` already in the URL rather than by click state, so a shared link still lands a colleague on the same case. The queue takes the freed column: full-width rows, the person id and its copy control on every card. Inside, the decisions sit beside the trail behind them and the history scrolls on its own — an account decided a dozen times can no longer push the verbs out of reach. The confirmations now say what changes rather than name the verb again. Re-asserting the binding in force is not a move: "Confirm" said the account would move to the person it already belongs to, which is the one thing that does NOT happen. A detach names who the account stops counting towards. The row also stopped announcing itself as its entire contents — both candidate cards, their ids and their copy buttons, read out before the account was named. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../portal/account-actions.test.tsx | 28 ++++ .../src/components/portal/account-actions.tsx | 23 ++- .../src/components/portal/account-detail.tsx | 139 ++++++++--------- .../portal/identities-view.test.tsx | 15 ++ .../src/components/portal/identities-view.tsx | 147 +++++++++++------- src/frontend/src/locales/en/translation.json | 15 +- 6 files changed, 223 insertions(+), 144 deletions(-) diff --git a/src/frontend/src/components/portal/account-actions.test.tsx b/src/frontend/src/components/portal/account-actions.test.tsx index 0f76d9ca5..a1e773e42 100644 --- a/src/frontend/src/components/portal/account-actions.test.tsx +++ b/src/frontend/src/components/portal/account-actions.test.tsx @@ -125,6 +125,34 @@ describe("AccountActions", () => { ); }); + // A confirmation an operator can consent to has to say what changes. + // Re-asserting the binding in force changes no binding at all — the + // description that suits a bind ("the account moves to X") states the + // opposite of what happens here. + it("tells a confirm apart from a bind in what it promises", async () => { + render( + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Confirm" })); + expect( + within(screen.getByRole("dialog")).getByText(/binding does not change/i), + ).toBeInTheDocument(); + }); + + it("names who a detach takes the account away from", async () => { + render( + , + ); + + await userEvent.click( + screen.getByRole("button", { name: /detach into a new person/i }), + ); + expect( + within(screen.getByRole("dialog")).getByText(/stops counting towards Bob Park/i), + ).toBeInTheDocument(); + }); + it("the merge dialog previews what moves before anything happens", async () => { hooks.personAccounts.data = { person_id: BOB.person_id, diff --git a/src/frontend/src/components/portal/account-actions.tsx b/src/frontend/src/components/portal/account-actions.tsx index 21b665bfc..6e15852ee 100644 --- a/src/frontend/src/components/portal/account-actions.tsx +++ b/src/frontend/src/components/portal/account-actions.tsx @@ -72,6 +72,8 @@ export function AccountActions({ id: accountRef.account_id, }; const boundId = binding.person_id ?? null; + const boundCard = candidates.find((c) => c.person_id === boundId); + const boundName = boundCard ? personDisplayName(boundCard) : boundId; const close = () => { setAction({ kind: "closed" }); @@ -167,9 +169,13 @@ export function AccountActions({ ? t("identities.dialogs.confirm_title") : t("identities.dialogs.bind_title") } - description={t("identities.dialogs.bind_description", { - name: personDisplayName(action.person), - })} + description={ + action.person.person_id === boundId + ? t("identities.dialogs.confirm_description") + : t("identities.dialogs.bind_description", { + name: personDisplayName(action.person), + }) + } confirmLabel={ action.person.person_id === boundId ? t("identities.actions.confirm") @@ -220,7 +226,16 @@ export function AccountActions({ open onOpenChange={(open) => !open && close()} title={t("identities.dialogs.detach_title")} - description={t("identities.dialogs.detach_description")} + // Naming who it stops counting towards is the consequence; without + // the current holder the sentence would describe only the new row. + description={[ + t("identities.dialogs.detach_description"), + boundName + ? t("identities.dialogs.detach_away_from", { name: boundName }) + : null, + ] + .filter(Boolean) + .join(" ")} confirmLabel={t("identities.actions.detach_confirm")} isPending={detach.isPending} error={ diff --git a/src/frontend/src/components/portal/account-detail.tsx b/src/frontend/src/components/portal/account-detail.tsx index 63de277bb..813e3fe0b 100644 --- a/src/frontend/src/components/portal/account-detail.tsx +++ b/src/frontend/src/components/portal/account-detail.tsx @@ -1,11 +1,11 @@ /** - * The detail panel for one selected account: what the resolver currently - * thinks (the binding), who it could belong to (the queue's hydrated - * candidates), and every decision ever recorded (the history — the journal is - * append-only, so this trail is complete by construction). + * One account under review — the body of the case window: what the resolver + * currently thinks (the binding), who it could belong to (the queue's + * hydrated candidates), and every decision ever recorded (the history — the + * journal is append-only, so this trail is complete by construction). * - * The panel answers a shared `?acct=` link even when the account is no longer - * in the queue. The binding read never 404s: an account nobody ever observed + * It answers for an account no longer in the queue, which is what a shared + * link lands on. The binding read never 404s: an account nobody ever observed * or decided answers 200 with an empty journal, so "not in the queue, no * binding, no history" is the stale-link state — it says so instead of * offering verbs whose bind would pre-register a typo as a real account. @@ -23,7 +23,6 @@ import type { import { AccountActions } from "@/components/portal/account-actions"; import { PersonCell } from "@/components/portal/person-cell"; import { Badge } from "@/components/ui/badge"; -import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { CenteredSpinner } from "@/components/widgets/centered-spinner"; import { ComingSoon } from "@/components/widgets/coming-soon"; import type { AccountRef } from "@/lib/identities/account-key"; @@ -52,17 +51,15 @@ export function AccountDetail({ const { t } = useTranslation(); const binding = useAccountBinding(accountRef); - if (binding.isLoading) return ; + if (binding.isLoading) return ; if (binding.isError) { return ( - - void binding.refetch()} - /> - + void binding.refetch()} + /> ); } if (!binding.data) return null; @@ -73,13 +70,11 @@ export function AccountDetail({ binding.data.history.length === 0; if (neverSeen) { return ( - - - + ); } @@ -89,59 +84,50 @@ export function AccountDetail({ ); return ( - - - - - {queueItem?.email?.trim() || - queueItem?.username?.trim() || - accountRef.account_id} - -
    - {accountRef.source} · {accountRef.account_id} -
    -     - -
    - {t("identities.detail.current_binding")} - {binding.data.person_id ? ( - boundCard ? ( - - ) : ( - - ) - ) : ( -

    - {t("identities.detail.unbound")} -

    - )} -
    - -
    - {t("identities.detail.history")} - {binding.data.history.length === 0 ? ( -

    - {t("identities.detail.no_history")} -

    + // Decisions on the start side, the trail behind them on the end side — + // and the trail scrolls on its own, so a long-lived account cannot push + // the verbs out of reach. +
    +
    +
    + {t("identities.detail.current_binding")} + {binding.data.person_id ? ( + boundCard ? ( + ) : ( -
      - {binding.data.history.map((entry, index) => ( - - ))} -
    - )} -
    - - - + + ) + ) : ( +

    + {t("identities.detail.unbound")} +

    + )} +
    + + +
    + {t("identities.detail.history")} + {binding.data.history.length === 0 ? ( +

    + {t("identities.detail.no_history")} +

    + ) : ( +
      + {binding.data.history.map((entry, index) => ( + + ))} +
    + )} +
    + ); } @@ -199,6 +185,3 @@ function SectionLabel({ children }: { children: React.ReactNode }) { ); } -function PanelShell({ children }: { children: React.ReactNode }) { - return
    {children}
    ; -} diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index 3d29f31c5..5835835d4 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -242,6 +242,21 @@ describe("IdentitiesView", () => { expect(portalRouter.search.acct).toBeUndefined(); }); + // The case is where every decision is taken, so it gets a window rather + // than the leftover column — and the window is opened by the URL, so a + // shared link lands a colleague on the same case rather than on a queue. + it("opens the case in a window, and closing it clears the shared link", async () => { + attention.q.data = { items: [item({})], rates: RATES }; + render(); + + await userEvent.click(screen.getByRole("button", { name: /dev42@example\.com/i })); + const dialog = screen.getByRole("dialog"); + expect(within(dialog).getByText(/github · dev-42/i)).toBeInTheDocument(); + + await userEvent.click(screen.getByRole("button", { name: /close/i })); + expect(portalRouter.search.acct).toBeUndefined(); + }); + it("offers a retry on a failed load", async () => { attention.q.isError = true; render(); diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 63bf8ac18..1b078d888 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -1,21 +1,30 @@ /** - * The identity-resolution operator console (Manage → Identities), phase 1: - * the review queue, read-only. + * The identity-resolution operator console (Manage → Identities): the review + * queue, and the window one case is decided in. * * A triage surface, not a roster: the operator lands in what NEEDS a * decision, grouped by why it does, and works the backlog to zero — the - * empty queue is the goal state and renders as one. The tenant-wide rates - * strip on top is honest about scale: it counts every observed account, - * never just the visible page. + * empty queue is the goal state and renders as one. The rates strip on top + * counts binding states across the tenant; only its first figure, the queue's + * own size, is work the operator can do. * - * Selection lives in `?acct=` so an operator can hand a colleague a link to - * the exact account under discussion — and that link answers whatever the - * queue looks like by then, an emptied backlog included. + * The queue picks a case; the window decides it. Selection lives in `?acct=` + * so an operator can hand a colleague a link to the exact account under + * discussion — and that link answers whatever the queue looks like by then, + * an emptied backlog included. */ import { useTranslation } from "react-i18next"; import type { AttentionItem, ResolutionRates } from "@/api/identity-client"; import { Alert, AlertDescription } from "@/components/ui/alert"; +import { CopyValueButton } from "@/components/copy-value-button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; import { CenteredSpinner } from "@/components/widgets/centered-spinner"; import { PersonCell } from "@/components/portal/person-cell"; import { Badge } from "@/components/ui/badge"; @@ -42,7 +51,7 @@ import { useAttention } from "@/queries/identity-resolution"; import { TEXT_FIGURE, TEXT_LABEL } from "@/lib/type-scale"; import { STATUS_SURFACE_CLASS, type Status } from "@/lib/status"; import { cn } from "@/lib/utils"; -import { Info, PartyPopper, TriangleAlert, UserSearch } from "lucide-react"; +import { Info, PartyPopper, TriangleAlert } from "lucide-react"; /** Queue groups in working order: conflicts first, then the unknowns. */ const KIND_ORDER = ["contested", "binding_conflict", "no_evidence"] as const; @@ -230,46 +239,86 @@ function Queue({ items }: { items: AttentionItem[] }) { // The worked-to-zero queue is the goal state — but a shared `?acct=` link // has to answer even then, and the backlog reaching zero is exactly when a - // colleague opens the link they were sent. So the celebration replaces the - // GROUP LIST, never the grid the detail panel lives in. - if (items.length === 0 && !acct) return ; - + // colleague opens the link they were sent. return ( -
    -
    - {items.length === 0 ? ( - - ) : ( - groups.map((group) => ( - setAcct(key === acct ? null : key)} - /> - )) - )} -
    - +
    + {items.length === 0 ? ( + + ) : ( + groups.map((group) => ( + setAcct(key === acct ? null : key)} + /> + )) + )} + setAcct(null)} />
    ); } -function DetailPane({ +/** + * One account under review, in a window rather than a column: this is where + * every decision is taken, and a decision that re-attributes a person's work + * deserves the room to show what it acts on. + * + * Opened by the `?acct=` in the URL — never by click state alone — so a link + * an operator shares lands their colleague on the same case. + */ +function CaseDialog({ acct, items, + onClose, }: { acct: string | undefined; items: AttentionItem[]; + onClose: () => void; }) { + const { t } = useTranslation(); const ref = parseAccountKey(acct); - if (!ref) return ; const queueItem = items.find((i) => itemKey(i) === acct); - // Keyed by the account: the panel holds per-account state (a verb's outcome - // alert, an open dialog), and a cached binding renders the next selection - // synchronously — an unkeyed panel would carry that state across accounts. - return ; + const heading = + queueItem?.email?.trim() || queueItem?.username?.trim() || ref?.account_id; + + return ( + { + if (!open) onClose(); + }} + > + + + {heading} + } + > + + {ref?.source} · {ref?.account_id} + + {ref ? ( + + ) : null} + + + {/* Keyed by the account: the body holds per-account state (a verb's + outcome, an open confirmation), and a cached binding renders the + next case synchronously — unkeyed, that state would follow. */} + {ref ? ( + + ) : null} + + + ); } function QueueGroup({ @@ -296,6 +345,8 @@ function QueueGroup({ {items.map((item) => { const key = itemKey(item); const selected = key === selectedKey; + const label = + item.email?.trim() || item.username?.trim() || item.account_id; return (
    - - {item.email?.trim() || item.username?.trim() || item.account_id} - + {label} {item.source} @@ -345,19 +398,3 @@ function QueueGroup({ ); } -function DetailPlaceholder() { - const { t } = useTranslation(); - return ( - - - - - - {t("identities.detail.no_selection")} - - {t("identities.detail.no_selection_description")} - - - - ); -} diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index ab96fd0db..c47b5f3b3 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -432,15 +432,14 @@ "empty_description": "Every observed account is bound to a person. New conflicts will appear here." }, "detail": { - "no_selection": "No account selected", - "no_selection_description": "Pick an account from the queue to see its evidence and history.", "not_found": "This account is unknown — the link may be stale.", "load_failed": "Unable to load the account", "current_binding": "Currently bound to", "unbound": "Nobody — the account is unresolved.", "candidates": "Candidates", "history": "Decision history", - "no_history": "No decisions recorded yet." + "no_history": "No decisions recorded yet.", + "copy_account_id": "Copy the account id" }, "person": { "terminated": "terminated", @@ -471,7 +470,7 @@ "dialogs": { "bind_title": "Bind this account?", "confirm_title": "Confirm the current binding?", - "bind_description": "The account will belong to {{name}} from here on.", + "bind_description": "This account moves to {{name}}. Its activity counts towards them from here on — another bind moves it back.", "merge_title": "Merge two persons?", "merge_description": "Every account of the currently bound person moves to {{name}}. Merging back is another merge — nothing is lost, but metrics re-attribute.", "merge_preview_loading": "Counting the accounts that would move…", @@ -480,10 +479,12 @@ "merge_preview_other": "{{count}} accounts move:", "merge_preview_more": "…and {{count}} more.", "detach_title": "Detach into a new person?", - "detach_description": "The account gets a freshly minted person of its own.", + "detach_description": "A new person is created holding this account alone.", "exclude_title": "Exclude this account?", - "exclude_description": "Bots, CI and service accounts are excluded from every person metric. Reversible by binding the account to a person later.", - "failed": "The correction was not applied. Try again." + "exclude_description": "Bots, CI and service accounts are excluded from every person metric. Reversible: binding the account to a person later brings it back.", + "failed": "The correction was not applied. Try again.", + "confirm_description": "The binding does not change. This records that you decided it, not the resolver, and takes the account off the queue.", + "detach_away_from": "Its activity stops counting towards {{name}}." }, "outcomes": { "applied_one": "{{count}} applied", From a66dd031b9250a96ee48d2fae6144d81f6da19ba Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 04:44:12 +0300 Subject: [PATCH 03/19] frontend: one argument is one case, not one row per account MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A binding conflict flags every account sharing the disputed value, and each of those rows carries the identical candidate list — so a single split person arrived as five near-identical rows. The rows are ordered by source, which then interleaved them with every other case holding an account in the same connector, and the queue read as a scattered list of unrelated problems. The candidate set is what identifies the case: it is exactly "who is being argued over". Accounts sharing one now render under a single block that states those people once, with a row per account beneath — the row a decision is still taken on. Accounts with nothing to match on stay cases of their own; collapsing them would claim an argument that does not exist. A group also says which connectors it spans, folds away, and stops at ten cases with the rest a press away — the largest group could otherwise bury the more urgent ones under it. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../portal/identities-view.test.tsx | 36 ++++- .../src/components/portal/identities-view.tsx | 153 +++++++++++++++--- src/frontend/src/lib/identities/cases.test.ts | 72 +++++++++ src/frontend/src/lib/identities/cases.ts | 46 ++++++ src/frontend/src/locales/en/translation.json | 8 +- 5 files changed, 285 insertions(+), 30 deletions(-) create mode 100644 src/frontend/src/lib/identities/cases.test.ts create mode 100644 src/frontend/src/lib/identities/cases.ts diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index 5835835d4..eda14620e 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -1,10 +1,12 @@ // @vitest-environment jsdom /** - * The review queue, phase 1. What matters: the empty queue is a celebrated - * goal state, not a blank table; groups come in working order with honest - * counts and an unknown kind still shows up (the vocabulary is open by - * contract); selection lives in the URL so an operator can share a link; and - * the rates strip shows the tenant-wide counts, not the page's. + * The review queue. What matters: the empty queue is a celebrated goal state, + * not a blank table; groups come in working order with honest counts and an + * unknown kind still shows up (the vocabulary is open by contract); accounts + * arguing over the same people are ONE case rather than as many rows as the + * server sends; selection lives in the URL so an operator can share a link; + * and the strip leads with the queue's own size — the one figure the operator + * can act on — over tenant-wide binding states. */ import { render, screen, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; @@ -164,6 +166,30 @@ describe("IdentitiesView", () => { expect(screen.getByText("q-1")).toBeInTheDocument(); }); + // Five rows repeating the same two candidates read as five problems. The + // people are stated once for the case; the rows underneath are the accounts + // each decision is taken on. + it("shows one case for the accounts arguing over the same people", () => { + const candidates = [ + { person_id: "01900000-0000-7000-8000-0000000000a0", display_name: "Ann Lee" }, + { person_id: "01900000-0000-7000-8000-0000000000b0", display_name: "Bob Park" }, + ]; + attention.q.data = { + items: [ + item({ kind: "binding_conflict", account_id: "a1", source: "hr", candidates }), + item({ kind: "binding_conflict", account_id: "a2", source: "wiki", candidates }), + item({ kind: "binding_conflict", account_id: "a3", source: "chat", candidates }), + ], + rates: RATES, + }; + render(); + + expect(screen.getAllByText("Ann Lee")).toHaveLength(1); + expect(screen.getByText(/1 case · 3 accounts/i)).toBeInTheDocument(); + // Each account still has its own row: a decision is taken per account. + expect(screen.getAllByRole("button", { name: /dev42@example\.com/i })).toHaveLength(3); + }); + it("renders candidates as person cells", () => { attention.q.data = { items: [ diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 1b078d888..a750c85a3 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -13,6 +13,7 @@ * discussion — and that link answers whatever the queue looks like by then, * an emptied backlog included. */ +import { useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; import type { AttentionItem, ResolutionRates } from "@/api/identity-client"; @@ -28,7 +29,13 @@ import { import { CenteredSpinner } from "@/components/widgets/centered-spinner"; import { PersonCell } from "@/components/portal/person-cell"; import { Badge } from "@/components/ui/badge"; -import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { Button } from "@/components/ui/button"; +import { Card, CardContent, CardTitle } from "@/components/ui/card"; +import { + Collapsible, + CollapsibleContent, + CollapsibleTrigger, +} from "@/components/ui/collapsible"; import { Empty, EmptyDescription, @@ -46,12 +53,13 @@ import { ComingSoon } from "@/components/widgets/coming-soon"; import { usePortalSearch } from "@/lib/portal/portal-search"; import { usePortalNavActions } from "@/lib/portal/portal-nav"; import { itemKey, parseAccountKey } from "@/lib/identities/account-key"; +import { groupIntoCases, type QueueCase } from "@/lib/identities/cases"; import { AccountDetail } from "@/components/portal/account-detail"; import { useAttention } from "@/queries/identity-resolution"; import { TEXT_FIGURE, TEXT_LABEL } from "@/lib/type-scale"; import { STATUS_SURFACE_CLASS, type Status } from "@/lib/status"; import { cn } from "@/lib/utils"; -import { Info, PartyPopper, TriangleAlert } from "lucide-react"; +import { ChevronDown, Info, PartyPopper, TriangleAlert } from "lucide-react"; /** Queue groups in working order: conflicts first, then the unknowns. */ const KIND_ORDER = ["contested", "binding_conflict", "no_evidence"] as const; @@ -321,6 +329,9 @@ function CaseDialog({ ); } +/** Cases rendered before the group asks to be expanded further. */ +const CASE_PAGE = 10; + function QueueGroup({ kind, items, @@ -333,16 +344,118 @@ function QueueGroup({ onSelect: (key: string) => void; }) { const { t } = useTranslation(); + const [shownCases, setShownCases] = useState(CASE_PAGE); + const cases = useMemo(() => groupIntoCases(items), [items]); + const visible = cases.slice(0, shownCases); + const hidden = cases.length - visible.length; + + return ( + + + + } + > + + + {t(`identities.kind.${kind}`, { defaultValue: kind })} + + {cases.length === items.length + ? items.length + : t("identities.queue.case_count", { + count: cases.length, + accounts: items.length, + })} + + + + + + + {visible.map((queueCase) => ( + + ))} + {hidden > 0 ? ( + + ) : null} + + + + + ); +} + +/** Which connectors this group's accounts came from, so a glance places it. */ +function SourceCounts({ items }: { items: AttentionItem[] }) { + const counts = new Map(); + for (const item of items) { + counts.set(item.source, (counts.get(item.source) ?? 0) + 1); + } return ( - - - - {t(`identities.kind.${kind}`, { defaultValue: kind })} - {items.length} - - - - {items.map((item) => { + + {[...counts.entries()] + .sort((a, b) => b[1] - a[1]) + .map(([source, count]) => ( + + {source} {count} + + ))} + + ); +} + +/** + * One argument, however many accounts it spans: the people under discussion + * once at the top, then the accounts each decision is taken on. + */ +function CaseBlock({ + queueCase, + selectedKey, + onSelect, +}: { + queueCase: QueueCase; + selectedKey: string | undefined; + onSelect: (key: string) => void; +}) { + const { t } = useTranslation(); + const disputed = queueCase.candidates.length > 0; + return ( +
    + {disputed ? ( +
    +
    + {t("identities.queue.case_summary", { + people: queueCase.candidates.length, + count: queueCase.items.length, + })} +
    + {queueCase.candidates.map((candidate) => ( + + ))} +
    + ) : null} +
    + {queueCase.items.map((item) => { const key = itemKey(item); const selected = key === selectedKey; const label = @@ -353,7 +466,7 @@ function QueueGroup({ // Not a
    - {item.candidates.length > 0 ? ( -
    - {item.candidates.map((candidate) => ( - - ))} -
    - ) : null}
    ); })} -     -     +
    +
    ); } diff --git a/src/frontend/src/lib/identities/cases.test.ts b/src/frontend/src/lib/identities/cases.test.ts new file mode 100644 index 000000000..cace9343c --- /dev/null +++ b/src/frontend/src/lib/identities/cases.test.ts @@ -0,0 +1,72 @@ +/** + * A conflict flags every account sharing the disputed value, each row + * carrying the same candidates — so the queue showed one split person as five + * problems, and ordering by source scattered them among the others. Grouping + * by "who is being argued over" is what puts one case back together. + */ +import { describe, expect, it } from "vitest"; + +import type { AttentionItem } from "@/api/identity-client"; + +import { groupIntoCases } from "./cases"; + +const ANN = { person_id: "01900000-0000-7000-8000-0000000000a0", display_name: "Ann Lee" }; +const BOB = { person_id: "01900000-0000-7000-8000-0000000000b0", display_name: "Bob Park" }; + +function item(over: Partial): AttentionItem { + return { + kind: "binding_conflict", + source: "github", + source_id: "01900000-0000-7000-8000-00000000aa01", + account_id: "a1", + email: "dev@example.com", + username: null, + candidates: [ANN, BOB], + ...over, + }; +} + +describe("groupIntoCases", () => { + it("collects every account arguing over the same people into one case", () => { + const cases = groupIntoCases([ + item({ account_id: "a1", source: "github" }), + item({ account_id: "a2", source: "gitlab" }), + item({ account_id: "a3", source: "hr" }), + ]); + + expect(cases).toHaveLength(1); + expect(cases[0]?.items).toHaveLength(3); + expect(cases[0]?.candidates).toEqual([ANN, BOB]); + }); + + // The server sorts candidates, but a case must not split because a future + // read hands the same two people back in the other order. + it("is blind to the order the candidates arrive in", () => { + const cases = groupIntoCases([ + item({ account_id: "a1", candidates: [ANN, BOB] }), + item({ account_id: "a2", candidates: [BOB, ANN] }), + ]); + + expect(cases).toHaveLength(1); + }); + + it("keeps separate arguments apart", () => { + const cases = groupIntoCases([ + item({ account_id: "a1", candidates: [ANN, BOB] }), + item({ account_id: "a2", candidates: [ANN] }), + ]); + + expect(cases).toHaveLength(2); + }); + + // Nothing to match on means nothing to join them by: collapsing these would + // claim an argument that does not exist. + it("leaves candidate-less accounts as cases of their own", () => { + const cases = groupIntoCases([ + item({ account_id: "a1", kind: "no_evidence", candidates: [] }), + item({ account_id: "a2", kind: "no_evidence", candidates: [] }), + ]); + + expect(cases).toHaveLength(2); + }); +}); diff --git a/src/frontend/src/lib/identities/cases.ts b/src/frontend/src/lib/identities/cases.ts new file mode 100644 index 000000000..45a00b422 --- /dev/null +++ b/src/frontend/src/lib/identities/cases.ts @@ -0,0 +1,46 @@ +/** + * Accounts arguing over the same people are ONE case, however many rows the + * server sends. + * + * A binding conflict flags every account that shares the disputed value, and + * each of those rows carries the identical candidate list — so a single split + * person reads as five near-identical rows, and the queue looks like five + * problems. Worse, the rows are ordered by source, which interleaves them with + * every other case that happens to have an account in the same connector. + * + * The candidate set is what identifies the case: it is exactly "who is being + * argued over". Accounts with no candidates (nothing to match on) are each + * their own case — there is no argument to join them by. + */ +import type { AttentionItem, PersonSummary } from "@/api/identity-client"; +import { itemKey } from "@/lib/identities/account-key"; + +export interface QueueCase { + /** Stable within one queue read — a render key, never an identifier. */ + key: string; + /** The people under discussion, once for the whole case. */ + candidates: PersonSummary[]; + /** Every account the case covers, in the order the server sent them. */ + items: AttentionItem[]; +} + +function caseKey(item: AttentionItem): string { + if (item.candidates.length === 0) return `account:${itemKey(item)}`; + const ids = item.candidates.map((c) => c.person_id).sort(); + return `people:${ids.join("|")}`; +} + +/** Group one kind's rows into cases, first-seen order kept. */ +export function groupIntoCases(items: AttentionItem[]): QueueCase[] { + const byKey = new Map(); + for (const item of items) { + const key = caseKey(item); + const existing = byKey.get(key); + if (existing) { + existing.items.push(item); + continue; + } + byKey.set(key, { key, candidates: item.candidates, items: [item] }); + } + return [...byKey.values()]; +} diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index c47b5f3b3..3df87301c 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -429,7 +429,13 @@ "items_truncated": "Only the first accounts needing review are listed — resolve these and reload to see the rest.", "truncated": "The evidence read hit its cap — these counts and this queue cover only part of the observed accounts, not the whole tenant.", "empty_title": "Everything is resolved", - "empty_description": "Every observed account is bound to a person. New conflicts will appear here." + "empty_description": "Every observed account is bound to a person. New conflicts will appear here.", + "case_summary_one": "{{people}} people, {{count}} account", + "case_summary_other": "{{people}} people, {{count}} accounts", + "show_more_one": "Show {{count}} more case", + "show_more_other": "Show {{count}} more cases", + "case_count_one": "{{count}} case · {{accounts}} accounts", + "case_count_other": "{{count}} cases · {{accounts}} accounts" }, "detail": { "not_found": "This account is unknown — the link may be stale.", From 60cf2e6ce3fbb3273a387f5f9f3c58ee88b38fa7 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 04:47:06 +0300 Subject: [PATCH 04/19] frontend: narrow the queue to what you are looking for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A group can hold more accounts than anyone scrolls through, and the operator usually arrives knowing what they are after: a person, a connector, an address someone pasted into a chat. There was no way to say so. The filter matches anything visible on a row — the account's own values, its source, and the candidates, including their person ids, which is what an operator copies off a card and pastes back. Every term must match, so a second word narrows. It rides in the URL like the rest of the portal's state, written on a pause in typing and replacing rather than pushing, so Back leaves the surface instead of walking backwards through the typing. Two things it must not do: celebrate an empty result — no matches is a mistyped filter, not a finished backlog — and hide a case a shared link points at, so the window is fed from the unfiltered queue. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../portal/identities-view.test.tsx | 47 +++++++- .../src/components/portal/identities-view.tsx | 109 ++++++++++++++---- src/frontend/src/lib/identities/cases.test.ts | 42 ++++++- src/frontend/src/lib/identities/cases.ts | 38 ++++++ src/frontend/src/lib/portal/portal-search.ts | 4 + src/frontend/src/locales/en/translation.json | 5 +- 6 files changed, 222 insertions(+), 23 deletions(-) diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index eda14620e..87157be82 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -8,7 +8,7 @@ * and the strip leads with the queue's own size — the one figure the operator * can act on — over tenant-wide binding states. */ -import { render, screen, within } from "@testing-library/react"; +import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -283,6 +283,51 @@ describe("IdentitiesView", () => { expect(portalRouter.search.acct).toBeUndefined(); }); + it("narrows the queue by anything on a row, and carries the filter in the URL", async () => { + attention.q.data = { + items: [ + item({ account_id: "a1", email: "ann@example.com" }), + item({ account_id: "a2", email: "bob@example.com" }), + ], + rates: RATES, + }; + render(); + + await userEvent.type(screen.getByRole("searchbox"), "ann@"); + await waitFor(() => expect(portalRouter.search.filter).toBe("ann@")); + expect(screen.getByRole("button", { name: /ann@example\.com/i })).toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: /bob@example\.com/i }), + ).not.toBeInTheDocument(); + }); + + // Celebrating here would tell an operator the backlog is done because they + // mistyped a filter. + it("does not celebrate an empty result — a filter that matches nothing says so", () => { + attention.q.data = { items: [item({})], rates: RATES }; + portalRouter.set({ zone: "manage", item: "identities", filter: "nobody" }); + render(); + + expect(screen.getByText(/nothing matches those terms/i)).toBeInTheDocument(); + expect(screen.queryByText(/everything is resolved/i)).not.toBeInTheDocument(); + }); + + // A colleague's link points at a row the reader's own filter hides; the + // case must still open, or the link is only as good as the recipient's + // current view. + it("answers a shared ?acct= link even while a filter hides its row", () => { + attention.q.data = { items: [item({})], rates: RATES }; + portalRouter.set({ + zone: "manage", + item: "identities", + filter: "nobody", + acct: "github:01900000-0000-7000-8000-00000000aa01:dev-42", + }); + render(); + + expect(screen.getByRole("dialog")).toBeInTheDocument(); + }); + it("offers a retry on a failed load", async () => { attention.q.isError = true; render(); diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index a750c85a3..1a7ce734c 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -13,7 +13,7 @@ * discussion — and that link answers whatever the queue looks like by then, * an emptied backlog included. */ -import { useMemo, useState } from "react"; +import { useEffect, useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; import type { AttentionItem, ResolutionRates } from "@/api/identity-client"; @@ -31,6 +31,7 @@ import { PersonCell } from "@/components/portal/person-cell"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Card, CardContent, CardTitle } from "@/components/ui/card"; +import { Input } from "@/components/ui/input"; import { Collapsible, CollapsibleContent, @@ -50,16 +51,30 @@ import { TooltipTrigger, } from "@/components/ui/tooltip"; import { ComingSoon } from "@/components/widgets/coming-soon"; -import { usePortalSearch } from "@/lib/portal/portal-search"; +import { useDebouncedValue } from "@/hooks/use-debounced-value"; +import { + usePortalSearch, + useSetPortalSearch, +} from "@/lib/portal/portal-search"; import { usePortalNavActions } from "@/lib/portal/portal-nav"; import { itemKey, parseAccountKey } from "@/lib/identities/account-key"; -import { groupIntoCases, type QueueCase } from "@/lib/identities/cases"; +import { + filterQueue, + groupIntoCases, + type QueueCase, +} from "@/lib/identities/cases"; import { AccountDetail } from "@/components/portal/account-detail"; import { useAttention } from "@/queries/identity-resolution"; import { TEXT_FIGURE, TEXT_LABEL } from "@/lib/type-scale"; import { STATUS_SURFACE_CLASS, type Status } from "@/lib/status"; import { cn } from "@/lib/utils"; -import { ChevronDown, Info, PartyPopper, TriangleAlert } from "lucide-react"; +import { + ChevronDown, + Info, + PartyPopper, + Search, + TriangleAlert, +} from "lucide-react"; /** Queue groups in working order: conflicts first, then the unknowns. */ const KIND_ORDER = ["contested", "binding_conflict", "no_evidence"] as const; @@ -233,9 +248,14 @@ function AllResolved() { ); } -function Queue({ items }: { items: AttentionItem[] }) { - const { acct } = usePortalSearch(); +function Queue({ items: everything }: { items: AttentionItem[] }) { + const { t } = useTranslation(); + const { acct, filter } = usePortalSearch(); const { setAcct } = usePortalNavActions(); + const items = useMemo( + () => filterQueue(everything, filter ?? ""), + [everything, filter], + ); const groups: Array<{ kind: string; items: AttentionItem[] }> = KIND_ORDER.map( (kind) => ({ kind, items: items.filter((i) => i.kind === kind) }), ).filter((g) => g.items.length > 0); @@ -250,20 +270,67 @@ function Queue({ items }: { items: AttentionItem[] }) { // colleague opens the link they were sent. return (
    - {items.length === 0 ? ( - - ) : ( - groups.map((group) => ( - setAcct(key === acct ? null : key)} - /> - )) - )} - setAcct(null)} /> + {everything.length > 0 ? : null} + {/* A filter that matches nothing is not an emptied backlog. Celebrating + there would tell an operator the work is done because they mistyped. */} + {items.length === 0 && everything.length > 0 ? ( + + + {t("identities.queue.no_matches")} + + {t("identities.queue.no_matches_description")} + + + + ) : null} + {items.length === 0 && everything.length === 0 ? : null} + {groups.map((group) => ( + setAcct(key === acct ? null : key)} + /> + ))} + {/* Fed from the unfiltered set: a link stays answerable even when the + reader's own filter hides the row it points at. */} + setAcct(null)} /> +
    + ); +} + +/** + * Narrow the queue by anything visible on a row — an address, a source, a + * candidate's name, a person id pasted back from a card. + * + * The query rides in the URL like every other portal state, so a narrowed + * queue is shareable; it is written on a pause in typing rather than per + * keystroke, and replaces rather than pushes, so Back leaves the surface + * instead of walking the operator backwards through their own typing. + */ +function QueueFilter() { + const { t } = useTranslation(); + const { filter } = usePortalSearch(); + const setSearch = useSetPortalSearch(); + const [query, setQuery] = useState(filter ?? ""); + const debounced = useDebouncedValue(query, FILTER_DEBOUNCE_MS); + + useEffect(() => { + setSearch({ filter: debounced.trim() || undefined }, { replace: true }); + }, [debounced, setSearch]); + + return ( +
    + + setQuery(event.target.value)} + placeholder={t("identities.queue.filter_placeholder")} + aria-label={t("identities.queue.filter_placeholder")} + className="ps-9" + />
    ); } @@ -332,6 +399,8 @@ function CaseDialog({ /** Cases rendered before the group asks to be expanded further. */ const CASE_PAGE = 10; +const FILTER_DEBOUNCE_MS = 250; + function QueueGroup({ kind, items, diff --git a/src/frontend/src/lib/identities/cases.test.ts b/src/frontend/src/lib/identities/cases.test.ts index cace9343c..50eed3e45 100644 --- a/src/frontend/src/lib/identities/cases.test.ts +++ b/src/frontend/src/lib/identities/cases.test.ts @@ -8,7 +8,7 @@ import { describe, expect, it } from "vitest"; import type { AttentionItem } from "@/api/identity-client"; -import { groupIntoCases } from "./cases"; +import { filterQueue, groupIntoCases } from "./cases"; const ANN = { person_id: "01900000-0000-7000-8000-0000000000a0", display_name: "Ann Lee" }; const BOB = { person_id: "01900000-0000-7000-8000-0000000000b0", display_name: "Bob Park" }; @@ -70,3 +70,43 @@ describe("groupIntoCases", () => { expect(cases).toHaveLength(2); }); }); + +describe("filterQueue", () => { + it("matches the account's own values and its source", () => { + const items = [ + item({ account_id: "a1", email: "ann@example.com", source: "hr" }), + item({ account_id: "a2", email: "bob@example.com", source: "wiki" }), + ]; + + expect(filterQueue(items, "wiki").map((i) => i.account_id)).toEqual(["a2"]); + expect(filterQueue(items, "ann@").map((i) => i.account_id)).toEqual(["a1"]); + }); + + // An operator hunting the accounts of one split person searches by the + // person — and by the id they copied off the card, which is the one term + // that cannot land on a namesake. + it("matches a candidate by name and by person id", () => { + const items = [ + item({ account_id: "a1", candidates: [ANN] }), + item({ account_id: "a2", candidates: [BOB] }), + ]; + + expect(filterQueue(items, "ann lee").map((i) => i.account_id)).toEqual(["a1"]); + expect(filterQueue(items, BOB.person_id).map((i) => i.account_id)).toEqual(["a2"]); + }); + + it("requires every term, so a second word narrows rather than widens", () => { + const items = [ + item({ account_id: "a1", source: "hr", candidates: [ANN] }), + item({ account_id: "a2", source: "wiki", candidates: [ANN] }), + ]; + + expect(filterQueue(items, "ann wiki").map((i) => i.account_id)).toEqual(["a2"]); + }); + + it("returns everything for a blank query", () => { + const items = [item({ account_id: "a1" })]; + + expect(filterQueue(items, " ")).toHaveLength(1); + }); +}); diff --git a/src/frontend/src/lib/identities/cases.ts b/src/frontend/src/lib/identities/cases.ts index 45a00b422..6ff066025 100644 --- a/src/frontend/src/lib/identities/cases.ts +++ b/src/frontend/src/lib/identities/cases.ts @@ -30,6 +30,44 @@ function caseKey(item: AttentionItem): string { return `people:${ids.join("|")}`; } +function haystack(item: AttentionItem): string { + return [ + item.email, + item.username, + item.account_id, + item.source, + ...item.candidates.flatMap((c) => [ + c.display_name, + c.email, + c.username, + c.person_id, + ]), + ] + .filter(Boolean) + .join(" ") + .toLowerCase(); +} + +/** + * Narrow the queue to the rows a query names, all terms required. + * + * The candidates are part of what a row IS — an operator hunting the accounts + * of one split person searches by that person, not by the addresses of the + * accounts. The person id matches too, since that is what they can copy off + * a card and paste back. + */ +export function filterQueue( + items: AttentionItem[], + query: string, +): AttentionItem[] { + const terms = query.trim().toLowerCase().split(/\s+/).filter(Boolean); + if (terms.length === 0) return items; + return items.filter((item) => { + const text = haystack(item); + return terms.every((term) => text.includes(term)); + }); +} + /** Group one kind's rows into cases, first-seen order kept. */ export function groupIntoCases(items: AttentionItem[]): QueueCase[] { const byKey = new Map(); diff --git a/src/frontend/src/lib/portal/portal-search.ts b/src/frontend/src/lib/portal/portal-search.ts index 2f371f967..1ac83a607 100644 --- a/src/frontend/src/lib/portal/portal-search.ts +++ b/src/frontend/src/lib/portal/portal-search.ts @@ -26,6 +26,8 @@ export interface PortalSearch { item?: string; /** Selected account inside the Identities surface (an opaque account key). */ acct?: string; + /** Free-text narrowing of the Identities queue. */ + filter?: string; /** Expanded direction + its active lens, within the Directions zone. */ dir?: string; lens?: string; @@ -73,6 +75,7 @@ export function validatePortalSearch(raw: Record): PortalSearch zone: str(raw.zone), item: str(raw.item), acct: str(raw.acct), + filter: str(raw.filter), dir: str(raw.dir), lens: str(raw.lens), // Lower-cased to match `normalizePersonId`: the same id reaches us from a @@ -96,6 +99,7 @@ export const PORTAL_SEARCH_KEYS = [ "zone", "item", "acct", + "filter", "dir", "lens", "scope", diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index 3df87301c..e7f82c800 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -435,7 +435,10 @@ "show_more_one": "Show {{count}} more case", "show_more_other": "Show {{count}} more cases", "case_count_one": "{{count}} case · {{accounts}} accounts", - "case_count_other": "{{count}} cases · {{accounts}} accounts" + "case_count_other": "{{count}} cases · {{accounts}} accounts", + "filter_placeholder": "Filter by address, source, person or person id…", + "no_matches": "Nothing matches those terms", + "no_matches_description": "Clear the filter to see the whole queue." }, "detail": { "not_found": "This account is unknown — the link may be stale.", From 4043b42cbdc1c8ea5271cfc1bead2e213959d220 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 05:07:31 +0300 Subject: [PATCH 05/19] frontend: work the queue like a queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Small things, all of them about the operator's second hour rather than their first minute. A decided account stayed on screen until the refetch came back — and the attention read folds every observed account, which can be seconds of looking at work already done. The rows the SERVER reported as decided now leave immediately; a refused account keeps its row, because it kept its binding, and everything else still comes from the refetch. The list moves like a list (arrows between rows), closing a case returns focus to the row it was opened from rather than the top of the page, and the window offers the next account so a backlog can be worked without a trip back through the list each time. Rows already opened this sitting are dimmed — session-scoped, since "have I looked at this" is about the sitting. Group headers stay put while their contents scroll: past thirty rows there was nothing on screen saying which group was being read. History entries lead with how long a binding has stood, which is the question an audit trail is read for — a date makes the reader do that subtraction. The exact instant stays beside it on hover, and it reads the journal's zone-less timestamps as UTC, the same trap the instant formatter already guards. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../components/portal/account-detail.test.tsx | 5 +- .../src/components/portal/account-detail.tsx | 9 +- .../portal/identities-view.test.tsx | 48 ++++++++ .../src/components/portal/identities-view.tsx | 111 +++++++++++++++++- src/frontend/src/lib/format.test.ts | 10 ++ src/frontend/src/lib/format.ts | 25 +++- src/frontend/src/lib/identities/cases.test.ts | 42 ++++++- src/frontend/src/lib/identities/cases.ts | Bin 2837 -> 3925 bytes src/frontend/src/locales/en/translation.json | 4 +- .../src/queries/identity-resolution.ts | 19 ++- 10 files changed, 254 insertions(+), 19 deletions(-) diff --git a/src/frontend/src/components/portal/account-detail.test.tsx b/src/frontend/src/components/portal/account-detail.test.tsx index 29c8889bb..3d8f44f56 100644 --- a/src/frontend/src/components/portal/account-detail.test.tsx +++ b/src/frontend/src/components/portal/account-detail.test.tsx @@ -122,7 +122,10 @@ describe("AccountDetail", () => { expect(screen.getByText("Merged")).toBeInTheDocument(); expect(screen.getByText("seed-backfill")).toBeInTheDocument(); expect(screen.getByText(/by an operator/i)).toBeInTheDocument(); - expect(screen.getByText(/1 Aug 2026/)).toBeInTheDocument(); + // How long a binding has stood is what the trail is read for; the exact + // instant stays beside the age rather than being replaced by it. + expect(screen.getAllByText(/ago$/i).length).toBeGreaterThan(0); + expect(screen.getByTitle(/1 Aug 2026, \d\d:\d\d/)).toBeInTheDocument(); }); // The resolver writes no reason at all — as an empty string, which is not diff --git a/src/frontend/src/components/portal/account-detail.tsx b/src/frontend/src/components/portal/account-detail.tsx index 813e3fe0b..e672c6dac 100644 --- a/src/frontend/src/components/portal/account-detail.tsx +++ b/src/frontend/src/components/portal/account-detail.tsx @@ -27,7 +27,7 @@ import { CenteredSpinner } from "@/components/widgets/centered-spinner"; import { ComingSoon } from "@/components/widgets/coming-soon"; import type { AccountRef } from "@/lib/identities/account-key"; import { personDisplayName } from "@/lib/identities/person-display"; -import { formatUtcInstant } from "@/lib/format"; +import { formatUtcAge, formatUtcInstant } from "@/lib/format"; import { useAccountBinding } from "@/queries/identity-resolution"; /** Known reason codes → i18n keys; anything else renders as-is (open vocabulary). */ @@ -151,8 +151,11 @@ function HistoryRow({ {verbKey ? t(verbKey) : (reason ?? t("identities.history.automatic"))} - - {formatUtcInstant(entry.recorded_at, "d MMM yyyy, HH:mm")} + + {formatUtcAge(entry.recorded_at)}
    diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index 87157be82..493399cb4 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -328,6 +328,54 @@ describe("IdentitiesView", () => { expect(screen.getByRole("dialog")).toBeInTheDocument(); }); + // A backlog is worked in one pass: the list moves like a list, and closing + // a case puts the operator back where they were rather than at the top. + it("moves between rows with the arrow keys", async () => { + attention.q.data = { + items: [ + item({ account_id: "a1", email: "ann@example.com" }), + item({ account_id: "a2", email: "bob@example.com" }), + ], + rates: RATES, + }; + render(); + + const first = screen.getByRole("button", { name: /ann@example\.com/i }); + first.focus(); + await userEvent.keyboard("{ArrowDown}"); + expect(screen.getByRole("button", { name: /bob@example\.com/i })).toHaveFocus(); + + await userEvent.keyboard("{ArrowUp}"); + expect(first).toHaveFocus(); + }); + + it("returns focus to the row when the case window closes", async () => { + attention.q.data = { items: [item({})], rates: RATES }; + render(); + + const row = screen.getByRole("button", { name: /dev42@example\.com/i }); + await userEvent.click(row); + await userEvent.click(screen.getByRole("button", { name: /close/i })); + + expect(row).toHaveFocus(); + }); + + it("offers the next account without a trip back to the list", async () => { + attention.q.data = { + items: [ + item({ account_id: "a1", email: "ann@example.com" }), + item({ account_id: "a2", email: "bob@example.com" }), + ], + rates: RATES, + }; + render(); + + await userEvent.click(screen.getByRole("button", { name: /ann@example\.com/i })); + await userEvent.click(screen.getByRole("button", { name: /next account/i })); + + expect(portalRouter.search.acct).toContain("a2"); + }); + it("offers a retry on a failed load", async () => { attention.q.isError = true; render(); diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 1a7ce734c..3e1db4103 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -13,7 +13,7 @@ * discussion — and that link answers whatever the queue looks like by then, * an emptied backlog included. */ -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import type { AttentionItem, ResolutionRates } from "@/api/identity-client"; @@ -252,6 +252,10 @@ function Queue({ items: everything }: { items: AttentionItem[] }) { const { t } = useTranslation(); const { acct, filter } = usePortalSearch(); const { setAcct } = usePortalNavActions(); + const listRef = useRef(null); + // Session-scoped on purpose: "have I looked at this one" is about the sitting + // an operator is in, not a preference worth outliving it. + const [visited, setVisited] = useState>(new Set()); const items = useMemo( () => filterQueue(everything, filter ?? ""), [everything, filter], @@ -265,11 +269,50 @@ function Queue({ items: everything }: { items: AttentionItem[] }) { const other = items.filter((i) => !known.has(i.kind)); if (other.length > 0) groups.push({ kind: "other", items: other }); + // The rendered order, flattened: what "the next case" means to someone + // working down the queue, and it must not be re-derived differently here. + const ordered = groups.flatMap((group) => + groupIntoCases(group.items).flatMap((c) => c.items.map(itemKey)), + ); + + const select = (key: string | null) => { + if (key) setVisited((seen) => new Set(seen).add(key)); + setAcct(key); + }; + + // Closing the window puts the operator back on the row they opened, not at + // the top of the page — the queue is worked in one pass. + const returnFocus = (key: string) => { + listRef.current + ?.querySelector(`[data-queue-row="${CSS.escape(key)}"]`) + ?.focus(); + }; + + // The queue is a list, so it moves like one. Enter and Space open a row; + // those stay on the row itself. + const onArrow = (event: React.KeyboardEvent) => { + if (event.key !== "ArrowDown" && event.key !== "ArrowUp") return; + const rows = [ + ...(listRef.current?.querySelectorAll("[data-queue-row]") ?? + []), + ]; + const at = rows.indexOf(document.activeElement as HTMLElement); + if (at === -1) return; + const next = rows[at + (event.key === "ArrowDown" ? 1 : -1)]; + if (!next) return; + event.preventDefault(); + next.focus(); + }; + // The worked-to-zero queue is the goal state — but a shared `?acct=` link // has to answer even then, and the backlog reaching zero is exactly when a // colleague opens the link they were sent. return ( -
    +
    {everything.length > 0 ? : null} {/* A filter that matches nothing is not an emptied backlog. Celebrating there would tell an operator the work is done because they mistyped. */} @@ -290,12 +333,23 @@ function Queue({ items: everything }: { items: AttentionItem[] }) { kind={group.kind} items={group.items} selectedKey={acct} - onSelect={(key) => setAcct(key === acct ? null : key)} + visited={visited} + onSelect={(key) => select(key === acct ? null : key)} /> ))} {/* Fed from the unfiltered set: a link stays answerable even when the reader's own filter hides the row it points at. */} - setAcct(null)} /> + { + const opened = acct; + setAcct(null); + if (opened) returnFocus(opened); + }} + />
    ); } @@ -346,10 +400,15 @@ function QueueFilter() { function CaseDialog({ acct, items, + ordered, + onSelect, onClose, }: { acct: string | undefined; items: AttentionItem[]; + /** Account keys in the order the queue renders them. */ + ordered: string[]; + onSelect: (key: string) => void; onClose: () => void; }) { const { t } = useTranslation(); @@ -357,6 +416,9 @@ function CaseDialog({ const queueItem = items.find((i) => itemKey(i) === acct); const heading = queueItem?.email?.trim() || queueItem?.username?.trim() || ref?.account_id; + const at = acct ? ordered.indexOf(acct) : -1; + const previous = at > 0 ? ordered[at - 1] : undefined; + const next = at >= 0 ? ordered[at + 1] : undefined; return ( ) : null} + {/* Working a backlog is a conveyor: the next case is one press away, + without a trip back through the list. */} + {previous || next ? ( +
    + + +
    + ) : null}     ); @@ -405,11 +491,13 @@ function QueueGroup({ kind, items, selectedKey, + visited, onSelect, }: { kind: string; items: AttentionItem[]; selectedKey: string | undefined; + visited: ReadonlySet; onSelect: (key: string) => void; }) { const { t } = useTranslation(); @@ -425,7 +513,7 @@ function QueueGroup({ render={
    ); })} diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index a7186d3e6..23fbc381b 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -440,7 +440,8 @@ "no_matches": "Nothing matches those terms", "no_matches_description": "Clear the filter to see the whole queue.", "previous_case": "‹ Previous account", - "next_case": "Next account ›" + "next_case": "Next account ›", + "reports_to": "reports to {{manager}}" }, "detail": { "not_found": "This account is unknown — the link may be stale.", diff --git a/src/frontend/src/mocks/handlers.ts b/src/frontend/src/mocks/handlers.ts index 0b1f7bea8..a827dea9a 100644 --- a/src/frontend/src/mocks/handlers.ts +++ b/src/frontend/src/mocks/handlers.ts @@ -552,8 +552,25 @@ export const handlers = [ username: "ci-bot-7", candidates: [], }, + { + // Neither address nor handle — nothing automation can match on. The + // source still describes the human, which is what the fold reads for + // the operator and what makes this row bindable by hand. + kind: "no_evidence", + source: "hr", + source_id: "01900000-0000-7000-8000-00000000aa03", + account_id: "921", + email: null, + username: null, + display_name: "Nadia Orlov", + job_title: "Office Manager", + department: "Operations", + status: "Inactive", + manager_email: "carol.chen@example.com", + candidates: [], + }, ], - rates: { observed: 60, bound: 55, pending: 3, no_evidence: 1, excluded: 1 }, + rates: { observed: 60, bound: 55, pending: 3, no_evidence: 2, excluded: 1 }, truncated: false, items_truncated: false, }); diff --git a/tests/stand/api/schemas/identity.py b/tests/stand/api/schemas/identity.py index cc3e361fa..30ee5fac5 100644 --- a/tests/stand/api/schemas/identity.py +++ b/tests/stand/api/schemas/identity.py @@ -348,10 +348,15 @@ class QueueItemResponse(BaseModel): ) account_id: str candidates: list[PersonSummaryResponse] = Field(..., description='Persons this account could belong to, if any are known — hydrated into\ncards so the operator UI never has to resolve bare ids itself.') + department: str | None = None + display_name: str | None = Field(None, description='How the source describes the account. Nothing here is matchable — it is\nwhat lets an operator recognise whose account this is when automation\ncannot, which is exactly the case for the ones only they can bind.') email: str | None = None + job_title: str | None = None kind: str = Field(..., description='`contested` | `binding_conflict` | `no_evidence`.') + manager_email: str | None = None source: str source_id: UUID + status: str | None = None username: str | None = None From 8348d3aa20431496902017d48c72532d1b964ae1 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 06:00:54 +0300 Subject: [PATCH 08/19] =?UTF-8?q?frontend:=20a=20second=20way=20into=20the?= =?UTF-8?q?=20console=20=E2=80=94=20a=20person=20and=20their=20accounts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The console could only be entered through the queue, which shows what automation could not decide. A binding that is settled and consistent is therefore invisible — and the questions that actually arrive are about settled bindings: this person's work looks split, that account is not theirs. The verbs already accept those accounts; there was no door to them. Not a list of every binding: thousands of rows nobody reads by eye, needing paging and filtering on top of a read that already folds every observed account — and a list invites binding things because they are on screen. Entered through a person, a correction stays attached to the question that prompted it. Built from reads that already exist: the person search behind "Assign to someone else", the accounts read that feeds the merge preview (it reports who made each binding, which is the first thing to know before changing one), and the case window, which already worked for an account outside the queue. Modes are tabs with the mode in the URL, so a link opens the one it was sent from, and adding the next one is an entry in a list. Switching drops the open account: a case picked in one mode means nothing in a list it is not part of. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../src/components/portal/case-dialog.tsx | 116 +++++++++++ .../portal/identities-view.test.tsx | 33 ++++ .../src/components/portal/identities-view.tsx | 171 +++++----------- .../portal/person-accounts-view.test.tsx | 148 ++++++++++++++ .../portal/person-accounts-view.tsx | 184 ++++++++++++++++++ src/frontend/src/lib/portal/portal-search.ts | 8 + src/frontend/src/locales/en/translation.json | 14 ++ 7 files changed, 554 insertions(+), 120 deletions(-) create mode 100644 src/frontend/src/components/portal/case-dialog.tsx create mode 100644 src/frontend/src/components/portal/person-accounts-view.test.tsx create mode 100644 src/frontend/src/components/portal/person-accounts-view.tsx diff --git a/src/frontend/src/components/portal/case-dialog.tsx b/src/frontend/src/components/portal/case-dialog.tsx new file mode 100644 index 000000000..84d5debcc --- /dev/null +++ b/src/frontend/src/components/portal/case-dialog.tsx @@ -0,0 +1,116 @@ +import { useTranslation } from "react-i18next"; + +import type { AttentionItem } from "@/api/identity-client"; +import { CopyValueButton } from "@/components/copy-value-button"; +import { AccountDetail } from "@/components/portal/account-detail"; +import { Button } from "@/components/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { itemKey, parseAccountKey } from "@/lib/identities/account-key"; + +/** + * One account under review, in a window rather than a column: this is where + * every decision is taken, and a decision that re-attributes a person's work + * deserves the room to show what it acts on. + * + * Opened by the `?acct=` in the URL — never by click state alone — so a link + * an operator shares lands their colleague on the same case. + */ +export function CaseDialog({ + acct, + items, + ordered, + onSelect, + onClose, +}: { + acct: string | undefined; + items: AttentionItem[]; + /** Account keys in the order the queue renders them. */ + ordered: string[]; + onSelect: (key: string) => void; + onClose: () => void; +}) { + const { t } = useTranslation(); + const ref = parseAccountKey(acct); + const queueItem = items.find((i) => itemKey(i) === acct); + const heading = + queueItem?.email?.trim() || queueItem?.username?.trim() || ref?.account_id; + const at = acct ? ordered.indexOf(acct) : -1; + const previous = at > 0 ? ordered[at - 1] : undefined; + const next = at >= 0 ? ordered[at + 1] : undefined; + + return ( + { + if (!open) onClose(); + }} + > + {/* A fixed height, not a fitted one: an operator walks from case to case + and a window that resizes to each one moves the verbs under their + cursor. The history takes the slack instead. */} + + + {heading} + } + > + + {ref?.source} · {ref?.account_id} + + {ref ? ( + + ) : null} + + + {/* Keyed by the account: the body holds per-account state (a verb's + outcome, an open confirmation), and a cached binding renders the + next case synchronously — unkeyed, that state would follow. */} + {ref ? ( + + ) : null} + {/* Working a backlog is a conveyor: the next case is one press away, + without a trip back through the list. */} + {previous || next ? ( +
    + + +
    + ) : null} +     +     + ); +} diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index b4b6778da..1ef104d45 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -30,6 +30,14 @@ const attention = vi.hoisted(() => ({ })); vi.mock("@/queries/identity-resolution", () => ({ useAttention: () => attention.q, + // The people mode has its own test file; here it only has to mount. + usePersonSearch: () => ({ data: undefined, isFetching: false, isError: false }), + usePersonAccounts: () => ({ + data: undefined, + isLoading: false, + isError: false, + refetch: vi.fn(), + }), // The panel under a selection; its own behaviour is account-detail.test's. useAccountBinding: () => ({ data: undefined, @@ -406,6 +414,31 @@ describe("IdentitiesView", () => { expect(portalRouter.search.acct).toContain("a2"); }); + // Modes are ways IN to the same decisions; the mode rides in the URL so a + // link opens the one it was sent from. + it("switches modes through the URL, dropping the account selected in the old one", async () => { + attention.q.data = { items: [item({})], rates: RATES }; + // A selection the window cannot open (a mistyped link) leaves the value in + // the URL with no modal over the tabs — the state the switch must clear. + portalRouter.set({ zone: "manage", item: "identities", acct: "malformed" }); + render(); + + await userEvent.click(screen.getByRole("tab", { name: /a person and their accounts/i })); + + expect(portalRouter.search.mode).toBe("people"); + // A case picked in the queue means nothing in a list it is not part of. + expect(portalRouter.search.acct).toBeUndefined(); + expect(screen.getByText(/nobody chosen yet/i)).toBeInTheDocument(); + }); + + it("falls back to the queue when the URL names a mode that does not exist", () => { + attention.q.data = { items: [item({})], rates: RATES }; + portalRouter.set({ zone: "manage", item: "identities", mode: "not-a-mode" }); + render(); + + expect(screen.getByRole("button", { name: /dev42@example\.com/i })).toBeInTheDocument(); + }); + it("offers a retry on a failed load", async () => { attention.q.isError = true; render(); diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 63d4bdc1c..85620d883 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -18,15 +18,9 @@ import { useTranslation } from "react-i18next"; import type { AttentionItem, ResolutionRates } from "@/api/identity-client"; import { Alert, AlertDescription } from "@/components/ui/alert"; -import { CopyValueButton } from "@/components/copy-value-button"; -import { - Dialog, - DialogContent, - DialogDescription, - DialogHeader, - DialogTitle, -} from "@/components/ui/dialog"; import { CenteredSpinner } from "@/components/widgets/centered-spinner"; +import { CaseDialog } from "@/components/portal/case-dialog"; +import { PersonAccountsView } from "@/components/portal/person-accounts-view"; import { PersonCell } from "@/components/portal/person-cell"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; @@ -44,6 +38,7 @@ import { EmptyMedia, EmptyTitle, } from "@/components/ui/empty"; +import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { Tooltip, TooltipContent, @@ -57,13 +52,12 @@ import { useSetPortalSearch, } from "@/lib/portal/portal-search"; import { usePortalNavActions } from "@/lib/portal/portal-nav"; -import { itemKey, parseAccountKey } from "@/lib/identities/account-key"; +import { itemKey } from "@/lib/identities/account-key"; import { filterQueue, groupIntoCases, type QueueCase, } from "@/lib/identities/cases"; -import { AccountDetail } from "@/components/portal/account-detail"; import { useAttention } from "@/queries/identity-resolution"; import { TEXT_FIGURE, TEXT_LABEL } from "@/lib/type-scale"; import { STATUS_SURFACE_CLASS, type Status } from "@/lib/status"; @@ -100,7 +94,53 @@ function opensTheCase(event: React.MouseEvent): boolean { return !selection || selection.isCollapsed; } +/** + * The modes the console offers. A mode is a way IN to the same decisions — + * the queue arrives at them from a problem, the person view from a name — so + * adding one is an entry here and a component, nothing else. + */ +const MODES = ["queue", "people"] as const; +const DEFAULT_MODE = MODES[0]; + export function IdentitiesView() { + const { t } = useTranslation(); + const { mode } = usePortalSearch(); + const setSearch = useSetPortalSearch(); + const active: string = MODES.find((m) => m === mode) ?? DEFAULT_MODE; + + return ( +
    +
    +

    + {t("identities.title")} +

    +

    + {t("identities.subtitle")} +

    +
    + + setSearch({ mode: String(next), acct: undefined }) + } + > + + {MODES.map((m) => ( + + {t(`identities.modes.${m}`)} + + ))} + + + {active === "people" ? : } +
    + ); +} + +function ReviewQueue() { const { t } = useTranslation(); const attention = useAttention(); @@ -121,15 +161,7 @@ export function IdentitiesView() { const { items, rates, truncated, items_truncated: itemsTruncated } = attention.data; return ( -
    -
    -

    - {t("identities.title")} -

    -

    - {t("identities.subtitle")} -

    -
    +
    {truncated ? ( @@ -389,107 +421,6 @@ function QueueFilter() { ); } -/** - * One account under review, in a window rather than a column: this is where - * every decision is taken, and a decision that re-attributes a person's work - * deserves the room to show what it acts on. - * - * Opened by the `?acct=` in the URL — never by click state alone — so a link - * an operator shares lands their colleague on the same case. - */ -function CaseDialog({ - acct, - items, - ordered, - onSelect, - onClose, -}: { - acct: string | undefined; - items: AttentionItem[]; - /** Account keys in the order the queue renders them. */ - ordered: string[]; - onSelect: (key: string) => void; - onClose: () => void; -}) { - const { t } = useTranslation(); - const ref = parseAccountKey(acct); - const queueItem = items.find((i) => itemKey(i) === acct); - const heading = - queueItem?.email?.trim() || queueItem?.username?.trim() || ref?.account_id; - const at = acct ? ordered.indexOf(acct) : -1; - const previous = at > 0 ? ordered[at - 1] : undefined; - const next = at >= 0 ? ordered[at + 1] : undefined; - - return ( - { - if (!open) onClose(); - }} - > - {/* A fixed height, not a fitted one: an operator walks from case to case - and a window that resizes to each one moves the verbs under their - cursor. The history takes the slack instead. */} - - - {heading} - } - > - - {ref?.source} · {ref?.account_id} - - {ref ? ( - - ) : null} - - - {/* Keyed by the account: the body holds per-account state (a verb's - outcome, an open confirmation), and a cached binding renders the - next case synchronously — unkeyed, that state would follow. */} - {ref ? ( - - ) : null} - {/* Working a backlog is a conveyor: the next case is one press away, - without a trip back through the list. */} - {previous || next ? ( -
    - - -
    - ) : null} -     -     - ); -} /** Cases rendered before the group asks to be expanded further. */ const CASE_PAGE = 10; diff --git a/src/frontend/src/components/portal/person-accounts-view.test.tsx b/src/frontend/src/components/portal/person-accounts-view.test.tsx new file mode 100644 index 000000000..7dca97515 --- /dev/null +++ b/src/frontend/src/components/portal/person-accounts-view.test.tsx @@ -0,0 +1,148 @@ +// @vitest-environment jsdom +/** + * The console's second mode. What matters: an operator arrives holding a name + * and reaches accounts the queue never shows, because a settled binding is not + * a problem — until someone asks about it; who decided each binding is on the + * row, since undoing automation is routine and overruling a colleague is not; + * and the person rides in the URL, so the view is shareable like the rest. + */ +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import "@/i18n"; +import type { PersonAccountEntry } from "@/api/identity-client"; + +vi.mock("@tanstack/react-router", async () => { + const { portalRouterMock } = await import("@/test/portal-router"); + return portalRouterMock(); +}); + +const hooks = vi.hoisted(() => ({ + accounts: { + data: undefined as + | { person_id: string; accounts: PersonAccountEntry[] } + | undefined, + isLoading: false, + isError: false, + refetch: vi.fn(), + }, + search: { + data: undefined as { items: unknown[]; truncated?: boolean } | undefined, + isFetching: false, + isError: false, + }, +})); +vi.mock("@/queries/identity-resolution", () => ({ + usePersonAccounts: () => hooks.accounts, + usePersonSearch: () => hooks.search, + // The window's own behaviour belongs to account-detail.test. + useAccountBinding: () => ({ + data: undefined, + isLoading: true, + isError: false, + error: null, + refetch: vi.fn(), + }), +})); + +import { portalRouter } from "@/test/portal-router"; + +import { PersonAccountsView } from "./person-accounts-view"; + +const ANN = "01900000-0000-7000-8000-0000000000a0"; + +function entry(over: Partial = {}): PersonAccountEntry { + return { + source: "github", + source_id: "01900000-0000-7000-8000-00000000aa01", + account_id: "gh-main", + email: "ann@example.com", + username: null, + bound_by_operator: false, + ...over, + }; +} + +beforeEach(() => { + hooks.accounts.data = undefined; + hooks.accounts.isLoading = false; + hooks.accounts.isError = false; + hooks.accounts.refetch.mockClear(); + hooks.search.data = undefined; + portalRouter.reset(); + portalRouter.set({ zone: "manage", item: "identities", mode: "people" }); +}); + +describe("PersonAccountsView", () => { + it("asks for a person before showing anything", () => { + render(); + + expect(screen.getByText(/nobody chosen yet/i)).toBeInTheDocument(); + }); + + it("puts the chosen person in the URL, so the view is a link", async () => { + hooks.search.data = { + items: [{ person_id: ANN, display_name: "Ann Lee", email: "ann@example.com" }], + }; + render(); + + await userEvent.type(screen.getByRole("searchbox"), "ann"); + await userEvent.click(screen.getByRole("button", { name: /ann lee/i })); + + expect(portalRouter.search.person).toBe(ANN); + }); + + it("lists every account bound to the person, saying who decided each", () => { + portalRouter.set({ person: ANN }); + hooks.accounts.data = { + person_id: ANN, + accounts: [ + entry(), + entry({ + source: "gitlab", + account_id: "gl-alt", + email: null, + username: "alee", + bound_by_operator: true, + }), + ], + }; + render(); + + expect(screen.getByText("ann@example.com")).toBeInTheDocument(); + expect(screen.getByText(/github · gh-main/)).toBeInTheDocument(); + expect(screen.getByText(/bound automatically/i)).toBeInTheDocument(); + expect(screen.getByText(/decided by an operator/i)).toBeInTheDocument(); + }); + + // The whole point of the mode: these accounts are settled, so the queue + // never shows them — and the verbs are still available for them. + it("opens a settled account in the same case window", async () => { + portalRouter.set({ person: ANN }); + hooks.accounts.data = { person_id: ANN, accounts: [entry()] }; + render(); + + await userEvent.click(screen.getByRole("button", { name: /^open$/i })); + + expect(portalRouter.search.acct).toContain("gh-main"); + expect(within(screen.getByRole("dialog")).getByText(/github · gh-main/)).toBeInTheDocument(); + }); + + it("states an empty result rather than an empty card", () => { + portalRouter.set({ person: ANN }); + hooks.accounts.data = { person_id: ANN, accounts: [] }; + render(); + + expect(screen.getByText(/no account is bound to this person/i)).toBeInTheDocument(); + }); + + it("offers a retry when the read fails", async () => { + portalRouter.set({ person: ANN }); + hooks.accounts.isError = true; + render(); + + await userEvent.click(screen.getByRole("button", { name: /retry/i })); + expect(hooks.accounts.refetch).toHaveBeenCalled(); + }); +}); diff --git a/src/frontend/src/components/portal/person-accounts-view.tsx b/src/frontend/src/components/portal/person-accounts-view.tsx new file mode 100644 index 000000000..6cc7dcc24 --- /dev/null +++ b/src/frontend/src/components/portal/person-accounts-view.tsx @@ -0,0 +1,184 @@ +/** + * The console's second mode: one person and every account bound to them. + * + * The review queue only ever shows what automation could not decide, so a + * settled binding is invisible — and the questions that actually arrive are + * about settled bindings ("this person's work looks split", "that account is + * not theirs"). The verbs already accept those accounts; this is the door to + * them. + * + * Deliberately entered through a person rather than a list of every binding: + * an operator arrives holding a name, and a decision reached that way stays + * attached to the question that prompted it. + */ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { UserSearch } from "lucide-react"; + +import type { PersonAccountEntry, PersonSummary } from "@/api/identity-client"; +import { CaseDialog } from "@/components/portal/case-dialog"; +import { PersonCell } from "@/components/portal/person-cell"; +import { PersonPicker } from "@/components/portal/person-picker"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { + Empty, + EmptyDescription, + EmptyHeader, + EmptyMedia, + EmptyTitle, +} from "@/components/ui/empty"; +import { CenteredSpinner } from "@/components/widgets/centered-spinner"; +import { ComingSoon } from "@/components/widgets/coming-soon"; +import { accountKey } from "@/lib/identities/account-key"; +import { usePortalNavActions } from "@/lib/portal/portal-nav"; +import { usePortalSearch, useSetPortalSearch } from "@/lib/portal/portal-search"; +import { usePersonAccounts } from "@/queries/identity-resolution"; +import { cn } from "@/lib/utils"; + +export function PersonAccountsView() { + const { t } = useTranslation(); + const { person } = usePortalSearch(); + const setSearch = useSetPortalSearch(); + // The URL owns which person is open; this remembers the card the operator + // picked, so the heading names them. Arriving by link there is no card — + // search resolves values, not ids — and the id stands alone, honestly. + const [picked, setPicked] = useState(null); + + return ( +
    + { + setPicked(next); + setSearch({ person: next.person_id, acct: undefined }); + }} + /> + {person ? ( + + ) : ( + + + + + + {t("identities.people.no_person")} + + {t("identities.people.no_person_description")} + + + + )} +
    + ); +} + +function PersonAccounts({ + personId, + card, +}: { + personId: string; + card: PersonSummary | null; +}) { + const { t } = useTranslation(); + const { acct } = usePortalSearch(); + const { setAcct } = usePortalNavActions(); + const accounts = usePersonAccounts(personId); + + if (accounts.isLoading) return ; + if (accounts.isError || !accounts.data) { + return ( + void accounts.refetch()} + /> + ); + } + + const entries = accounts.data.accounts; + const ordered = entries.map((entry) => accountKey(entry)); + + return ( + + + + {t("identities.people.accounts")} + {entries.length} + + + + + {entries.length === 0 ? ( +

    + {t("identities.people.no_accounts")} +

    + ) : ( + entries.map((entry) => ( + setAcct(accountKey(entry))} + /> + )) + )} +     + setAcct(null)} + /> +     + ); +} + +function AccountRow({ + entry, + selected, + onOpen, +}: { + entry: PersonAccountEntry; + selected: boolean; + onOpen: () => void; +}) { + const { t } = useTranslation(); + const label = entry.email?.trim() || entry.username?.trim() || entry.account_id; + return ( +
    +
    +
    {label}
    +
    + {entry.source} · {entry.account_id} +
    +
    + {/* Who decided this binding is the first thing to know before changing + it: undoing automation is routine, overruling a colleague is not. */} + + {entry.bound_by_operator + ? t("identities.people.by_operator") + : t("identities.people.by_automation")} + + +
    + ); +} diff --git a/src/frontend/src/lib/portal/portal-search.ts b/src/frontend/src/lib/portal/portal-search.ts index 1ac83a607..bb910520b 100644 --- a/src/frontend/src/lib/portal/portal-search.ts +++ b/src/frontend/src/lib/portal/portal-search.ts @@ -28,6 +28,10 @@ export interface PortalSearch { acct?: string; /** Free-text narrowing of the Identities queue. */ filter?: string; + /** Which Identities mode is open (the review queue, a person). */ + mode?: string; + /** Person under inspection in the Identities people mode. */ + person?: string; /** Expanded direction + its active lens, within the Directions zone. */ dir?: string; lens?: string; @@ -76,6 +80,8 @@ export function validatePortalSearch(raw: Record): PortalSearch item: str(raw.item), acct: str(raw.acct), filter: str(raw.filter), + mode: str(raw.mode), + person: str(raw.person)?.toLowerCase(), dir: str(raw.dir), lens: str(raw.lens), // Lower-cased to match `normalizePersonId`: the same id reaches us from a @@ -100,6 +106,8 @@ export const PORTAL_SEARCH_KEYS = [ "item", "acct", "filter", + "mode", + "person", "dir", "lens", "scope", diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index 23fbc381b..ecc21229d 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -517,6 +517,20 @@ "title": "Identities is an admin surface", "description": "Managing identity resolution needs the administrator role. Ask an existing administrator to grant it if this is part of your job.", "unverified": "Could not verify the administrator role" + }, + "modes": { + "queue": "Review queue", + "people": "A person and their accounts" + }, + "people": { + "no_person": "Nobody chosen yet", + "no_person_description": "Find a person to see every account bound to them — and correct any of it.", + "accounts": "Accounts bound to this person", + "no_accounts": "No account is bound to this person.", + "by_operator": "decided by an operator", + "by_automation": "bound automatically", + "open": "Open", + "load_failed": "Unable to load this person’s accounts" } } } From 0a94913da37fa62cc9545cfdf068823628ad10d9 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 06:07:54 +0300 Subject: [PATCH 09/19] identity: say which candidate holds the account being argued over MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A binding conflict lists the same candidates on every row of the case — that is what makes it one case — so the row asked an operator to choose between two people without saying which of them they would be taking the account FROM. The fact was reachable only by opening the account, one at a time. The queue is built from the bindings joined with the evidence, so the answer was already in hand and simply not carried: the item now reports who holds it, and the row names them. Absent means nobody, which is exactly what a contested account is — unbound, which is why two people can claim it. Additive on the wire, like the descriptive fields before it. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../backend/identity-resolution/openapi.json | 8 +++ .../identity-resolution/src/api/resolution.rs | 5 ++ .../src/domain/review_queue.rs | 58 +++++++++++++++++-- src/frontend/src/api/identity-client.ts | 3 + .../portal/identities-view.test.tsx | 28 ++++++++- .../src/components/portal/identities-view.tsx | 15 +++++ src/frontend/src/locales/en/translation.json | 3 +- src/frontend/src/mocks/handlers.ts | 4 ++ tests/stand/api/schemas/identity.py | 1 + 9 files changed, 117 insertions(+), 8 deletions(-) diff --git a/docs/components/backend/identity-resolution/openapi.json b/docs/components/backend/identity-resolution/openapi.json index de1a252a0..91d4fe602 100644 --- a/docs/components/backend/identity-resolution/openapi.json +++ b/docs/components/backend/identity-resolution/openapi.json @@ -1001,6 +1001,14 @@ "account_id": { "type": "string" }, + "bound_to": { + "description": "Who holds the account right now. Absent = nobody, which is what an\nunbound account on the queue means; present, it names which of the\ncandidates below is the one being disagreed with.", + "format": "uuid", + "type": [ + "string", + "null" + ] + }, "candidates": { "description": "Persons this account could belong to, if any are known — hydrated into\ncards so the operator UI never has to resolve bare ids itself.", "items": { diff --git a/src/backend/services/identity-resolution/src/api/resolution.rs b/src/backend/services/identity-resolution/src/api/resolution.rs index b7493b242..517713cf9 100644 --- a/src/backend/services/identity-resolution/src/api/resolution.rs +++ b/src/backend/services/identity-resolution/src/api/resolution.rs @@ -642,6 +642,10 @@ pub struct QueueItemResponse { pub department: Option, pub status: Option, pub manager_email: Option, + /// Who holds the account right now. Absent = nobody, which is what an + /// unbound account on the queue means; present, it names which of the + /// candidates below is the one being disagreed with. + pub bound_to: Option, /// Persons this account could belong to, if any are known — hydrated into /// cards so the operator UI never has to resolve bare ids itself. pub candidates: Vec, @@ -734,6 +738,7 @@ pub async fn attention( department: i.description.department, status: i.description.status, manager_email: i.description.manager_email, + bound_to: i.bound_to, candidates: person_card::in_requested_order(&i.candidates, &cards) .into_iter() .map(PersonSummaryResponse::from) diff --git a/src/backend/services/identity-resolution/src/domain/review_queue.rs b/src/backend/services/identity-resolution/src/domain/review_queue.rs index 2bd9fe662..934c05155 100644 --- a/src/backend/services/identity-resolution/src/domain/review_queue.rs +++ b/src/backend/services/identity-resolution/src/domain/review_queue.rs @@ -55,6 +55,9 @@ pub struct QueueItem { pub email: Option, pub username: Option, pub description: AccountDescription, + /// The person holding this account right now, when one does. Absent means + /// unbound — which is itself the answer for a contested account. + pub bound_to: Option, pub candidates: Vec, } @@ -117,13 +120,13 @@ pub fn build( Some(_) => rates.bound += 1, None if !has_matchable_evidence(account) => { rates.no_evidence += 1; - items.push(item(ItemKind::NoEvidence, account, Vec::new())); + items.push(item(ItemKind::NoEvidence, account, None, Vec::new())); } None => { let candidates = candidates_for(account, &by_email, bindings); if candidates.len() > 1 { rates.pending += 1; - items.push(item(ItemKind::Contested, account, candidates)); + items.push(item(ItemKind::Contested, account, None, candidates)); } else { // One candidate or none: the account has an e-mail, so // automation will bind it on its next run — not the @@ -179,7 +182,13 @@ fn binding_conflicts( } for account in group { - conflicts.push(item(ItemKind::BindingConflict, account, persons.clone())); + let bound_to = bindings.get(&account.account).map(|b| b.person_id); + conflicts.push(item( + ItemKind::BindingConflict, + account, + bound_to, + persons.clone(), + )); } } @@ -217,13 +226,19 @@ fn has_matchable_evidence(account: &EvidenceAccount) -> bool { account.email.is_some() } -fn item(kind: ItemKind, account: &EvidenceAccount, candidates: Vec) -> QueueItem { +fn item( + kind: ItemKind, + account: &EvidenceAccount, + bound_to: Option, + candidates: Vec, +) -> QueueItem { QueueItem { kind, account: account.account.clone(), email: account.email.clone(), username: account.username.clone(), description: account.description.clone(), + bound_to, candidates, } } @@ -367,6 +382,41 @@ mod tests { ); } + #[test] + fn a_conflict_row_names_the_person_holding_that_account() { + // The candidates are the same for every row of the case; which of them + // holds THIS account is the fact each decision turns on. + let mut bindings = HashMap::new(); + bindings.insert(account("hr", "1"), seed_bound(1)); + bindings.insert(account("chat", "2"), seed_bound(2)); + + let review = build( + vec![ + observed("hr", "1", Some("a@example.com")), + observed("chat", "2", Some("a@example.com")), + ], + &bindings, + ); + + let hr: Vec<&QueueItem> = review + .items + .iter() + .filter(|i| i.account.source_type == "hr") + .collect(); + + assert_eq!(hr.len(), 1, "one row for the hr account"); + assert_eq!(hr[0].kind, ItemKind::BindingConflict); + assert_eq!(hr[0].bound_to, Some(Uuid::from_u128(1))); + assert_eq!(hr[0].candidates.len(), 2, "both sides stay listed"); + } + + #[test] + fn an_unbound_queue_item_is_bound_to_nobody() { + let review = build(vec![observed("jira", "jr-1", None)], &HashMap::new()); + + assert_eq!(review.items[0].bound_to, None); + } + #[test] fn all_seed_divergence_surfaces_as_a_binding_conflict() { let a = observed("github", "gh-1", Some("legacy@example.com")); diff --git a/src/frontend/src/api/identity-client.ts b/src/frontend/src/api/identity-client.ts index cd1cced6d..460c42d85 100644 --- a/src/frontend/src/api/identity-client.ts +++ b/src/frontend/src/api/identity-client.ts @@ -123,6 +123,9 @@ export interface AttentionItem { department?: string | null; status?: string | null; manager_email?: string | null; + /** Who holds the account right now; absent = nobody. Names which of the + * candidates is the one being disagreed with. */ + bound_to?: string | null; /** Hydrated person cards, not bare ids. */ candidates: PersonSummary[]; } diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index 1ef104d45..e302497ef 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -184,9 +184,27 @@ describe("IdentitiesView", () => { ]; attention.q.data = { items: [ - item({ kind: "binding_conflict", account_id: "a1", source: "hr", candidates }), - item({ kind: "binding_conflict", account_id: "a2", source: "wiki", candidates }), - item({ kind: "binding_conflict", account_id: "a3", source: "chat", candidates }), + item({ + kind: "binding_conflict", + account_id: "a1", + source: "hr", + candidates, + bound_to: candidates[0]?.person_id, + }), + item({ + kind: "binding_conflict", + account_id: "a2", + source: "wiki", + candidates, + bound_to: candidates[1]?.person_id, + }), + item({ + kind: "binding_conflict", + account_id: "a3", + source: "chat", + candidates, + bound_to: candidates[1]?.person_id, + }), ], rates: RATES, }; @@ -194,6 +212,10 @@ describe("IdentitiesView", () => { expect(screen.getAllByText("Ann Lee")).toHaveLength(1); expect(screen.getByText(/1 case · 3 accounts/i)).toBeInTheDocument(); + // The candidates are stated once for the case, so each row has to say + // which of them it would be taking the account from. + expect(screen.getByText(/now Ann Lee/i)).toBeInTheDocument(); + expect(screen.getAllByText(/now Bob Park/i)).toHaveLength(2); // Each account still has its own row: a decision is taken per account. expect(screen.getAllByRole("button", { name: /dev42@example\.com/i })).toHaveLength(3); }); diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 85620d883..4d72e0e60 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -53,6 +53,7 @@ import { } from "@/lib/portal/portal-search"; import { usePortalNavActions } from "@/lib/portal/portal-nav"; import { itemKey } from "@/lib/identities/account-key"; +import { personDisplayName } from "@/lib/identities/person-display"; import { filterQueue, groupIntoCases, @@ -592,6 +593,13 @@ function CaseBlock({ .map((s) => s?.trim()) .filter(Boolean) .join(" · "); + // Which of the case's candidates holds THIS account: the candidates + // are stated once for the whole case, so without this the row asks + // an operator to decide between two people without saying which one + // they would be taking it from. + const boundTo = queueCase.candidates.find( + (c) => c.person_id === item.bound_to, + ); return (
    {label} + {boundTo ? ( + + {t("identities.queue.bound_to", { + name: personDisplayName(boundTo), + })} + + ) : null} {label === item.account_id ? item.source : `${item.source} · ${item.account_id}`} diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index ecc21229d..cd61b0aa1 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -441,7 +441,8 @@ "no_matches_description": "Clear the filter to see the whole queue.", "previous_case": "‹ Previous account", "next_case": "Next account ›", - "reports_to": "reports to {{manager}}" + "reports_to": "reports to {{manager}}", + "bound_to": "now {{name}}" }, "detail": { "not_found": "This account is unknown — the link may be stale.", diff --git a/src/frontend/src/mocks/handlers.ts b/src/frontend/src/mocks/handlers.ts index a827dea9a..b83677b60 100644 --- a/src/frontend/src/mocks/handlers.ts +++ b/src/frontend/src/mocks/handlers.ts @@ -532,6 +532,9 @@ export const handlers = [ account_id: "dev-42", email: "dev42@example.com", username: "dev42", + // Contested means unbound: nobody holds it, which is why two people + // can claim it. + bound_to: null, candidates: [card(bob), card(carol)], }, { @@ -541,6 +544,7 @@ export const handlers = [ account_id: "a.kim", email: alice?.email ?? "alice.kim@example.com", username: null, + bound_to: alice?.person_id, candidates: [card(alice)], }, { diff --git a/tests/stand/api/schemas/identity.py b/tests/stand/api/schemas/identity.py index 30ee5fac5..5a685e5e8 100644 --- a/tests/stand/api/schemas/identity.py +++ b/tests/stand/api/schemas/identity.py @@ -347,6 +347,7 @@ class QueueItemResponse(BaseModel): extra='forbid', ) account_id: str + bound_to: UUID | None = Field(None, description='Who holds the account right now. Absent = nobody, which is what an\nunbound account on the queue means; present, it names which of the\ncandidates below is the one being disagreed with.') candidates: list[PersonSummaryResponse] = Field(..., description='Persons this account could belong to, if any are known — hydrated into\ncards so the operator UI never has to resolve bare ids itself.') department: str | None = None display_name: str | None = Field(None, description='How the source describes the account. Nothing here is matchable — it is\nwhat lets an operator recognise whose account this is when automation\ncannot, which is exactly the case for the ones only they can bind.') From 458e7d91082040dca5e4ec5b8f9661c3c4542f0d Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 06:10:47 +0300 Subject: [PATCH 10/19] identity: let the person search take an id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The console now prints a person id on every card and offers it for copying, and pasting one back into the search found nothing: the search matches observed values, and an id is a column, not a value anyone observed. Worse for the case it matters most in — a person minted during a first sign-in carries no values at all until the resolver attaches the roster's name, so no value search can reach them, and the id is the only handle they have. A term that parses as a UUID now names a person; terms beside it still match values, so an id and a name together narrow rather than contradict. The excluded-person sentinel is refused by name as it is by every verb: it accumulates a journal row per exclusion and is still not a person. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../identity-resolution/src/api/persons.rs | 89 ++++++++++++++++++- src/frontend/src/locales/en/translation.json | 2 +- src/frontend/src/mocks/handlers.ts | 6 +- 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/src/backend/services/identity-resolution/src/api/persons.rs b/src/backend/services/identity-resolution/src/api/persons.rs index 1e6b148d9..9bf704111 100644 --- a/src/backend/services/identity-resolution/src/api/persons.rs +++ b/src/backend/services/identity-resolution/src/api/persons.rs @@ -8,6 +8,10 @@ //! both — the operator is the disambiguator, and hiding one of them by //! recency would decide a contested case silently. //! +//! A term that parses as a UUID names a person id instead: it is the one +//! identifier an operator can copy off a card, and the only way to reach a +//! person the journal holds no values for. +//! //! Admin-gated and deliberately NOT visibility-filtered: this is the operator //! surface, and the seeded operator sits outside the org chart on purpose. @@ -20,12 +24,14 @@ use serde::{Deserialize, Serialize}; use toolkit_canonical_errors::CanonicalError; use toolkit_security::SecurityContext; use utoipa::ToSchema; +use uuid::Uuid; use super::AppState; use super::error::PersonSearchError; use super::gate::require_admin; use super::resolution::PersonSummaryResponse; use crate::domain::person_card; +use crate::domain::resolution::EXCLUDED_PERSON; use crate::infra::db::persons_repo; const DEFAULT_LIMIT: u64 = 20; @@ -68,11 +74,16 @@ pub async fn search_persons( let terms = search_terms(params.q.as_deref())?; let limit = super::listing::clamp_limit(params.limit, DEFAULT_LIMIT, MAX_LIMIT); + let (named, values) = partition_terms(&terms); + // Over-fetch by one: the extra row is the truncation probe, never served. - let mut ids = - persons_repo::search_persons_by_current_values(&state.db, tenant, &terms, limit + 1) + let mut ids = if named.is_empty() { + persons_repo::search_persons_by_current_values(&state.db, tenant, &values, limit + 1) .await - .map_err(|e| read_err(&e))?; + .map_err(|e| read_err(&e))? + } else { + persons_named_by_id(&state, tenant, &named, &values, limit + 1).await? + }; let truncated = ids.len() > usize::try_from(limit).unwrap_or(usize::MAX); if truncated { ids.pop(); @@ -94,6 +105,53 @@ pub async fn search_persons( })) } +/// A term that parses as a UUID names a person id; everything else is matched +/// against observed values. +/// +/// Without this the one identifier an operator can copy off a card finds +/// nothing, and a person the journal holds no attributes for — minted at first +/// sign-in, before the resolver attaches the roster's name — cannot be found at +/// all, since a value search has no value to match. +fn partition_terms(terms: &[String]) -> (Vec, Vec) { + let mut named = Vec::new(); + let mut values = Vec::new(); + for term in terms { + match Uuid::parse_str(term) { + // The excluded-person sentinel is not a person; naming it finds + // nobody rather than serving the row every exclusion appends to. + Ok(id) if id != EXCLUDED_PERSON => named.push(id), + Ok(_) => {} + Err(_) => values.push(term.clone()), + } + } + (named, values) +} + +/// Persons named by id, narrowed by any value terms alongside them. +async fn persons_named_by_id( + state: &AppState, + tenant: Uuid, + named: &[Uuid], + values: &[String], + limit: u64, +) -> Result, CanonicalError> { + let known = persons_repo::persons_in_tenant(&state.db, tenant, named) + .await + .map_err(|e| read_err(&e))?; + if values.is_empty() { + return Ok(known); + } + + let by_value = persons_repo::search_persons_by_current_values(&state.db, tenant, values, limit) + .await + .map_err(|e| read_err(&e))?; + + Ok(known + .into_iter() + .filter(|id| by_value.contains(id)) + .collect()) +} + /// Split `q` into terms: non-empty, whitespace-separated, capped in count and /// total length. fn search_terms(q: Option<&str>) -> Result, CanonicalError> { @@ -169,4 +227,29 @@ mod tests { ); Ok(()) } + + #[test] + fn a_uuid_term_names_a_person_while_the_rest_match_values() { + let terms = vec![ + "019e27bc-dec6-7773-b1e7-820ea2624b1b".to_owned(), + "ann".to_owned(), + ]; + + let (named, values) = partition_terms(&terms); + + assert_eq!(named.len(), 1, "the id is a name, not a value to match"); + assert_eq!(values, vec!["ann".to_owned()]); + } + + #[test] + fn the_excluded_sentinel_names_nobody() { + // It accumulates a journal row per exclusion, so it exists in the + // table — and it is still not a person the picker may offer. + let terms = vec![EXCLUDED_PERSON.to_string()]; + + let (named, values) = partition_terms(&terms); + + assert!(named.is_empty()); + assert!(values.is_empty(), "not matched as a value either"); + } } diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index cd61b0aa1..0f2333e05 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -509,7 +509,7 @@ "new_person": "New person:" }, "picker": { - "placeholder": "Search people by name, email or handle…", + "placeholder": "Search people by name, email, handle or person id…", "no_matches": "Nobody matches those terms.", "truncated": "More people matched than shown — narrow the terms.", "failed": "Search failed. Try again." diff --git a/src/frontend/src/mocks/handlers.ts b/src/frontend/src/mocks/handlers.ts index b83677b60..b6e96ee37 100644 --- a/src/frontend/src/mocks/handlers.ts +++ b/src/frontend/src/mocks/handlers.ts @@ -447,9 +447,13 @@ export const handlers = [ ); } const terms = q.toLowerCase().split(/\s+/); + // A term that parses as an id names a person, mirroring the service: it is + // the only way to reach someone the journal holds no values for. const items = PEOPLE.filter((p) => terms.every((term) => - [p.name, p.email, p.role].some((v) => v.toLowerCase().includes(term)), + isPersonId(term) + ? p.person_id.toLowerCase() === term + : [p.name, p.email, p.role].some((v) => v.toLowerCase().includes(term)), ), ) .slice(0, 20) From ab52e09bccb0c5a17d6dcdd4d1a7752b195469ad Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 06:14:38 +0300 Subject: [PATCH 11/19] identity: keep a login-minted person on the queue until a human decides it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First-login provisioning mints a person for a roster account with no address, so its owner can sign in. The trade is deliberate: "cannot sign in" becomes "possibly a duplicate person", because the same human may already exist from a source that does publish an address — and with no address here, nothing can join the two. Before this, that account left the operator's view at the moment it acquired a live owner: unbound with nothing to match on, it sat in the queue; bound by the sign-in, it counted as resolved and disappeared. Work vanished instead of arriving. It now has a group of its own, kept apart from the conflicts — a conflict asks which of two people, this asks whether one person or two — and it retires itself the moment an operator's decision lands, since the question it poses is exactly the one they answered. Still open (not in this change): marking such a person wherever they are CHOSEN, so nobody merges into a stub by accident. That needs a person-level flag, not a per-account one. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../backend/identity-resolution/openapi.json | 2 +- .../identity-resolution/src/api/resolution.rs | 3 +- .../src/domain/resolution.rs | 2 + .../src/domain/review_queue.rs | 72 ++++++++++++++++++- .../identity-resolution/src/domain/seed.rs | 6 ++ .../src/domain/seed_service.rs | 1 + .../src/infra/db/resolution_repo.rs | 7 +- .../src/infra/db/seed_repo.rs | 1 + .../portal/identities-view.test.tsx | 28 ++++++++ .../src/components/portal/identities-view.tsx | 7 +- src/frontend/src/locales/en/translation.json | 1 + src/frontend/src/mocks/handlers.ts | 13 ++++ tests/stand/api/schemas/identity.py | 2 +- 13 files changed, 139 insertions(+), 6 deletions(-) diff --git a/docs/components/backend/identity-resolution/openapi.json b/docs/components/backend/identity-resolution/openapi.json index 91d4fe602..3f6dc87d7 100644 --- a/docs/components/backend/identity-resolution/openapi.json +++ b/docs/components/backend/identity-resolution/openapi.json @@ -1042,7 +1042,7 @@ ] }, "kind": { - "description": "`contested` | `binding_conflict` | `no_evidence`.", + "description": "`contested` | `binding_conflict` | `provisioned_at_login` | `no_evidence`.", "type": "string" }, "manager_email": { diff --git a/src/backend/services/identity-resolution/src/api/resolution.rs b/src/backend/services/identity-resolution/src/api/resolution.rs index 517713cf9..c053d704a 100644 --- a/src/backend/services/identity-resolution/src/api/resolution.rs +++ b/src/backend/services/identity-resolution/src/api/resolution.rs @@ -627,7 +627,7 @@ pub struct AttentionParams { #[derive(Debug, Serialize, ToSchema)] pub struct QueueItemResponse { - /// `contested` | `binding_conflict` | `no_evidence`. + /// `contested` | `binding_conflict` | `provisioned_at_login` | `no_evidence`. pub kind: String, pub source: String, pub source_id: Uuid, @@ -894,6 +894,7 @@ fn kind_label(kind: ItemKind) -> &'static str { match kind { ItemKind::Contested => "contested", ItemKind::BindingConflict => "binding_conflict", + ItemKind::ProvisionedAtLogin => "provisioned_at_login", ItemKind::NoEvidence => "no_evidence", } } diff --git a/src/backend/services/identity-resolution/src/domain/resolution.rs b/src/backend/services/identity-resolution/src/domain/resolution.rs index 20c6bff69..1f90b94b4 100644 --- a/src/backend/services/identity-resolution/src/domain/resolution.rs +++ b/src/backend/services/identity-resolution/src/domain/resolution.rs @@ -199,6 +199,7 @@ mod tests { KnownBinding { person_id: Uuid::from_u128(person), author_person_id: Uuid::nil(), + provisioned_at_login: false, } } @@ -206,6 +207,7 @@ mod tests { KnownBinding { person_id: Uuid::from_u128(person), author_person_id: Uuid::from_u128(0xAD_1119), + provisioned_at_login: false, } } diff --git a/src/backend/services/identity-resolution/src/domain/review_queue.rs b/src/backend/services/identity-resolution/src/domain/review_queue.rs index 934c05155..340a99081 100644 --- a/src/backend/services/identity-resolution/src/domain/review_queue.rs +++ b/src/backend/services/identity-resolution/src/domain/review_queue.rs @@ -22,6 +22,11 @@ pub enum ItemKind { /// The account's identity value is shared by accounts bound to different /// persons, with no operator decision explaining the divergence. BindingConflict, + /// The binding was minted during a sign-in so its owner could get in. The + /// person may duplicate one the roster already knows — nothing could join + /// them, since the account carries no address — and only an operator can + /// say whether it is its own person or the same human. + ProvisionedAtLogin, /// The account carries no identity evidence automation can match on — /// e-mail is the only matching key today, so a username-only account is /// here too (shown with its username). Visible, never hidden: nothing @@ -117,7 +122,20 @@ pub fn build( for account in &active { match bindings.get(&account.account) { Some(binding) if binding.person_id == EXCLUDED_PERSON => rates.excluded += 1, - Some(_) => rates.bound += 1, + Some(binding) => { + rates.bound += 1; + // Bound, and still undecided: a login mint trades "cannot sign + // in" for "possibly a duplicate person", and the trade is only + // closed when a human says which it is. + if binding.provisioned_at_login && !binding.is_operator_authored() { + items.push(item( + ItemKind::ProvisionedAtLogin, + account, + Some(binding.person_id), + vec![binding.person_id], + )); + } + } None if !has_matchable_evidence(account) => { rates.no_evidence += 1; items.push(item(ItemKind::NoEvidence, account, None, Vec::new())); @@ -280,6 +298,15 @@ mod tests { KnownBinding { person_id: Uuid::from_u128(person), author_person_id: Uuid::nil(), + provisioned_at_login: false, + } + } + + fn login_bound(person: u128) -> KnownBinding { + KnownBinding { + person_id: Uuid::from_u128(person), + author_person_id: Uuid::nil(), + provisioned_at_login: true, } } @@ -287,6 +314,7 @@ mod tests { KnownBinding { person_id: Uuid::from_u128(person), author_person_id: Uuid::from_u128(0xAD_1119), + provisioned_at_login: false, } } @@ -410,6 +438,47 @@ mod tests { assert_eq!(hr[0].candidates.len(), 2, "both sides stay listed"); } + #[test] + fn a_login_minted_binding_waits_for_a_human_to_say_whose_it_is() { + // It IS bound — that is the point, its owner can sign in — so it counts + // as resolved while still needing a decision. + let mut bindings = HashMap::new(); + bindings.insert(account("github", "gh-1"), login_bound(7)); + + let review = build(vec![observed("github", "gh-1", None)], &bindings); + + assert_eq!(review.items.len(), 1); + assert_eq!(review.items[0].kind, ItemKind::ProvisionedAtLogin); + assert_eq!(review.items[0].bound_to, Some(Uuid::from_u128(7))); + assert_eq!( + review.items[0].candidates, + vec![Uuid::from_u128(7)], + "the minted person is the one being confirmed" + ); + assert_eq!(review.rates.bound, 1); + assert_eq!(review.rates.no_evidence, 0, "bound, so not unmatched"); + } + + #[test] + fn an_operator_decision_retires_the_login_mint() { + let mut bindings = HashMap::new(); + bindings.insert( + account("github", "gh-1"), + KnownBinding { + person_id: Uuid::from_u128(7), + author_person_id: Uuid::from_u128(99), + provisioned_at_login: true, + }, + ); + + let review = build(vec![observed("github", "gh-1", None)], &bindings); + + assert!( + review.items.is_empty(), + "a human has said whose it is; nothing left to ask" + ); + } + #[test] fn an_unbound_queue_item_is_bound_to_nobody() { let review = build(vec![observed("jira", "jr-1", None)], &HashMap::new()); @@ -480,6 +549,7 @@ mod tests { KnownBinding { person_id: EXCLUDED_PERSON, author_person_id: Uuid::from_u128(0xAD_1119), + provisioned_at_login: false, }, ); diff --git a/src/backend/services/identity-resolution/src/domain/seed.rs b/src/backend/services/identity-resolution/src/domain/seed.rs index f89df2df0..a84f06ef5 100644 --- a/src/backend/services/identity-resolution/src/domain/seed.rs +++ b/src/backend/services/identity-resolution/src/domain/seed.rs @@ -78,6 +78,9 @@ pub struct ProfileGroup { pub struct KnownBinding { pub person_id: Uuid, pub author_person_id: Uuid, + /// Written during a sign-in rather than by the batch: the person exists so + /// somebody could get in, and nobody has confirmed it is their own. + pub provisioned_at_login: bool, } impl KnownBinding { @@ -499,6 +502,7 @@ mod tests { KnownBinding { person_id: Uuid::from_u128(person), author_person_id: Uuid::nil(), + provisioned_at_login: false, } } @@ -506,6 +510,7 @@ mod tests { KnownBinding { person_id: Uuid::from_u128(person), author_person_id: Uuid::from_u128(0xAD_1119), + provisioned_at_login: false, } } @@ -716,6 +721,7 @@ mod tests { KnownBinding { person_id: EXCLUDED_PERSON, author_person_id: Uuid::from_u128(0xAD_1119), + provisioned_at_login: false, } } diff --git a/src/backend/services/identity-resolution/src/domain/seed_service.rs b/src/backend/services/identity-resolution/src/domain/seed_service.rs index 6614e9510..356fc6320 100644 --- a/src/backend/services/identity-resolution/src/domain/seed_service.rs +++ b/src/backend/services/identity-resolution/src/domain/seed_service.rs @@ -260,6 +260,7 @@ mod tests { KnownBinding { person_id: Uuid::from_u128(7), author_person_id: Uuid::nil(), + provisioned_at_login: false, }, ); let store = FakeStore { diff --git a/src/backend/services/identity-resolution/src/infra/db/resolution_repo.rs b/src/backend/services/identity-resolution/src/infra/db/resolution_repo.rs index 895e23a78..197d1796d 100644 --- a/src/backend/services/identity-resolution/src/infra/db/resolution_repo.rs +++ b/src/backend/services/identity-resolution/src/infra/db/resolution_repo.rs @@ -15,6 +15,7 @@ use sea_orm::{ }; use uuid::Uuid; +use crate::domain::login_bootstrap::LOGIN_BOOTSTRAP_REASON; use crate::domain::resolution::{BINDING_VALUE_TYPE, BindingRow}; use crate::domain::seed::{KnownBinding, SourceAccountKey}; @@ -38,6 +39,7 @@ pub async fn current_bindings( value_id AS source_account_id, person_id, author_person_id, + reason, ROW_NUMBER() OVER ( PARTITION BY insight_tenant_id, insight_source_type, insight_source_id, value_id ORDER BY created_at DESC, id DESC @@ -49,7 +51,8 @@ pub async fn current_bindings( AND (insight_source_type, insight_source_id, value_id) IN ("; const SQL_SUFFIX: &str = r") ) - SELECT insight_source_type, insight_source_id, source_account_id, person_id, author_person_id + SELECT insight_source_type, insight_source_id, source_account_id, person_id, + author_person_id, reason FROM ranked WHERE rn = 1 "; @@ -95,6 +98,7 @@ fn collect_bindings( let account_id: String = row.try_get("", "source_account_id")?; let person_id: Vec = row.try_get("", "person_id")?; let author_person_id: Vec = row.try_get("", "author_person_id")?; + let reason: Option = row.try_get("", "reason")?; map.insert( SourceAccountKey { source_type, @@ -104,6 +108,7 @@ fn collect_bindings( KnownBinding { person_id: Uuid::from_slice(&person_id)?, author_person_id: Uuid::from_slice(&author_person_id)?, + provisioned_at_login: reason.as_deref() == Some(LOGIN_BOOTSTRAP_REASON), }, ); } diff --git a/src/backend/services/identity-resolution/src/infra/db/seed_repo.rs b/src/backend/services/identity-resolution/src/infra/db/seed_repo.rs index d66e4d8c6..e151b6ee9 100644 --- a/src/backend/services/identity-resolution/src/infra/db/seed_repo.rs +++ b/src/backend/services/identity-resolution/src/infra/db/seed_repo.rs @@ -191,6 +191,7 @@ pub async fn known_account_bindings( KnownBinding { person_id: Uuid::from_slice(&person_id)?, author_person_id: Uuid::from_slice(&author_person_id)?, + provisioned_at_login: false, }, ); } diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index e302497ef..2872f323a 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -250,6 +250,34 @@ describe("IdentitiesView", () => { expect(within(row).getByText(/github · 921/)).toBeInTheDocument(); }); + // First-login provisioning trades "cannot sign in" for "possibly a duplicate + // person". Bound is not decided: without a group of its own the trade would + // leave the queue silently, at the moment the account gained a live owner. + it("keeps a login-minted binding on the queue until a human decides it", () => { + attention.q.data = { + items: [ + item({ + kind: "provisioned_at_login", + account_id: "new-joiner", + email: null, + username: "new-joiner", + bound_to: "01900000-0000-7000-8000-0000000000c0", + candidates: [ + { + person_id: "01900000-0000-7000-8000-0000000000c0", + display_name: "Carol Chen", + }, + ], + }), + ], + rates: RATES, + }; + render(); + + expect(screen.getByText(/given a person at first sign-in/i)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /new-joiner/i })).toBeInTheDocument(); + }); + it("renders candidates as person cells", () => { attention.q.data = { items: [ diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 4d72e0e60..e8508c8f6 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -72,7 +72,12 @@ import { } from "lucide-react"; /** Queue groups in working order: conflicts first, then the unknowns. */ -const KIND_ORDER = ["contested", "binding_conflict", "no_evidence"] as const; +const KIND_ORDER = [ + "contested", + "binding_conflict", + "provisioned_at_login", + "no_evidence", +] as const; // Binding states, not workloads: every one of these counts accounts the // resolver has already placed or will place by itself. The only number an diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index 0f2333e05..9c02d36dd 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -421,6 +421,7 @@ "kind": { "contested": "Contested — more than one person claims the account", "binding_conflict": "Binding conflicts — evidence disagrees with the current binding", + "provisioned_at_login": "Given a person at first sign-in — not yet confirmed", "no_evidence": "No address to match on — only a person can bind these", "other": "Needs review" }, diff --git a/src/frontend/src/mocks/handlers.ts b/src/frontend/src/mocks/handlers.ts index b6e96ee37..af60cc4f4 100644 --- a/src/frontend/src/mocks/handlers.ts +++ b/src/frontend/src/mocks/handlers.ts @@ -560,6 +560,19 @@ export const handlers = [ username: "ci-bot-7", candidates: [], }, + { + // Minted during a sign-in so its owner could get in: bound, and + // still nobody's decision. It may duplicate a person the roster + // already knows, which only an operator can settle. + kind: "provisioned_at_login", + source: "github", + source_id: "01900000-0000-7000-8000-00000000aa01", + account_id: "new-joiner", + email: null, + username: "new-joiner", + bound_to: carol?.person_id, + candidates: [card(carol)], + }, { // Neither address nor handle — nothing automation can match on. The // source still describes the human, which is what the fold reads for diff --git a/tests/stand/api/schemas/identity.py b/tests/stand/api/schemas/identity.py index 5a685e5e8..7c6b1ee73 100644 --- a/tests/stand/api/schemas/identity.py +++ b/tests/stand/api/schemas/identity.py @@ -353,7 +353,7 @@ class QueueItemResponse(BaseModel): display_name: str | None = Field(None, description='How the source describes the account. Nothing here is matchable — it is\nwhat lets an operator recognise whose account this is when automation\ncannot, which is exactly the case for the ones only they can bind.') email: str | None = None job_title: str | None = None - kind: str = Field(..., description='`contested` | `binding_conflict` | `no_evidence`.') + kind: str = Field(..., description='`contested` | `binding_conflict` | `provisioned_at_login` | `no_evidence`.') manager_email: str | None = None source: str source_id: UUID From 8f089ee8ed90847e607e5d996c1990702cf644ba Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 06:34:57 +0300 Subject: [PATCH 12/19] identity: let an account's trail say who decided it and why MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every verb takes a comment, and every call has always stored one — into the operations journal, next to the operator, the verb and the per-account outcomes. The account's trail read a different table and showed none of it, so the one question an audit trail is opened for went unanswered: not what changed, but why somebody changed it. The trail now carries both records. A binding row still says what a decision did; an operator call says who ran it, what they typed, how far it reached and what it did to this account. They are matched by the account named in the call's own payload — never by proximity in time — and rendered as two rows, not folded into one: a merge moves a dozen accounts, and claiming this account's row IS that call would invent a link the journal does not record. The people a trail names are resolved too, so an entry no longer degrades to a bare id whenever it points at somebody who is not a queue candidate — which is most of them, once an account has been decided. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../backend/identity-resolution/openapi.json | 90 ++++++++++++- .../identity-resolution/src/api/resolution.rs | 119 ++++++++++++++++++ .../src/infra/db/ops_repo.rs | 40 ++++++ src/frontend/src/api/identity-client.ts | 26 ++++ .../components/portal/account-detail.test.tsx | 62 +++++++++ .../src/components/portal/account-detail.tsx | 117 +++++++++++++++-- .../src/components/portal/identities-view.tsx | 7 +- src/frontend/src/locales/en/translation.json | 14 ++- src/frontend/src/mocks/handlers.ts | 37 ++++++ tests/stand/api/schemas/identity.py | 64 +++++++--- 10 files changed, 537 insertions(+), 39 deletions(-) diff --git a/docs/components/backend/identity-resolution/openapi.json b/docs/components/backend/identity-resolution/openapi.json index 3f6dc87d7..5bbbf98af 100644 --- a/docs/components/backend/identity-resolution/openapi.json +++ b/docs/components/backend/identity-resolution/openapi.json @@ -12,6 +12,13 @@ }, "type": "array" }, + "operations": { + "description": "Operator calls that named this account, newest first. Absent from the\nbinding journal by design: one call can move many accounts, and only\nthe call knows why it was made.", + "items": { + "$ref": "#/components/schemas/AccountOperationResponse" + }, + "type": "array" + }, "person_id": { "description": "The binding in force now, if the account has one.", "format": "uuid", @@ -32,7 +39,66 @@ "source", "source_id", "account_id", - "history" + "history", + "operations" + ], + "type": "object" + }, + "AccountOperationResponse": { + "description": "One operator call that named this account.\n\nThe binding journal says what a decision did; this says who ran it, the\nreason they gave, and how far it reached — a merge lands one row here and\none in every other account it moved.", + "properties": { + "accounts_touched": { + "description": "Accounts the call named, this one included.", + "minimum": 0, + "type": "integer" + }, + "author": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/PersonSummaryResponse", + "description": "The author as a card, when the journal knows them by more than an id." + } + ] + }, + "author_person_id": { + "format": "uuid", + "type": "string" + }, + "comment": { + "description": "What the operator typed, when they typed anything.", + "type": [ + "string", + "null" + ] + }, + "operation_id": { + "format": "uuid", + "type": "string" + }, + "outcome": { + "description": "What the call did to THIS account: `applied` | `already_decided` |\n`refused`. A refusal changed nothing and still belongs in the trail.", + "type": [ + "string", + "null" + ] + }, + "recorded_at": { + "type": "string" + }, + "verb": { + "description": "`operator-bind` | `operator-merge` | `operator-detach` | `operator-exclude`.", + "type": "string" + } + }, + "required": [ + "operation_id", + "verb", + "author_person_id", + "accounts_touched", + "recorded_at" ], "type": "object" }, @@ -249,6 +315,17 @@ "HistoryEntry": { "description": "One decision in an account's history.", "properties": { + "author": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/PersonSummaryResponse", + "description": "The operator who decided it; absent for automation, which has no card." + } + ] + }, "author_person_id": { "format": "uuid", "type": "string" @@ -257,6 +334,17 @@ "description": "`true` when a person made this decision, `false` for automation.", "type": "boolean" }, + "person": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/PersonSummaryResponse", + "description": "The person this decision pointed at, as a card when one is known." + } + ] + }, "person_id": { "format": "uuid", "type": "string" diff --git a/src/backend/services/identity-resolution/src/api/resolution.rs b/src/backend/services/identity-resolution/src/api/resolution.rs index c053d704a..f29f78084 100644 --- a/src/backend/services/identity-resolution/src/api/resolution.rs +++ b/src/backend/services/identity-resolution/src/api/resolution.rs @@ -29,6 +29,10 @@ use crate::infra::identity_evidence::{ AccountEvidence, ClickHouseEvidenceReader, EvidenceSnapshot, }; +/// A trail is read, not paged: enough calls to cover any account's history +/// without letting one response grow without bound. +const MAX_ACCOUNT_OPERATIONS: u64 = 50; + /// How many accounts one bulk call may carry — a prepared matching table is /// pasted by a human, not streamed. const MAX_BULK_ITEMS: usize = 1_000; @@ -764,13 +768,40 @@ pub async fn attention( #[derive(Debug, Serialize, ToSchema)] pub struct HistoryEntry { pub person_id: Uuid, + /// The person this decision pointed at, as a card when one is known. + pub person: Option, pub author_person_id: Uuid, + /// The operator who decided it; absent for automation, which has no card. + pub author: Option, /// `true` when a person made this decision, `false` for automation. pub by_operator: bool, pub reason: Option, pub recorded_at: String, } +/// One operator call that named this account. +/// +/// The binding journal says what a decision did; this says who ran it, the +/// reason they gave, and how far it reached — a merge lands one row here and +/// one in every other account it moved. +#[derive(Debug, Serialize, ToSchema)] +pub struct AccountOperationResponse { + pub operation_id: Uuid, + /// `operator-bind` | `operator-merge` | `operator-detach` | `operator-exclude`. + pub verb: String, + pub author_person_id: Uuid, + /// The author as a card, when the journal knows them by more than an id. + pub author: Option, + /// What the operator typed, when they typed anything. + pub comment: Option, + /// Accounts the call named, this one included. + pub accounts_touched: usize, + /// What the call did to THIS account: `applied` | `already_decided` | + /// `refused`. A refusal changed nothing and still belongs in the trail. + pub outcome: Option, + pub recorded_at: String, +} + #[derive(Debug, Serialize, ToSchema)] pub struct AccountBindingResponse { pub source: String, @@ -779,6 +810,10 @@ pub struct AccountBindingResponse { /// The binding in force now, if the account has one. pub person_id: Option, pub history: Vec, + /// Operator calls that named this account, newest first. Absent from the + /// binding journal by design: one call can move many accounts, and only + /// the call knows why it was made. + pub operations: Vec, } impl toolkit::api::api_dto::ResponseApiDto for AccountBindingResponse {} @@ -806,26 +841,110 @@ pub async fn account_binding( .await .map_err(|e| internal(&e, "failed to read the binding history"))?; + let operations = ops_repo::corrections_for_account( + &state.db, + tenant, + RESOLUTION_OP, + &source, + source_id, + &account_id, + MAX_ACCOUNT_OPERATIONS, + ) + .await + .map_err(|e| internal(&e, "failed to read the account's operations"))?; + + // One hydration for every person the trail names — the people it points at + // and the operators who decided — so no consumer is left resolving ids. + let named: Vec = history + .iter() + .flat_map(|h| [h.person_id, h.author_person_id]) + .chain(operations.iter().map(|o| o.author_person_id)) + .filter(|id| !id.is_nil()) + .collect::>() + .into_iter() + .collect(); + let cards = persons_repo::person_cards(&state.db, tenant, &named) + .await + .map_err(|e| internal(&e, "failed to read the persons named in the trail"))?; + let card_of = |id: Uuid| cards.get(&id).cloned().map(PersonSummaryResponse::from); + let entries: Vec = history .iter() .map(|h| HistoryEntry { person_id: h.person_id, + person: card_of(h.person_id), author_person_id: h.author_person_id, + author: card_of(h.author_person_id), by_operator: !h.author_person_id.is_nil(), reason: h.reason.clone(), recorded_at: super::seed::fmt_ts(h.created_at), }) .collect(); + let calls = operations + .iter() + .map(|op| account_operation(op, &account, card_of(op.author_person_id))) + .collect(); + Ok(Json(AccountBindingResponse { source, source_id, account_id, person_id: history.first().map(|h| h.person_id), history: entries, + operations: calls, })) } +/// Read one correction call as the trail shows it, from the payload it stored. +fn account_operation( + op: &ops_repo::Operation, + account: &SourceAccountKey, + author: Option, +) -> AccountOperationResponse { + let request: serde_json::Value = op + .request_json + .as_deref() + .and_then(|raw| serde_json::from_str(raw).ok()) + .unwrap_or(serde_json::Value::Null); + let accounts = request + .get("accounts") + .and_then(serde_json::Value::as_array) + .map_or(&[][..], Vec::as_slice); + + AccountOperationResponse { + operation_id: op.operation_id, + verb: request + .get("verb") + .and_then(serde_json::Value::as_str) + .unwrap_or_default() + .to_owned(), + author_person_id: op.author_person_id, + author, + comment: request + .get("comment") + .and_then(serde_json::Value::as_str) + .map(str::trim) + .filter(|c| !c.is_empty()) + .map(str::to_owned), + accounts_touched: accounts.len(), + outcome: accounts + .iter() + .find(|entry| names_account(entry, account)) + .and_then(|entry| entry.get("outcome")) + .and_then(serde_json::Value::as_str) + .map(str::to_owned), + recorded_at: super::seed::fmt_ts(op.started_at), + } +} + +fn names_account(entry: &serde_json::Value, account: &SourceAccountKey) -> bool { + entry.get("source").and_then(serde_json::Value::as_str) == Some(&account.source_type) + && entry.get("account_id").and_then(serde_json::Value::as_str) == Some(&account.account_id) + && entry.get("source_id").and_then(serde_json::Value::as_str) + == Some(&account.source_id.to_string()) +} + #[derive(Debug, Serialize, ToSchema)] pub struct PersonAccountEntry { pub source: String, diff --git a/src/backend/services/identity-resolution/src/infra/db/ops_repo.rs b/src/backend/services/identity-resolution/src/infra/db/ops_repo.rs index a1ba5064c..654709231 100644 --- a/src/backend/services/identity-resolution/src/infra/db/ops_repo.rs +++ b/src/backend/services/identity-resolution/src/infra/db/ops_repo.rs @@ -243,6 +243,46 @@ pub async fn list( rows.iter().map(row_to_operation).collect() } +/// Every correction call that named this account, newest first. +/// +/// The binding journal records WHAT each decision did; the operation records +/// who ran it, why they said they ran it, and how many accounts went with it. +/// Matching is by containment in the request payload rather than by time, so +/// nothing is attributed to a call that did not name the account. +/// +/// # Errors +/// +/// Returns an error if the query fails. +pub async fn corrections_for_account( + db: &DatabaseConnection, + tenant_id: Uuid, + operation_type: &str, + source_type: &str, + source_id: Uuid, + account_id: &str, + limit: u64, +) -> anyhow::Result> { + let sql = format!( + "SELECT {COLUMNS} FROM operations WHERE insight_tenant_id = ? AND operation_type = ? AND JSON_CONTAINS( request_json, JSON_OBJECT('source', ?, 'source_id', ?, 'account_id', ?), '$.accounts') ORDER BY started_at DESC, operation_id DESC LIMIT ?" + ); + + let rows = db + .query_all(Statement::from_sql_and_values( + DbBackend::MySql, + &sql, + [ + tenant_id.as_bytes().to_vec().into(), + operation_type.into(), + source_type.into(), + source_id.to_string().into(), + account_id.into(), + limit.into(), + ], + )) + .await?; + rows.iter().map(row_to_operation).collect() +} + /// Fail every `queued`/`running` operation whose `started_at` is older than /// `older_than`. Run once at worker startup so a pod restart cannot leave a row /// stuck in `running` forever (its in-memory job is gone). Intentionally NOT diff --git a/src/frontend/src/api/identity-client.ts b/src/frontend/src/api/identity-client.ts index 460c42d85..a68912e19 100644 --- a/src/frontend/src/api/identity-client.ts +++ b/src/frontend/src/api/identity-client.ts @@ -177,7 +177,11 @@ export async function getAttention(limit = 200): Promise { /** One decision recorded for an account, newest first on the wire. */ export interface BindingHistoryEntry { person_id: string; + /** The person it pointed at, as a card when the journal knows one. */ + person?: PersonSummary | null; author_person_id: string; + /** The operator who decided it; absent for automation. */ + author?: PersonSummary | null; /** True when a human recorded it; false = automation (seed/resolver). */ by_operator: boolean; /** Verb code (`operator-bind`, …) or an automation reason. Open vocabulary. */ @@ -185,6 +189,25 @@ export interface BindingHistoryEntry { recorded_at: string; } +/** + * One operator call that named this account. Separate from the binding journal + * by design: a call can move many accounts, and only the call knows why it was + * made. + */ +export interface AccountOperation { + operation_id: string; + verb: string; + author_person_id: string; + author?: PersonSummary | null; + /** What the operator typed, when they typed anything. */ + comment?: string | null; + /** Accounts the call named, this one included. */ + accounts_touched: number; + /** What it did to THIS account: `applied` | `already_decided` | `refused`. */ + outcome?: string | null; + recorded_at: string; +} + export interface AccountBinding { source: string; source_id: string; @@ -192,6 +215,9 @@ export interface AccountBinding { /** Absent = the account is currently bound to nobody. */ person_id?: string | null; history: BindingHistoryEntry[]; + /** Operator calls naming this account, newest first. Optional so a client + * deployed ahead of the backend keeps working. */ + operations?: AccountOperation[]; } /** diff --git a/src/frontend/src/components/portal/account-detail.test.tsx b/src/frontend/src/components/portal/account-detail.test.tsx index 17f35bdfa..3e7631718 100644 --- a/src/frontend/src/components/portal/account-detail.test.tsx +++ b/src/frontend/src/components/portal/account-detail.test.tsx @@ -45,6 +45,11 @@ const BOB = { display_name: "Bob Park", email: "bob.park@example.com", }; +const CAROL = { + person_id: "01900000-0000-7000-8000-0000000000c0", + display_name: "Carol Chen", + email: "carol.chen@example.com", +}; function queueItem(over: Partial = {}): AttentionItem { return { @@ -171,6 +176,63 @@ describe("AccountDetail", () => { expect(screen.queryByText("login-bootstrap")).not.toBeInTheDocument(); }); + // The comment is the one thing no other record holds — why a human did + // this — and it was written to the operations journal from the first verb, + // never read back. The reach matters beside it: a merge lands one row here + // and one in every other account it moved. + it("shows the operator call behind a decision: who, why and how far", () => { + binding.q.data = bound({ + history: [ + { + person_id: BOB.person_id, + author_person_id: CAROL.person_id, + by_operator: true, + reason: "operator-merge", + recorded_at: "2026-08-01T10:15:00.000000", + }, + ], + operations: [ + { + operation_id: "01900000-0000-7000-8000-0000000000f1", + verb: "operator-merge", + author_person_id: CAROL.person_id, + author: CAROL, + comment: "Same person — confirmed with HR.", + accounts_touched: 12, + outcome: "applied", + recorded_at: "2026-08-01T10:15:00.000000", + }, + ], + }); + render(); + + expect(screen.getByText(/same person — confirmed with HR/i)).toBeInTheDocument(); + expect(screen.getByText(/12 accounts in this call/i)).toBeInTheDocument(); + expect(screen.getAllByText(/Carol Chen/).length).toBeGreaterThan(0); + expect(screen.getByText("applied")).toBeInTheDocument(); + }); + + // The service resolves the people a trail names, so an entry pointing at + // somebody who is not a candidate stops being a bare id. + it("names the person an entry points at, candidate or not", () => { + binding.q.data = bound({ + history: [ + { + person_id: CAROL.person_id, + person: CAROL, + author_person_id: "00000000-0000-0000-0000-000000000000", + by_operator: false, + reason: "", + recorded_at: "2026-07-01T10:15:00.000000", + }, + ], + }); + // CAROL is not in the queue item's candidates. + render(); + + expect(screen.getByText("Carol Chen")).toBeInTheDocument(); + }); + it("reads an off-queue empty journal as a stale link, offering no verbs", () => { binding.q.data = bound({ person_id: null, history: [] }); render(); diff --git a/src/frontend/src/components/portal/account-detail.tsx b/src/frontend/src/components/portal/account-detail.tsx index 748e7c39e..a4ed5c4ec 100644 --- a/src/frontend/src/components/portal/account-detail.tsx +++ b/src/frontend/src/components/portal/account-detail.tsx @@ -9,13 +9,15 @@ * or decided answers 200 with an empty journal, so "not in the queue, no * binding, no history" is the stale-link state — it says so instead of * offering verbs whose bind would pre-register a typo as a real account. - * Person ids in the history arrive bare (there is no id→name read for - * arbitrary persons yet); when an id matches a hydrated candidate we show the - * card, otherwise the id itself — honest over pretty. + * The trail keeps two records in one order: a binding row says what changed, + * an operator call says who ran it and why. They stay separate rows — one call + * can move a dozen accounts, and folding it into this account's row would + * invent a link the journal does not record. */ import { useTranslation } from "react-i18next"; import type { + AccountOperation, AttentionItem, BindingHistoryEntry, PersonSummary, @@ -122,13 +124,17 @@ export function AccountDetail({ // remaining space collapses to a sliver, and a one-line history is // unreadable. Below the floor the window itself scrolls.
      - {binding.data.history.map((entry, index) => ( - - ))} + {trail(binding.data.history, binding.data.operations).map((row) => + row.kind === "decision" ? ( + + ) : ( + + ), + )}
    )} @@ -136,6 +142,84 @@ export function AccountDetail({ ); } +type TrailRow = + | { kind: "decision"; key: string; entry: BindingHistoryEntry } + | { kind: "call"; key: string; operation: AccountOperation }; + +/** + * The two records an account's past is kept in, in one order. + * + * A binding row says what changed; an operator call says who ran it and why. + * They are deliberately not merged into one another — the call may have moved + * a dozen other accounts, and claiming otherwise would invent a link the + * journal does not record. Shown side by side in time, the pair reads itself. + */ +function trail( + history: BindingHistoryEntry[], + operations: AccountOperation[] | undefined, +): TrailRow[] { + const rows: TrailRow[] = [ + ...history.map((entry, index) => ({ + kind: "decision" as const, + key: `d-${entry.recorded_at}-${index}`, + entry, + })), + ...(operations ?? []).map((operation) => ({ + kind: "call" as const, + key: `c-${operation.operation_id}`, + operation, + })), + ]; + const at = (row: TrailRow) => + row.kind === "decision" ? row.entry.recorded_at : row.operation.recorded_at; + return rows.sort((a, b) => at(b).localeCompare(at(a))); +} + +function OperationRow({ operation }: { operation: AccountOperation }) { + const { t } = useTranslation(); + const verbKey = VERB_KEYS[operation.verb]; + return ( +
  • +
    + + {verbKey ? t(verbKey) : operation.verb} + + {operation.accounts_touched > 1 ? ( + + {t("identities.history.accounts_touched", { + count: operation.accounts_touched, + })} + + ) : null} + + {formatUtcInstant(operation.recorded_at, "d MMM yyyy, HH:mm")} + + {formatUtcAge(operation.recorded_at)} + + +
    +
    + {t("identities.history.by")} + {operation.author ? ( + + {personDisplayName(operation.author)} + + ) : null} + + {operation.outcome ? ( + {operation.outcome} + ) : null} +
    + {/* The one thing no other record holds: why a human did this. */} + {operation.comment ? ( +

    + {operation.comment} +

    + ) : null} +
    • + ); +} + function HistoryRow({ entry, candidates, @@ -149,7 +233,10 @@ function HistoryRow({ // entry, which is most of them. const reason = entry.reason?.trim() || undefined; const verbKey = reason ? VERB_KEYS[reason] : undefined; - const target = candidates.find((c) => c.person_id === entry.person_id); + // The card the service resolved wins; the queue's candidates are the + // fallback for a backend that does not send one yet. + const target = + entry.person ?? candidates.find((c) => c.person_id === entry.person_id); return (
  • @@ -175,7 +262,13 @@ function HistoryRow({ ) : null} {entry.by_operator ? ( - {t("identities.history.by_operator")} + + {entry.author + ? t("identities.history.by_name", { + name: personDisplayName(entry.author), + }) + : t("identities.history.by_operator")} + ) : null}
    • diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index e8508c8f6..618bfd887 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -564,8 +564,11 @@ function CaseBlock({ {disputed ? (
    - {t("identities.queue.case_summary", { - people: queueCase.candidates.length, + {t("identities.queue.case_people", { + count: queueCase.candidates.length, + })} + {" · "} + {t("identities.queue.case_accounts", { count: queueCase.items.length, })}
    diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index 9c02d36dd..0d34af80c 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -431,8 +431,6 @@ "truncated": "The evidence read hit its cap — these counts and this queue cover only part of the observed accounts, not the whole tenant.", "empty_title": "Everything is resolved", "empty_description": "Every observed account is bound to a person. New conflicts will appear here.", - "case_summary_one": "{{people}} people, {{count}} account", - "case_summary_other": "{{people}} people, {{count}} accounts", "show_more_one": "Show {{count}} more case", "show_more_other": "Show {{count}} more cases", "case_count_one": "{{count}} case · {{accounts}} accounts", @@ -443,7 +441,11 @@ "previous_case": "‹ Previous account", "next_case": "Next account ›", "reports_to": "reports to {{manager}}", - "bound_to": "now {{name}}" + "bound_to": "now {{name}}", + "case_people_one": "{{count}} person", + "case_people_other": "{{count}} people", + "case_accounts_one": "{{count}} account", + "case_accounts_other": "{{count}} accounts" }, "detail": { "not_found": "This account is unknown — the link may be stale.", @@ -468,7 +470,11 @@ "automatic": "Automatic", "to": "to", "by_operator": "by an operator", - "login_bootstrap": "First sign-in" + "login_bootstrap": "First sign-in", + "by": "by", + "by_name": "by {{name}}", + "accounts_touched_one": "{{count}} account in this call", + "accounts_touched_other": "{{count}} accounts in this call" }, "actions": { "bind": "Bind", diff --git a/src/frontend/src/mocks/handlers.ts b/src/frontend/src/mocks/handlers.ts index af60cc4f4..9e02fee0d 100644 --- a/src/frontend/src/mocks/handlers.ts +++ b/src/frontend/src/mocks/handlers.ts @@ -412,7 +412,23 @@ export const handlers = [ history: [ { person_id: bob?.person_id, + person: bob + ? { + person_id: bob.person_id, + email: bob.email, + display_name: bob.name, + job_title: bob.role, + } + : null, author_person_id: carol?.person_id, + author: carol + ? { + person_id: carol.person_id, + email: carol.email, + display_name: carol.name, + job_title: carol.role, + } + : null, by_operator: true, reason: "operator-bind", recorded_at: "2026-08-01T10:15:00.000000", @@ -434,6 +450,27 @@ export const handlers = [ recorded_at: "2026-07-01T06:30:00.000000", }, ], + // The call behind the operator's row above: who ran it, how far it + // reached, and the one thing no other record holds — why. + operations: [ + { + operation_id: "01900000-0000-7000-8000-0000000000f1", + verb: "operator-bind", + author_person_id: carol?.person_id, + author: carol + ? { + person_id: carol.person_id, + email: carol.email, + display_name: carol.name, + job_title: carol.role, + } + : null, + comment: "Checked with HR — same person, the chat handle is theirs.", + accounts_touched: 3, + outcome: "applied", + recorded_at: "2026-08-01T10:15:00.000000", + }, + ], }); }, ), diff --git a/tests/stand/api/schemas/identity.py b/tests/stand/api/schemas/identity.py index 7c6b1ee73..137bb6f4d 100644 --- a/tests/stand/api/schemas/identity.py +++ b/tests/stand/api/schemas/identity.py @@ -107,20 +107,6 @@ class CreateVisibilityRequest(BaseModel): viewer_person_id: UUID -class HistoryEntry(BaseModel): - """ - One decision in an account's history. - """ - model_config = ConfigDict( - extra='forbid', - ) - author_person_id: UUID - by_operator: bool = Field(..., description='`true` when a person made this decision, `false` for automation.') - person_id: UUID - reason: str | None = None - recorded_at: str - - class ItemResult(BaseModel): """ What happened to one requested account. @@ -477,15 +463,25 @@ class VisiblePersonsResponse(BaseModel): visible: list[UUID] -class AccountBindingResponse(BaseModel): +class AccountOperationResponse(BaseModel): + """ + One operator call that named this account. + + The binding journal says what a decision did; this says who ran it, the + reason they gave, and how far it reached — a merge lands one row here and + one in every other account it moved. + """ model_config = ConfigDict( extra='forbid', ) - account_id: str - history: list[HistoryEntry] - person_id: UUID | None = Field(None, description='The binding in force now, if the account has one.') - source: str - source_id: UUID + accounts_touched: int = Field(..., description='Accounts the call named, this one included.', ge=0) + author: PersonSummaryResponse | None = None + author_person_id: UUID + comment: str | None = Field(None, description='What the operator typed, when they typed anything.') + operation_id: UUID + outcome: str | None = Field(None, description='What the call did to THIS account: `applied` | `already_decided` |\n`refused`. A refusal changed nothing and still belongs in the trail.') + recorded_at: str + verb: str = Field(..., description='`operator-bind` | `operator-merge` | `operator-detach` | `operator-exclude`.') class AttentionResponse(BaseModel): @@ -508,6 +504,22 @@ class CorrectionResponse(BaseModel): new_person_id: UUID | None = Field(None, description='Set by `detach` when the account reached the new person; absent when\nthe write was refused, since no binding points at that id.') +class HistoryEntry(BaseModel): + """ + One decision in an account's history. + """ + model_config = ConfigDict( + extra='forbid', + ) + author: PersonSummaryResponse | None = None + author_person_id: UUID + by_operator: bool = Field(..., description='`true` when a person made this decision, `false` for automation.') + person: PersonSummaryResponse | None = None + person_id: UUID + reason: str | None = None + recorded_at: str + + class MeResponse(BaseModel): """ The caller as the gateway JWT identifies them, with their active roles. @@ -596,5 +608,17 @@ class VisibilityListResponse(BaseModel): next_cursor: str | None = Field(None, description='Wire parity with the .NET `ListResponse`: the cursor is declared\nbut pagination is not implemented — always `null` (both\nimplementations return every row; consumers already tolerate it).') +class AccountBindingResponse(BaseModel): + model_config = ConfigDict( + extra='forbid', + ) + account_id: str + history: list[HistoryEntry] + operations: list[AccountOperationResponse] = Field(..., description='Operator calls that named this account, newest first. Absent from the\nbinding journal by design: one call can move many accounts, and only\nthe call knows why it was made.') + person_id: UUID | None = Field(None, description='The binding in force now, if the account has one.') + source: str + source_id: UUID + + PersonResponse.model_rebuild() SubchartNode.model_rebuild() From 1f9b9571b3c360747c4cb95324e915feea281f81 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 06:41:39 +0300 Subject: [PATCH 13/19] identity: mark a person who exists only because somebody signed in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A login-minted person is a stub: the journal holds one binding for them and nothing else — no name, no address, no decision — until the resolver adopts them or an operator says whose they are. They may well duplicate a person the roster already knows, which is the trade first-login provisioning makes. The queue says so for the account. The person themselves said nothing, and they appear where it matters most: in the picker, which is where the wrong person gets chosen, and in a candidate list, where a merge INTO the stub is the wrong direction — the history is on the other side of it. The mark is a read over their bindings rather than a card attribute, because that is what it is a fact about: a card is assembled from observed values, and a stub has none. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../backend/identity-resolution/openapi.json | 4 ++ .../identity-resolution/src/api/persons.rs | 3 ++ .../identity-resolution/src/api/resolution.rs | 46 ++++++++++++++++- .../src/infra/db/persons_repo.rs | 50 ++++++++++++++++++- src/frontend/src/api/identity-client.ts | 4 ++ .../components/portal/person-cell.test.tsx | 10 ++++ .../src/components/portal/person-cell.tsx | 9 ++++ src/frontend/src/locales/en/translation.json | 3 +- src/frontend/src/mocks/handlers.ts | 2 +- tests/stand/api/schemas/identity.py | 1 + 10 files changed, 128 insertions(+), 4 deletions(-) diff --git a/docs/components/backend/identity-resolution/openapi.json b/docs/components/backend/identity-resolution/openapi.json index 5bbbf98af..91b21c14e 100644 --- a/docs/components/backend/identity-resolution/openapi.json +++ b/docs/components/backend/identity-resolution/openapi.json @@ -722,6 +722,10 @@ "format": "uuid", "type": "string" }, + "provisional": { + "description": "The journal holds nothing but a login-mint for this person: they exist\nso somebody could sign in, and may duplicate one the roster knows. Not\na merge target — the history is on the other side.", + "type": "boolean" + }, "status": { "type": [ "string", diff --git a/src/backend/services/identity-resolution/src/api/persons.rs b/src/backend/services/identity-resolution/src/api/persons.rs index 9bf704111..2243540c2 100644 --- a/src/backend/services/identity-resolution/src/api/persons.rs +++ b/src/backend/services/identity-resolution/src/api/persons.rs @@ -97,6 +97,9 @@ pub async fn search_persons( .map(PersonSummaryResponse::from) .collect(); sort_for_display(&mut items); + // A picker is where the wrong person gets chosen, so a person who exists + // only because somebody signed in must say so here of all places. + super::resolution::mark_provisional(&state, tenant, &mut items).await?; Ok(Json(PersonListResponse { items, diff --git a/src/backend/services/identity-resolution/src/api/resolution.rs b/src/backend/services/identity-resolution/src/api/resolution.rs index f29f78084..882a36fa7 100644 --- a/src/backend/services/identity-resolution/src/api/resolution.rs +++ b/src/backend/services/identity-resolution/src/api/resolution.rs @@ -661,6 +661,11 @@ pub struct QueueItemResponse { #[derive(Debug, Serialize, ToSchema)] pub struct PersonSummaryResponse { pub person_id: Uuid, + /// The journal holds nothing but a login-mint for this person: they exist + /// so somebody could sign in, and may duplicate one the roster knows. Not + /// a merge target — the history is on the other side. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub provisional: bool, pub email: Option, /// Source-native handle (e.g. a git login) — often the only recognisable /// field of an identity no HR system has observed yet. @@ -674,6 +679,7 @@ impl From for PersonSummaryResponse { fn from(card: PersonCard) -> Self { Self { person_id: card.person_id, + provisional: false, email: card.email, username: card.username, display_name: card.display_name, @@ -683,6 +689,30 @@ impl From for PersonSummaryResponse { } } +/// Mark the cards the journal holds nothing but a login-mint for. +/// +/// A separate read rather than a card attribute: the mark is a fact about the +/// person's bindings, and the card is assembled from observed values. +/// +/// # Errors +/// +/// Returns an error if the read fails. +pub async fn mark_provisional( + state: &AppState, + tenant: Uuid, + people: &mut [PersonSummaryResponse], +) -> Result<(), CanonicalError> { + let ids: Vec = people.iter().map(|p| p.person_id).collect(); + let provisional = persons_repo::provisional_persons(&state.db, tenant, &ids) + .await + .map_err(|e| internal(&e, "failed to read which persons are provisional"))?; + + for person in people { + person.provisional = provisional.contains(&person.person_id); + } + Ok(()) +} + /// Share of observed accounts per resolution state — the operator-visible match /// rate. #[derive(Debug, Serialize, ToSchema)] @@ -728,6 +758,14 @@ pub async fn attention( let page: Vec<_> = review.items.into_iter().take(limit).collect(); let cards = candidate_cards(state.as_ref(), tenant, &page).await?; + let provisional = persons_repo::provisional_persons( + &state.db, + tenant, + &cards.keys().copied().collect::>(), + ) + .await + .map_err(|e| internal(&e, "failed to read which persons are provisional"))?; + let items = page .into_iter() .map(|i| QueueItemResponse { @@ -745,7 +783,13 @@ pub async fn attention( bound_to: i.bound_to, candidates: person_card::in_requested_order(&i.candidates, &cards) .into_iter() - .map(PersonSummaryResponse::from) + .map(|card| { + let is_provisional = provisional.contains(&card.person_id); + PersonSummaryResponse { + provisional: is_provisional, + ..PersonSummaryResponse::from(card) + } + }) .collect(), }) .collect(); diff --git a/src/backend/services/identity-resolution/src/infra/db/persons_repo.rs b/src/backend/services/identity-resolution/src/infra/db/persons_repo.rs index 24d4f8059..e5025711d 100644 --- a/src/backend/services/identity-resolution/src/infra/db/persons_repo.rs +++ b/src/backend/services/identity-resolution/src/infra/db/persons_repo.rs @@ -12,7 +12,7 @@ //! so an excluded account resolves as no person rather than as the shared //! sentinel, and an older binding is never resurrected past an exclusion. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use sea_orm::{ ColumnTrait, ConnectionTrait, DatabaseConnection, DbBackend, EntityTrait, QueryFilter, @@ -21,6 +21,7 @@ use sea_orm::{ use uuid::Uuid; use super::entities::persons; +use crate::domain::login_bootstrap::LOGIN_BOOTSTRAP_REASON; use crate::domain::person_card::{self, CARD_VALUE_TYPES, PersonCard}; use crate::domain::resolution::EXCLUDED_PERSON; @@ -423,6 +424,53 @@ fn like_pattern(term: &str) -> String { /// # Errors /// /// Returns an error if the query fails. +/// Of the given persons, those the journal holds nothing but login-mints for. +/// +/// Such a person exists so somebody could sign in, and nothing else about them +/// has been observed or decided yet — so they may well duplicate a person the +/// roster already knows. Naming one as a merge target is the wrong direction: +/// the history is on the other side. +/// +/// # Errors +/// +/// Returns an error if the query fails or a stored `person_id` is not 16 bytes. +pub async fn provisional_persons( + db: &DatabaseConnection, + tenant_id: Uuid, + person_ids: &[Uuid], +) -> anyhow::Result> { + if person_ids.is_empty() { + return Ok(HashSet::new()); + } + + let placeholders = vec!["?"; person_ids.len()].join(", "); + let sql = format!( + "SELECT person_id FROM persons WHERE insight_tenant_id = ? AND person_id IN ({placeholders}) GROUP BY person_id HAVING SUM(CASE WHEN reason <=> ? THEN 0 ELSE 1 END) = 0" + ); + + let mut params: Vec = Vec::with_capacity(person_ids.len() + 2); + params.push(tenant_id.as_bytes().to_vec().into()); + for id in person_ids { + params.push(id.as_bytes().to_vec().into()); + } + params.push(LOGIN_BOOTSTRAP_REASON.into()); + + let rows = db + .query_all(Statement::from_sql_and_values( + DbBackend::MySql, + &sql, + params, + )) + .await?; + + let mut provisional = HashSet::with_capacity(rows.len()); + for row in rows { + let person_id: Vec = row.try_get("", "person_id")?; + provisional.insert(Uuid::from_slice(&person_id)?); + } + Ok(provisional) +} + pub async fn person_cards( db: &DatabaseConnection, tenant_id: Uuid, diff --git a/src/frontend/src/api/identity-client.ts b/src/frontend/src/api/identity-client.ts index a68912e19..6e58a6065 100644 --- a/src/frontend/src/api/identity-client.ts +++ b/src/frontend/src/api/identity-client.ts @@ -100,6 +100,10 @@ function isMeRole(role: unknown): role is MeRole { /** A person as operator surfaces display them — the wire `PersonSummaryResponse`. */ export interface PersonSummary { person_id: string; + /** The journal holds nothing but a login-mint for them: they exist so + * somebody could sign in, and may duplicate a person the roster knows. + * Never a merge target — the history is on the other side. */ + provisional?: boolean; email?: string | null; username?: string | null; display_name?: string | null; diff --git a/src/frontend/src/components/portal/person-cell.test.tsx b/src/frontend/src/components/portal/person-cell.test.tsx index 525ebab89..c63b075bc 100644 --- a/src/frontend/src/components/portal/person-cell.test.tsx +++ b/src/frontend/src/components/portal/person-cell.test.tsx @@ -70,6 +70,16 @@ describe("PersonCell", () => { ).toHaveLength(1); }); + // The picker is where the wrong person gets chosen, and a stub minted at a + // sign-in is the wrong side of a merge — its counterpart holds the history. + it("marks a person the journal knows only from a first sign-in", () => { + render( + , + ); + + expect(screen.getByText(/provisional/i)).toBeInTheDocument(); + }); + it("marks a terminated person", () => { render( {name} + {person.provisional ? ( + + {t("identities.person.provisional")} + + ) : null} {person.status?.trim().toLowerCase() === TERMINATED ? ( Date: Sat, 15 Aug 2026 06:47:14 +0300 Subject: [PATCH 14/19] identity: find an account, and say whose it is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both ways into the console are entered through a person: the queue arrives from a problem the resolver could not settle, the person mode from a name. An operator handed an account instead — a git login off a review, an address off a ticket — could not ask the only question they have. `GET /v1/resolution/accounts?q=` matches the same folded evidence the queue reads, filtered to what an account carries: address, handle, id, observed name. Each match answers with the holder, hydrated, and who decided that binding — undoing automation is routine, overruling a colleague is not. Bound to nobody is an answer of its own and says so rather than leaving a gap. Three characters minimum: a shorter needle scans the whole fold to answer with everything. Matches open in the same case window as everywhere else, so a correction is one press from the search that found it. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../backend/identity-resolution/openapi.json | 187 ++++++++++++++++++ .../identity-resolution/src/api/mod.rs | 26 +++ .../identity-resolution/src/api/resolution.rs | 119 ++++++++++- .../src/infra/identity_evidence.rs | 47 +++++ src/frontend/src/api/identity-client.ts | 38 ++++ .../portal/account-search-view.test.tsx | 117 +++++++++++ .../components/portal/account-search-view.tsx | 157 +++++++++++++++ .../portal/identities-view.test.tsx | 1 + .../src/components/portal/identities-view.tsx | 7 +- src/frontend/src/locales/en/translation.json | 10 +- src/frontend/src/mocks/handlers.ts | 29 +++ .../src/queries/identity-resolution.ts | 15 ++ tests/stand/api/operations.py | 2 + tests/stand/api/schemas/identity.py | 25 +++ 14 files changed, 776 insertions(+), 4 deletions(-) create mode 100644 src/frontend/src/components/portal/account-search-view.test.tsx create mode 100644 src/frontend/src/components/portal/account-search-view.tsx diff --git a/docs/components/backend/identity-resolution/openapi.json b/docs/components/backend/identity-resolution/openapi.json index 91b21c14e..026e2e6ab 100644 --- a/docs/components/backend/identity-resolution/openapi.json +++ b/docs/components/backend/identity-resolution/openapi.json @@ -44,6 +44,61 @@ ], "type": "object" }, + "AccountMatchResponse": { + "description": "One account as a search answers for it: what it is, and whose it is.", + "properties": { + "account_id": { + "type": "string" + }, + "bound_by_operator": { + "description": "`true` when a person decided this binding rather than automation.", + "type": "boolean" + }, + "display_name": { + "type": [ + "string", + "null" + ] + }, + "email": { + "type": [ + "string", + "null" + ] + }, + "person": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/PersonSummaryResponse", + "description": "The person holding it, hydrated. Absent means nobody holds it yet." + } + ] + }, + "source": { + "type": "string" + }, + "source_id": { + "format": "uuid", + "type": "string" + }, + "username": { + "type": [ + "string", + "null" + ] + } + }, + "required": [ + "source", + "source_id", + "account_id", + "bound_by_operator" + ], + "type": "object" + }, "AccountOperationResponse": { "description": "One operator call that named this account.\n\nThe binding journal says what a decision did; this says who ran it, the\nreason they gave, and how far it reached — a merge lands one row here and\none in every other account it moved.", "properties": { @@ -140,6 +195,25 @@ ], "type": "object" }, + "AccountSearchResponse": { + "properties": { + "items": { + "items": { + "$ref": "#/components/schemas/AccountMatchResponse" + }, + "type": "array" + }, + "truncated": { + "description": "More accounts matched than `limit` allowed — narrow the terms.", + "type": "boolean" + } + }, + "required": [ + "items", + "truncated" + ], + "type": "object" + }, "AttentionResponse": { "properties": { "items": { @@ -2566,6 +2640,119 @@ "summary": "Resolve a profile by email or source-native id" } }, + "/v1/resolution/accounts": { + "get": { + "operationId": "identity_resolution.resolution.search_accounts", + "parameters": [ + { + "description": "Needle matched against the account's current address, handle, id and observed name (at least 3 characters).", + "in": "query", + "name": "q", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Cap on returned matches (1..=100, default 20); `truncated` says whether it cut the list.", + "in": "query", + "name": "limit", + "required": false, + "schema": { + "type": "integer" + } + } + ], + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/AccountSearchResponse" + } + } + }, + "description": "Matching accounts with their holders" + }, + "400": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Bad Request" + }, + "401": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Unauthorized" + }, + "403": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Forbidden" + }, + "404": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Not Found" + }, + "409": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Conflict" + }, + "429": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Too Many Requests" + }, + "500": { + "content": { + "application/problem+json": { + "schema": { + "$ref": "#/components/schemas/Problem" + } + } + }, + "description": "Internal Server Error" + } + }, + "security": [ + { + "bearerAuth": [] + } + ], + "summary": "Find an observed account by a value it carries, and whose it is" + } + }, "/v1/resolution/accounts/{source}/{source_id}/{account_id}": { "get": { "operationId": "identity_resolution.resolution.account_binding", diff --git a/src/backend/services/identity-resolution/src/api/mod.rs b/src/backend/services/identity-resolution/src/api/mod.rs index b63ce4e57..dbb217a73 100644 --- a/src/backend/services/identity-resolution/src/api/mod.rs +++ b/src/backend/services/identity-resolution/src/api/mod.rs @@ -264,6 +264,32 @@ fn build_operations(router: Router, openapi: &dyn OpenApiRegistry) -> Router { .handler(resolution::attention) .register(router, openapi); + let router = OperationBuilder::get("/v1/resolution/accounts") + .operation_id("identity_resolution.resolution.search_accounts") + .summary("Find an observed account by a value it carries, and whose it is") + .authenticated() + .no_license_required() + .query_param_typed( + "q", + true, + "Needle matched against the account's current address, handle, id and observed name (at least 3 characters).", + "string", + ) + .query_param_typed( + "limit", + false, + "Cap on returned matches (1..=100, default 20); `truncated` says whether it cut the list.", + "integer", + ) + .json_response_with_schema::( + openapi, + StatusCode::OK, + "Matching accounts with their holders", + ) + .standard_errors(openapi) + .handler(resolution::search_accounts) + .register(router, openapi); + let router = OperationBuilder::get("/v1/resolution/accounts/{source}/{source_id}/{account_id}") .operation_id("identity_resolution.resolution.account_binding") .summary("Current binding of an account and every decision behind it") diff --git a/src/backend/services/identity-resolution/src/api/resolution.rs b/src/backend/services/identity-resolution/src/api/resolution.rs index 882a36fa7..051ad6408 100644 --- a/src/backend/services/identity-resolution/src/api/resolution.rs +++ b/src/backend/services/identity-resolution/src/api/resolution.rs @@ -23,12 +23,18 @@ use super::gate::require_admin; use crate::domain::person_card::{self, PersonCard}; use crate::domain::resolution::{self, EXCLUDED_PERSON, Target, Verb}; use crate::domain::review_queue::{self, EvidenceAccount, ItemKind, Review}; -use crate::domain::seed::SourceAccountKey; +use crate::domain::seed::{KnownBinding, SourceAccountKey}; use crate::infra::db::{ops_repo, persons_repo, resolution_repo}; use crate::infra::identity_evidence::{ AccountEvidence, ClickHouseEvidenceReader, EvidenceSnapshot, }; +/// A handle or an address, not a prefix of one: a one-character needle scans +/// the whole fold to answer with everything. +const MIN_ACCOUNT_SEARCH_CHARS: usize = 3; +const DEFAULT_ACCOUNT_SEARCH_LIMIT: u64 = 20; +const MAX_ACCOUNT_SEARCH_LIMIT: u64 = 100; + /// A trail is read, not paged: enough calls to cover any account's history /// without letting one response grow without bound. const MAX_ACCOUNT_OPERATIONS: u64 = 50; @@ -861,6 +867,117 @@ pub struct AccountBindingResponse { } impl toolkit::api::api_dto::ResponseApiDto for AccountBindingResponse {} +/// One account as a search answers for it: what it is, and whose it is. +#[derive(Debug, Serialize, ToSchema)] +pub struct AccountMatchResponse { + pub source: String, + pub source_id: Uuid, + pub account_id: String, + pub email: Option, + pub username: Option, + pub display_name: Option, + /// The person holding it, hydrated. Absent means nobody holds it yet. + pub person: Option, + /// `true` when a person decided this binding rather than automation. + pub bound_by_operator: bool, +} + +#[derive(Debug, Serialize, ToSchema)] +pub struct AccountSearchResponse { + pub items: Vec, + /// More accounts matched than `limit` allowed — narrow the terms. + pub truncated: bool, +} +impl toolkit::api::api_dto::ResponseApiDto for AccountSearchResponse {} + +#[derive(Debug, Deserialize)] +pub struct AccountSearchParams { + pub q: Option, + pub limit: Option, +} + +/// `GET /v1/resolution/accounts?q=` — find an account by a value it carries, +/// and say whose it is. +/// +/// The person search answers "which person is this"; this answers the question +/// an operator arrives with when they hold an account instead: a git handle +/// from a review, an address from a ticket. +pub async fn search_accounts( + Extension(state): Extension>, + Extension(ctx): Extension, + axum::extract::Query(params): axum::extract::Query, +) -> Result { + require_admin(&state.db, &ctx).await?; + let tenant = ctx.subject_tenant_id(); + + let needle = params.q.unwrap_or_default().trim().to_owned(); + if needle.chars().count() < MIN_ACCOUNT_SEARCH_CHARS { + return Err(invalid( + "q", + &format!("at least {MIN_ACCOUNT_SEARCH_CHARS} characters are required"), + )); + } + let limit = super::listing::clamp_limit( + params.limit, + DEFAULT_ACCOUNT_SEARCH_LIMIT, + MAX_ACCOUNT_SEARCH_LIMIT, + ); + + let reader = evidence_reader(&state); + // Over-fetch by one: the extra row is the truncation probe, never served. + let mut found = reader + .search(&needle, limit + 1) + .await + .map_err(|e| internal(&e, "failed to search the observed accounts"))?; + let truncated = found.len() > usize::try_from(limit).unwrap_or(usize::MAX); + if truncated { + found.pop(); + } + + let keys: Vec = found.iter().map(|e| e.account.clone()).collect(); + let bindings = resolution_repo::current_bindings(&state.db, tenant, &keys) + .await + .map_err(|e| internal(&e, "failed to read current bindings"))?; + + let person_ids: Vec = bindings + .values() + .map(|b| b.person_id) + .filter(|id| *id != EXCLUDED_PERSON) + .collect::>() + .into_iter() + .collect(); + let cards = persons_repo::person_cards(&state.db, tenant, &person_ids) + .await + .map_err(|e| internal(&e, "failed to hydrate the holders"))?; + let provisional = persons_repo::provisional_persons(&state.db, tenant, &person_ids) + .await + .map_err(|e| internal(&e, "failed to read which persons are provisional"))?; + + let items = found + .into_iter() + .map(|account| { + let binding = bindings.get(&account.account); + AccountMatchResponse { + source: account.account.source_type, + source_id: account.account.source_id, + account_id: account.account.account_id, + email: account.email, + username: account.username, + display_name: account.description.display_name, + person: binding + .and_then(|b| cards.get(&b.person_id).cloned()) + .map(|card| PersonSummaryResponse { + provisional: provisional.contains(&card.person_id), + ..PersonSummaryResponse::from(card) + }), + bound_by_operator: binding.is_some_and(KnownBinding::is_operator_authored), + } + }) + .collect(); + + Ok(Json(AccountSearchResponse { items, truncated })) +} + /// `GET /v1/resolution/accounts/{source}/{source_id}/{account_id}` — why this /// account belongs to this person: the binding in force and every decision /// behind it. diff --git a/src/backend/services/identity-resolution/src/infra/identity_evidence.rs b/src/backend/services/identity-resolution/src/infra/identity_evidence.rs index c3864da13..7ef599be7 100644 --- a/src/backend/services/identity-resolution/src/infra/identity_evidence.rs +++ b/src/backend/services/identity-resolution/src/infra/identity_evidence.rs @@ -97,6 +97,22 @@ const FOLD_SQL: &str = r" ORDER BY source_type, source_id, account_id "; +/// Accounts whose current values contain the needle, for the operator who has +/// an account in hand and no idea whose it is. The fold is the same one the +/// queue reads; only the filter and the ceiling differ. +const SEARCH_SQL: &str = r" + SELECT source_type, source_id, account_id, latest_op, email, username, + display_name, first_name, last_name, job_title, department, status, + manager_email + FROM ({FOLD}) + WHERE latest_op != 'DELETE' + AND (positionCaseInsensitive(email, ?) > 0 + OR positionCaseInsensitive(username, ?) > 0 + OR positionCaseInsensitive(account_id, ?) > 0 + OR positionCaseInsensitive(display_name, ?) > 0) + LIMIT ? +"; + /// Ceiling on the fold, which is read whole into memory. The queue's rates are /// meant to cover every observed account, so this is a safety valve against an /// unbounded read rather than a pagination knob: reaching it truncates what the @@ -297,6 +313,37 @@ fn compose_name(first: Option, last: Option) -> Option { } impl ClickHouseEvidenceReader { + /// Accounts whose current values contain `needle`, newest-agnostic. + /// + /// # Errors + /// + /// Returns an error if the query fails or a stored source id is not a UUID. + pub async fn search(&self, needle: &str, limit: u64) -> anyhow::Result> { + let sql = SEARCH_SQL.replace("{FOLD}", FOLD_SQL); + let rows: Vec = match self + .client + .query(&sql) + .bind(needle) + .bind(needle) + .bind(needle) + .bind(needle) + .bind(limit) + .fetch_all() + .await + { + Ok(rows) => rows, + Err(e) if is_missing_relation(&e) => return Ok(Vec::new()), + Err(e) => return Err(e.into()), + }; + + // A row with an unreadable source id is one unusable account, not a + // reason to answer nothing — the same rule the full fold applies. + Ok(rows + .into_iter() + .filter_map(|row| map_row(row).ok()) + .collect()) + } + /// Whether the evidence knows this account. /// /// # Errors diff --git a/src/frontend/src/api/identity-client.ts b/src/frontend/src/api/identity-client.ts index 6e58a6065..af5cba77c 100644 --- a/src/frontend/src/api/identity-client.ts +++ b/src/frontend/src/api/identity-client.ts @@ -178,6 +178,44 @@ export async function getAttention(limit = 200): Promise { return attention; } +/** One account a search matched, with whose it is. */ +export interface AccountMatch { + source: string; + source_id: string; + account_id: string; + email?: string | null; + username?: string | null; + display_name?: string | null; + /** The person holding it; absent = nobody holds it yet. */ + person?: PersonSummary | null; + bound_by_operator: boolean; +} + +export interface AccountSearchResponse { + items: AccountMatch[]; + /** More matched than the limit allowed — narrow the terms. */ + truncated: boolean; +} + +/** + * Find an observed account by a value it carries (`GET /resolution/accounts`). + * The person search answers "which person is this"; this answers the question + * an operator arrives with when they hold an account instead. + */ +export async function searchAccounts( + q: string, + limit = 20, +): Promise { + const res = await fetchWithAuth( + `${BASE}/resolution/accounts?q=${encodeURIComponent(q)}&limit=${limit}`, + ); + if (!res.ok) { + const body = await res.json().catch(() => null); + throw new IdentityApiError(res.status, body); + } + return (await res.json()) as AccountSearchResponse; +} + /** One decision recorded for an account, newest first on the wire. */ export interface BindingHistoryEntry { person_id: string; diff --git a/src/frontend/src/components/portal/account-search-view.test.tsx b/src/frontend/src/components/portal/account-search-view.test.tsx new file mode 100644 index 000000000..99af86a17 --- /dev/null +++ b/src/frontend/src/components/portal/account-search-view.test.tsx @@ -0,0 +1,117 @@ +// @vitest-environment jsdom +/** + * The account mode. What matters: an operator holding a handle or an address + * learns whose it is — the question neither other mode can answer, since both + * are entered through a person; unbound is stated as an answer rather than + * left blank; and the account opens in the same case window, so the verbs are + * one click from the search. + */ +import { render, screen, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import "@/i18n"; +import type { AccountMatch } from "@/api/identity-client"; + +vi.mock("@tanstack/react-router", async () => { + const { portalRouterMock } = await import("@/test/portal-router"); + return portalRouterMock(); +}); + +const hooks = vi.hoisted(() => ({ + search: { + data: undefined as { items: AccountMatch[]; truncated: boolean } | undefined, + isFetching: false, + isError: false, + }, +})); +vi.mock("@/queries/identity-resolution", () => ({ + useAccountSearch: () => hooks.search, + useAccountBinding: () => ({ + data: undefined, + isLoading: true, + isError: false, + error: null, + refetch: vi.fn(), + }), +})); + +import { portalRouter } from "@/test/portal-router"; + +import { AccountSearchView } from "./account-search-view"; + +function match(over: Partial = {}): AccountMatch { + return { + source: "github", + source_id: "01900000-0000-7000-8000-00000000aa01", + account_id: "gh-main", + email: null, + username: "octocat", + display_name: null, + person: { + person_id: "01900000-0000-7000-8000-0000000000a0", + display_name: "Ann Lee", + }, + bound_by_operator: false, + ...over, + }; +} + +beforeEach(() => { + hooks.search.data = undefined; + hooks.search.isFetching = false; + hooks.search.isError = false; + portalRouter.reset(); + portalRouter.set({ zone: "manage", item: "identities", mode: "accounts" }); +}); + +describe("AccountSearchView", () => { + it("answers whose an account is", async () => { + hooks.search.data = { items: [match()], truncated: false }; + render(); + + await userEvent.type(screen.getByRole("searchbox"), "octocat"); + + expect(screen.getByText("octocat")).toBeInTheDocument(); + expect(screen.getByText(/github · gh-main/)).toBeInTheDocument(); + expect(screen.getByText("Ann Lee")).toBeInTheDocument(); + }); + + // Nobody holding it is an answer, and a different one from "nobody has + // decided yet" — leaving the column blank would read as neither. + it("states an account bound to nobody rather than leaving a gap", () => { + hooks.search.data = { items: [match({ person: null })], truncated: false }; + render(); + + expect(screen.getByText(/bound to nobody/i)).toBeInTheDocument(); + }); + + it("says who decided each binding", () => { + hooks.search.data = { + items: [match({ bound_by_operator: true })], + truncated: false, + }; + render(); + + expect(screen.getByText(/decided by an operator/i)).toBeInTheDocument(); + }); + + it("opens a match in the same case window", async () => { + hooks.search.data = { items: [match()], truncated: false }; + render(); + + await userEvent.click(screen.getByRole("button", { name: /^open$/i })); + + expect(portalRouter.search.acct).toContain("gh-main"); + expect( + within(screen.getByRole("dialog")).getByText(/github · gh-main/), + ).toBeInTheDocument(); + }); + + it("says a cut list was cut", () => { + hooks.search.data = { items: [match()], truncated: true }; + render(); + + expect(screen.getByText(/narrow the terms/i)).toBeInTheDocument(); + }); +}); diff --git a/src/frontend/src/components/portal/account-search-view.tsx b/src/frontend/src/components/portal/account-search-view.tsx new file mode 100644 index 000000000..35ed738dd --- /dev/null +++ b/src/frontend/src/components/portal/account-search-view.tsx @@ -0,0 +1,157 @@ +/** + * The console's third mode: an account in hand, and whose it is. + * + * The queue arrives from a problem and the person mode from a name. This one + * answers the question an operator gets handed instead — a git login from a + * review, an address from a ticket — which neither of the others can, because + * both are entered through a person. + */ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { Search } from "lucide-react"; + +import type { AccountMatch } from "@/api/identity-client"; +import { CaseDialog } from "@/components/portal/case-dialog"; +import { PersonCell } from "@/components/portal/person-cell"; +import { Badge } from "@/components/ui/badge"; +import { Button } from "@/components/ui/button"; +import { Card, CardContent } from "@/components/ui/card"; +import { Input } from "@/components/ui/input"; +import { Spinner } from "@/components/ui/spinner"; +import { ComingSoon } from "@/components/widgets/coming-soon"; +import { useDebouncedValue } from "@/hooks/use-debounced-value"; +import { accountKey } from "@/lib/identities/account-key"; +import { usePortalNavActions } from "@/lib/portal/portal-nav"; +import { usePortalSearch } from "@/lib/portal/portal-search"; +import { useAccountSearch } from "@/queries/identity-resolution"; +import { cn } from "@/lib/utils"; + +const DEBOUNCE_MS = 250; +/** The service's own floor: a shorter needle scans the fold to answer with everything. */ +const MIN_QUERY_CHARS = 3; + +export function AccountSearchView() { + const { t } = useTranslation(); + const { acct } = usePortalSearch(); + const { setAcct } = usePortalNavActions(); + const [query, setQuery] = useState(""); + const debounced = useDebouncedValue(query, DEBOUNCE_MS); + const search = useAccountSearch(debounced); + + const items = search.data?.items ?? []; + const ordered = items.map((item) => accountKey(item)); + const asked = debounced.trim().length >= MIN_QUERY_CHARS; + + return ( +
    +
    + + setQuery(event.target.value)} + placeholder={t("identities.accounts.placeholder")} + aria-label={t("identities.accounts.placeholder")} + className="ps-9" + /> + {search.isFetching ? ( + + ) : null} +
    + + {search.isError ? ( + + ) : null} + + {asked && !search.isFetching && items.length === 0 && !search.isError ? ( +

    + {t("identities.accounts.no_matches")} +

    + ) : null} + + {items.length > 0 ? ( + + + {items.map((item) => ( + setAcct(accountKey(item))} + /> + ))} + {search.data?.truncated ? ( +

    + {t("identities.accounts.truncated")} +

    + ) : null} +     +     + ) : null} + + setAcct(null)} + /> +
    + ); +} + +function AccountRow({ + item, + selected, + onOpen, +}: { + item: AccountMatch; + selected: boolean; + onOpen: () => void; +}) { + const { t } = useTranslation(); + const label = + item.email?.trim() || + item.username?.trim() || + item.display_name?.trim() || + item.account_id; + return ( +
    +
    +
    {label}
    +
    + {item.source} · {item.account_id} +
    +
    + {/* Whose it is — the answer the mode exists for. Unbound is an answer + too, and a different one from "nobody has decided yet". */} + {item.person ? ( + + ) : ( + + {t("identities.accounts.unbound")} + + )} + + {item.bound_by_operator + ? t("identities.people.by_operator") + : t("identities.people.by_automation")} + + +
    + ); +} diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index 2872f323a..dd2c26ae6 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -30,6 +30,7 @@ const attention = vi.hoisted(() => ({ })); vi.mock("@/queries/identity-resolution", () => ({ useAttention: () => attention.q, + useAccountSearch: () => ({ data: undefined, isFetching: false, isError: false }), // The people mode has its own test file; here it only has to mount. usePersonSearch: () => ({ data: undefined, isFetching: false, isError: false }), usePersonAccounts: () => ({ diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 618bfd887..7176f1394 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -19,6 +19,7 @@ import { useTranslation } from "react-i18next"; import type { AttentionItem, ResolutionRates } from "@/api/identity-client"; import { Alert, AlertDescription } from "@/components/ui/alert"; import { CenteredSpinner } from "@/components/widgets/centered-spinner"; +import { AccountSearchView } from "@/components/portal/account-search-view"; import { CaseDialog } from "@/components/portal/case-dialog"; import { PersonAccountsView } from "@/components/portal/person-accounts-view"; import { PersonCell } from "@/components/portal/person-cell"; @@ -105,7 +106,7 @@ function opensTheCase(event: React.MouseEvent): boolean { * the queue arrives at them from a problem, the person view from a name — so * adding one is an entry here and a component, nothing else. */ -const MODES = ["queue", "people"] as const; +const MODES = ["queue", "people", "accounts"] as const; const DEFAULT_MODE = MODES[0]; export function IdentitiesView() { @@ -141,7 +142,9 @@ export function IdentitiesView() { ))} - {active === "people" ? : } + {active === "people" ? : null} + {active === "accounts" ? : null} + {active === "queue" ? : null}
    ); } diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index c16ec9fdc..8460eab97 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -529,7 +529,8 @@ }, "modes": { "queue": "Review queue", - "people": "A person and their accounts" + "people": "A person and their accounts", + "accounts": "An account and whose it is" }, "people": { "no_person": "Nobody chosen yet", @@ -540,6 +541,13 @@ "by_automation": "bound automatically", "open": "Open", "load_failed": "Unable to load this person’s accounts" + }, + "accounts": { + "placeholder": "Find an account by address, handle, id or observed name…", + "no_matches": "No observed account carries that.", + "truncated": "More accounts matched than shown — narrow the terms.", + "unbound": "bound to nobody", + "failed": "Account search failed. Try again." } } } diff --git a/src/frontend/src/mocks/handlers.ts b/src/frontend/src/mocks/handlers.ts index 618ffd5ce..308a06745 100644 --- a/src/frontend/src/mocks/handlers.ts +++ b/src/frontend/src/mocks/handlers.ts @@ -504,6 +504,35 @@ export const handlers = [ })); return HttpResponse.json({ items, truncated: false, next_cursor: null }); }), + // Account search: the same roster, matched by what an account carries. + http.get("/api/identity/v1/resolution/accounts", ({ request }) => { + const q = new URL(request.url).searchParams.get("q")?.trim() ?? ""; + if (q.length < 3) { + return HttpResponse.json( + { type: "urn:insight:error:invalid_argument" }, + { status: 400 }, + ); + } + const needle = q.toLowerCase(); + const items = PEOPLE.filter((p) => + [p.name, p.email].some((v) => v.toLowerCase().includes(needle)), + ).map((p, index) => ({ + source: index % 2 === 0 ? "github" : "gitlab", + source_id: "01900000-0000-7000-8000-00000000aa01", + account_id: `acct-${index + 1}`, + email: p.email, + username: null, + display_name: p.name, + person: { + person_id: p.person_id, + email: p.email, + display_name: p.name, + job_title: p.role, + }, + bound_by_operator: index % 3 === 0, + })); + return HttpResponse.json({ items, truncated: false }); + }), // A merge preview's substance: two synthetic accounts for anyone. http.get( "/api/identity/v1/resolution/persons/:personId/accounts", diff --git a/src/frontend/src/queries/identity-resolution.ts b/src/frontend/src/queries/identity-resolution.ts index 44da710a0..a7eea0b1a 100644 --- a/src/frontend/src/queries/identity-resolution.ts +++ b/src/frontend/src/queries/identity-resolution.ts @@ -21,8 +21,10 @@ import { getAttention, getPersonAccounts, mergePersons, + searchAccounts, searchPersons, type AccountBinding, + type AccountSearchResponse, type AttentionResponse, type CorrectionResponse, type PersonAccountEntry, @@ -126,6 +128,19 @@ export function usePersonSearch(q: string): UseQueryResult }); } +/** Live account search for the account mode; the component debounces. */ +export function useAccountSearch(q: string): UseQueryResult { + const { session } = useAuth(); + const sessionScope = sessionAuthorizationScope(session); + const trimmed = q.trim(); + return useQuery({ + queryKey: [...RESOLUTION_KEY, "account-search", sessionScope, trimmed], + queryFn: () => searchAccounts(trimmed), + staleTime: ATTENTION_STALE_TIME, + enabled: sessionScope != null && trimmed.length >= 3, + }); +} + /** The accounts a merge would move — fetched only while the preview is open. */ export function usePersonAccounts( personId: string | null, diff --git a/tests/stand/api/operations.py b/tests/stand/api/operations.py index 70f1d3ec3..f2bd0e67f 100644 --- a/tests/stand/api/operations.py +++ b/tests/stand/api/operations.py @@ -132,6 +132,7 @@ def _i(method: str, suffix: str) -> Operation: # the accounts read: it is a connector type, not an id, and the tests # address the same literal — a stand-in would fold a segment nothing varies. _i("GET", "/v1/resolution/attention"), + _i("GET", "/v1/resolution/accounts"), _i("GET", f"/v1/resolution/accounts/github/{SOME_ID}/{SOME_ACCOUNT_ID}"), _i("GET", f"/v1/resolution/persons/{SOME_ID}/accounts"), _i("POST", "/v1/resolution/bind"), @@ -166,6 +167,7 @@ def _i(method: str, suffix: str) -> Operation: _ADMIN_GATED_SUFFIXES: Final[tuple[str, ...]] = ( "/v1/persons", "/v1/resolution/attention", + "/v1/resolution/accounts", f"/v1/resolution/accounts/github/{SOME_ID}/{SOME_ACCOUNT_ID}", f"/v1/resolution/persons/{SOME_ID}/accounts", "/v1/resolution/bind", diff --git a/tests/stand/api/schemas/identity.py b/tests/stand/api/schemas/identity.py index 2366b2772..15dd03fcd 100644 --- a/tests/stand/api/schemas/identity.py +++ b/tests/stand/api/schemas/identity.py @@ -464,6 +464,23 @@ class VisiblePersonsResponse(BaseModel): visible: list[UUID] +class AccountMatchResponse(BaseModel): + """ + One account as a search answers for it: what it is, and whose it is. + """ + model_config = ConfigDict( + extra='forbid', + ) + account_id: str + bound_by_operator: bool = Field(..., description='`true` when a person decided this binding rather than automation.') + display_name: str | None = None + email: str | None = None + person: PersonSummaryResponse | None = None + source: str + source_id: UUID + username: str | None = None + + class AccountOperationResponse(BaseModel): """ One operator call that named this account. @@ -485,6 +502,14 @@ class AccountOperationResponse(BaseModel): verb: str = Field(..., description='`operator-bind` | `operator-merge` | `operator-detach` | `operator-exclude`.') +class AccountSearchResponse(BaseModel): + model_config = ConfigDict( + extra='forbid', + ) + items: list[AccountMatchResponse] + truncated: bool = Field(..., description='More accounts matched than `limit` allowed — narrow the terms.') + + class AttentionResponse(BaseModel): model_config = ConfigDict( extra='forbid', From 1aca9fdc0b6ce6b4aebbe75311aa29c0601f87cb Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 06:51:08 +0300 Subject: [PATCH 15/19] frontend: say what the account mode is for before anything is searched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It opened to a search box over blank space, which reads as a surface that failed to load rather than one waiting for a question. It now says what can be searched — an address, a handle, an id, the name a source gives the account — and that three characters is the floor, matching the mode beside it. A search that matches nothing gets the same treatment, plus the fact that frames the result: only accounts a connector has actually seen can be found here, so "nothing" is an answer about the evidence, not about the search. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../portal/account-search-view.test.tsx | 9 +++++ .../components/portal/account-search-view.tsx | 36 ++++++++++++++++--- src/frontend/src/locales/en/translation.json | 7 ++-- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/frontend/src/components/portal/account-search-view.test.tsx b/src/frontend/src/components/portal/account-search-view.test.tsx index 99af86a17..161b95aa4 100644 --- a/src/frontend/src/components/portal/account-search-view.test.tsx +++ b/src/frontend/src/components/portal/account-search-view.test.tsx @@ -66,6 +66,15 @@ beforeEach(() => { }); describe("AccountSearchView", () => { + // An empty surface reads as one that failed to load; the mode says what it + // is for instead. + it("says what to search for before anything is asked", () => { + render(); + + expect(screen.getByText(/nothing searched yet/i)).toBeInTheDocument(); + expect(screen.getByText(/at least three characters/i)).toBeInTheDocument(); + }); + it("answers whose an account is", async () => { hooks.search.data = { items: [match()], truncated: false }; render(); diff --git a/src/frontend/src/components/portal/account-search-view.tsx b/src/frontend/src/components/portal/account-search-view.tsx index 35ed738dd..825fba743 100644 --- a/src/frontend/src/components/portal/account-search-view.tsx +++ b/src/frontend/src/components/portal/account-search-view.tsx @@ -8,7 +8,7 @@ */ import { useState } from "react"; import { useTranslation } from "react-i18next"; -import { Search } from "lucide-react"; +import { ScanSearch, Search } from "lucide-react"; import type { AccountMatch } from "@/api/identity-client"; import { CaseDialog } from "@/components/portal/case-dialog"; @@ -16,6 +16,13 @@ import { PersonCell } from "@/components/portal/person-cell"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; import { Card, CardContent } from "@/components/ui/card"; +import { + Empty, + EmptyDescription, + EmptyHeader, + EmptyMedia, + EmptyTitle, +} from "@/components/ui/empty"; import { Input } from "@/components/ui/input"; import { Spinner } from "@/components/ui/spinner"; import { ComingSoon } from "@/components/widgets/coming-soon"; @@ -67,10 +74,31 @@ export function AccountSearchView() { /> ) : null} + {/* Before anything is asked the mode has nothing to show, and a bare + gap reads as a surface that failed to load. */} + {!asked && !search.isError ? ( + + + + + + {t("identities.accounts.no_query")} + + {t("identities.accounts.no_query_description")} + + + + ) : null} + {asked && !search.isFetching && items.length === 0 && !search.isError ? ( -

    - {t("identities.accounts.no_matches")} -

    + + + {t("identities.accounts.no_matches")} + + {t("identities.accounts.no_matches_description")} + + + ) : null} {items.length > 0 ? ( diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index 8460eab97..99f88f7ac 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -544,10 +544,13 @@ }, "accounts": { "placeholder": "Find an account by address, handle, id or observed name…", - "no_matches": "No observed account carries that.", + "no_matches": "No observed account carries that", "truncated": "More accounts matched than shown — narrow the terms.", "unbound": "bound to nobody", - "failed": "Account search failed. Try again." + "failed": "Account search failed. Try again.", + "no_query": "Nothing searched yet", + "no_query_description": "Find an account by any value it carries — an address, a handle, its id, or the name its source gives it. At least three characters.", + "no_matches_description": "Only accounts a connector has seen can be found here." } } } From f668c568f2204f28994299a09e41183097343366 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 07:43:38 +0300 Subject: [PATCH 16/19] identity: fix what the review caught in the new reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects, all found by adversarial review of the branch: The provisional flag was computed over the hydrated cards' keys — and a login-minted person has no card attributes at all, their journal holds only the binding row. So the one candidate the flag was built to mark, the stub on every provisioned-at-first-sign-in queue item, was the one person it was never true for. The set is now computed over the candidate ids themselves. The id+value person search intersected the named ids with a tenant-wide value search that is truncated to a prefix by person id. A person who genuinely matched both could sort past that prefix and vanish from the answer with truncated=false — "this person does not match", the exact wrong reading on a disambiguation surface. The value filter now runs WITHIN the named ids. The id-named path also ignored the limit and returned persons in database order, so the truncation probe popped a nondeterministic id. Sorted and capped before the probe. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../identity-resolution/src/api/persons.rs | 22 ++++++++++--------- .../identity-resolution/src/api/resolution.rs | 20 +++++++++++------ .../src/infra/db/persons_repo.rs | 11 ++++++++++ 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/backend/services/identity-resolution/src/api/persons.rs b/src/backend/services/identity-resolution/src/api/persons.rs index 2243540c2..f4fcca4ab 100644 --- a/src/backend/services/identity-resolution/src/api/persons.rs +++ b/src/backend/services/identity-resolution/src/api/persons.rs @@ -78,7 +78,7 @@ pub async fn search_persons( // Over-fetch by one: the extra row is the truncation probe, never served. let mut ids = if named.is_empty() { - persons_repo::search_persons_by_current_values(&state.db, tenant, &values, limit + 1) + persons_repo::search_persons_by_current_values(&state.db, tenant, &values, &[], limit + 1) .await .map_err(|e| read_err(&e))? } else { @@ -131,6 +131,11 @@ fn partition_terms(terms: &[String]) -> (Vec, Vec) { } /// Persons named by id, narrowed by any value terms alongside them. +/// +/// The value filter runs WITHIN the named ids — intersecting with a tenant-wide +/// value search would test membership in an independently truncated prefix and +/// silently drop a genuine match. Sorted and capped so the caller's truncation +/// probe drops a deterministic id, never whichever the database returned last. async fn persons_named_by_id( state: &AppState, tenant: Uuid, @@ -138,21 +143,18 @@ async fn persons_named_by_id( values: &[String], limit: u64, ) -> Result, CanonicalError> { - let known = persons_repo::persons_in_tenant(&state.db, tenant, named) + let mut known = persons_repo::persons_in_tenant(&state.db, tenant, named) .await .map_err(|e| read_err(&e))?; - if values.is_empty() { + known.sort_unstable(); + known.truncate(usize::try_from(limit).unwrap_or(usize::MAX)); + if values.is_empty() || known.is_empty() { return Ok(known); } - let by_value = persons_repo::search_persons_by_current_values(&state.db, tenant, values, limit) + persons_repo::search_persons_by_current_values(&state.db, tenant, values, &known, limit) .await - .map_err(|e| read_err(&e))?; - - Ok(known - .into_iter() - .filter(|id| by_value.contains(id)) - .collect()) + .map_err(|e| read_err(&e)) } /// Split `q` into terms: non-empty, whitespace-separated, capped in count and diff --git a/src/backend/services/identity-resolution/src/api/resolution.rs b/src/backend/services/identity-resolution/src/api/resolution.rs index 051ad6408..dd0ddb17c 100644 --- a/src/backend/services/identity-resolution/src/api/resolution.rs +++ b/src/backend/services/identity-resolution/src/api/resolution.rs @@ -764,13 +764,19 @@ pub async fn attention( let page: Vec<_> = review.items.into_iter().take(limit).collect(); let cards = candidate_cards(state.as_ref(), tenant, &page).await?; - let provisional = persons_repo::provisional_persons( - &state.db, - tenant, - &cards.keys().copied().collect::>(), - ) - .await - .map_err(|e| internal(&e, "failed to read which persons are provisional"))?; + // Over the candidate ids, NOT the hydrated cards' keys: a login-minted + // person has no card attributes at all (their journal holds only the + // binding row), so the card set misses exactly the persons this flag + // exists to mark. + let candidate_ids: Vec = page + .iter() + .flat_map(|i| i.candidates.iter().copied()) + .collect::>() + .into_iter() + .collect(); + let provisional = persons_repo::provisional_persons(&state.db, tenant, &candidate_ids) + .await + .map_err(|e| internal(&e, "failed to read which persons are provisional"))?; let items = page .into_iter() diff --git a/src/backend/services/identity-resolution/src/infra/db/persons_repo.rs b/src/backend/services/identity-resolution/src/infra/db/persons_repo.rs index e5025711d..d5b54fa4b 100644 --- a/src/backend/services/identity-resolution/src/infra/db/persons_repo.rs +++ b/src/backend/services/identity-resolution/src/infra/db/persons_repo.rs @@ -347,6 +347,10 @@ pub async fn search_persons_by_current_values( db: &DatabaseConnection, tenant_id: Uuid, terms: &[String], + // Restrict matching to these persons; empty = the whole tenant. The id+value + // search needs the value filter applied WITHIN the named ids — intersecting + // with an independently truncated tenant-wide result drops genuine matches. + within: &[Uuid], limit: u64, ) -> anyhow::Result> { if terms.is_empty() { @@ -381,6 +385,12 @@ pub async fn search_persons_by_current_values( WHERE cv.person_id != ? ", ); + if !within.is_empty() { + let placeholders = vec!["?"; within.len()].join(", "); + sql.push_str(" AND cv.person_id IN ("); + sql.push_str(&placeholders); + sql.push(')'); + } for _ in terms { sql.push_str( " AND EXISTS (SELECT 1 FROM current_vals t \ @@ -393,6 +403,7 @@ pub async fn search_persons_by_current_values( tenant_id.as_bytes().to_vec().into(), EXCLUDED_PERSON.as_bytes().to_vec().into(), ]; + values.extend(within.iter().map(|id| id.as_bytes().to_vec().into())); values.extend(terms.iter().map(|t| like_pattern(t).into())); values.push(limit.into()); From d4e2821ddd41eb4dc49f57c80b77526adf7cf816 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 07:50:17 +0300 Subject: [PATCH 17/19] frontend: hold the open case while the list moves under it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings with one root: the queue prunes a decided row at once (the point of the optimistic prune), and the case window was fed straight from that list — so the operator's own success yanked the case out from under them. The outcome alert unmounted unread, unbound cases flashed "the link may be stale", and the Next button vanished at the exact moment a conveyor needs it. Each piece was tested in isolation; the breakage lived between them. The window now holds what it knew about the open case — the row and its position. A pruned row keeps its heading, candidates and outcome; the held index points at whatever shifted into the slot, which is precisely where Next should go. The second root: "this account is unknown — the link may be stale" fired on `queueItem == null`, and the search and person modes can never supply a queue item — so an unbound account those modes just FOUND opened with no verbs, against the account-mode's central use case (binding an unplaced account). Both modes now hand the window their rows as cases: proof the account exists, a proper heading instead of a raw id, and the holder rendered as a card. The stale-link guard still fires where it should — on a mistyped link no caller vouches for. Also from the review: the filter box desynced from the URL on Back (input kept "bob" over an unfiltered list); closing the window after deciding the last- focused row dropped keyboard focus to nowhere — it now falls to the top of the list; and the dialog's `initialFocus={false}` turned out to move focus NOT AT ALL, stranding it in the aria-hidden page behind the dialog — the window itself takes focus now, not its copy button, not the background. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../components/portal/account-detail.test.tsx | 12 ++++ .../src/components/portal/account-detail.tsx | 7 ++ .../portal/account-search-view.test.tsx | 68 ++++++++++++++++--- .../components/portal/account-search-view.tsx | 17 ++++- .../src/components/portal/case-dialog.tsx | 68 ++++++++++++++++--- .../portal/identities-view.test.tsx | 31 +++++++++ .../src/components/portal/identities-view.tsx | 24 ++++++- .../portal/person-accounts-view.tsx | 20 +++++- 8 files changed, 222 insertions(+), 25 deletions(-) diff --git a/src/frontend/src/components/portal/account-detail.test.tsx b/src/frontend/src/components/portal/account-detail.test.tsx index 3e7631718..c6b9ef808 100644 --- a/src/frontend/src/components/portal/account-detail.test.tsx +++ b/src/frontend/src/components/portal/account-detail.test.tsx @@ -233,6 +233,18 @@ describe("AccountDetail", () => { expect(screen.getByText("Carol Chen")).toBeInTheDocument(); }); + // The voucher: a caller who can prove the account is observed (a search + // hit, a person's own account list) gets the verbs even with an empty + // journal — the stale-link guard exists for mistyped links, not for the + // accounts an operator just found. + it("offers the verbs on an empty journal when the caller vouches the account exists", () => { + binding.q.data = bound({ person_id: null, history: [] }); + render(); + + expect(screen.queryByText(/link may be stale/i)).not.toBeInTheDocument(); + expect(screen.getByTestId("account-actions")).toBeInTheDocument(); + }); + it("reads an off-queue empty journal as a stale link, offering no verbs", () => { binding.q.data = bound({ person_id: null, history: [] }); render(); diff --git a/src/frontend/src/components/portal/account-detail.tsx b/src/frontend/src/components/portal/account-detail.tsx index a4ed5c4ec..787513d8d 100644 --- a/src/frontend/src/components/portal/account-detail.tsx +++ b/src/frontend/src/components/portal/account-detail.tsx @@ -45,11 +45,17 @@ const VERB_KEYS: Record = { export function AccountDetail({ accountRef, queueItem, + observed = false, }: { accountRef: AccountRef; /** The queue row for this account, when it is still in the queue — the * source of hydrated candidate cards and observed evidence. */ queueItem: AttentionItem | undefined; + /** The caller vouches the account exists (a queue row, a search hit, a + * person's account list). Without a voucher, an account with no binding + * and no history reads as a stale link — offering verbs there would let a + * mistyped `?acct=` pre-register a typo as a real account. */ + observed?: boolean; }) { const { t } = useTranslation(); const binding = useAccountBinding(accountRef); @@ -68,6 +74,7 @@ export function AccountDetail({ if (!binding.data) return null; const neverSeen = + !observed && queueItem == null && binding.data.person_id == null && binding.data.history.length === 0; diff --git a/src/frontend/src/components/portal/account-search-view.test.tsx b/src/frontend/src/components/portal/account-search-view.test.tsx index 161b95aa4..4533fcc9e 100644 --- a/src/frontend/src/components/portal/account-search-view.test.tsx +++ b/src/frontend/src/components/portal/account-search-view.test.tsx @@ -18,22 +18,44 @@ vi.mock("@tanstack/react-router", async () => { return portalRouterMock(); }); -const hooks = vi.hoisted(() => ({ - search: { - data: undefined as { items: AccountMatch[]; truncated: boolean } | undefined, - isFetching: false, +const hooks = vi.hoisted(() => { + const verb = () => ({ + mutate: vi.fn(), + reset: vi.fn(), + isPending: false, isError: false, - }, -})); + error: null as unknown, + }); + return { + search: { + data: undefined as { items: AccountMatch[]; truncated: boolean } | undefined, + isFetching: false, + isError: false, + }, + binding: { + data: undefined as unknown, + isLoading: true, + isError: false, + error: null as unknown, + refetch: vi.fn(), + }, + verb, + }; +}); vi.mock("@/queries/identity-resolution", () => ({ useAccountSearch: () => hooks.search, - useAccountBinding: () => ({ + useAccountBinding: () => hooks.binding, + useBindAccount: () => hooks.verb(), + useMergePersons: () => hooks.verb(), + useDetachAccount: () => hooks.verb(), + useExcludeAccount: () => hooks.verb(), + usePersonAccounts: () => ({ data: undefined, - isLoading: true, + isLoading: false, isError: false, - error: null, refetch: vi.fn(), }), + usePersonSearch: () => ({ data: undefined, isFetching: false, isError: false }), })); import { portalRouter } from "@/test/portal-router"; @@ -61,6 +83,8 @@ beforeEach(() => { hooks.search.data = undefined; hooks.search.isFetching = false; hooks.search.isError = false; + hooks.binding.data = undefined; + hooks.binding.isLoading = true; portalRouter.reset(); portalRouter.set({ zone: "manage", item: "identities", mode: "accounts" }); }); @@ -117,6 +141,32 @@ describe("AccountSearchView", () => { ).toBeInTheDocument(); }); + // The search itself proves the account exists — an unbound, never-decided + // one must open ready to bind, not as "the link may be stale" with no verbs. + // Binding an unplaced account is this mode's central use case. + it("opens an unbound, never-decided search hit with the verbs offered", async () => { + hooks.search.data = { items: [match({ person: null })], truncated: false }; + hooks.binding.data = { + source: "github", + source_id: "01900000-0000-7000-8000-00000000aa01", + account_id: "gh-main", + person_id: null, + history: [], + }; + hooks.binding.isLoading = false; + render(); + + await userEvent.click(screen.getByRole("button", { name: /^open$/i })); + + const dialog = screen.getByRole("dialog"); + expect( + within(dialog).queryByText(/link may be stale/i), + ).not.toBeInTheDocument(); + expect( + within(dialog).getByRole("button", { name: /detach into a new person/i }), + ).toBeInTheDocument(); + }); + it("says a cut list was cut", () => { hooks.search.data = { items: [match()], truncated: true }; render(); diff --git a/src/frontend/src/components/portal/account-search-view.tsx b/src/frontend/src/components/portal/account-search-view.tsx index 825fba743..f50b4e792 100644 --- a/src/frontend/src/components/portal/account-search-view.tsx +++ b/src/frontend/src/components/portal/account-search-view.tsx @@ -10,7 +10,7 @@ import { useState } from "react"; import { useTranslation } from "react-i18next"; import { ScanSearch, Search } from "lucide-react"; -import type { AccountMatch } from "@/api/identity-client"; +import type { AccountMatch, AttentionItem } from "@/api/identity-client"; import { CaseDialog } from "@/components/portal/case-dialog"; import { PersonCell } from "@/components/portal/person-cell"; import { Badge } from "@/components/ui/badge"; @@ -48,6 +48,19 @@ export function AccountSearchView() { const items = search.data?.items ?? []; const ordered = items.map((item) => accountKey(item)); const asked = debounced.trim().length >= MIN_QUERY_CHARS; + // The window takes queue-shaped rows; a search hit adapts. This is also the + // voucher that the account exists — without it an unbound, never-decided + // account the search just FOUND would open as a stale link with no verbs. + const asCases: AttentionItem[] = items.map((m) => ({ + kind: "match", + source: m.source, + source_id: m.source_id, + account_id: m.account_id, + email: m.email, + username: m.username, + display_name: m.display_name, + candidates: m.person ? [m.person] : [], + })); return (
    @@ -123,7 +136,7 @@ export function AccountSearchView() { setAcct(null)} diff --git a/src/frontend/src/components/portal/case-dialog.tsx b/src/frontend/src/components/portal/case-dialog.tsx index 84d5debcc..7338a1c24 100644 --- a/src/frontend/src/components/portal/case-dialog.tsx +++ b/src/frontend/src/components/portal/case-dialog.tsx @@ -1,3 +1,4 @@ +import { useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import type { AttentionItem } from "@/api/identity-client"; @@ -13,6 +14,13 @@ import { } from "@/components/ui/dialog"; import { itemKey, parseAccountKey } from "@/lib/identities/account-key"; +/** What the open case keeps when the list under it moves (see below). */ +interface HeldCase { + acct?: string; + item?: AttentionItem; + at?: number; +} + /** * One account under review, in a window rather than a column: this is where * every decision is taken, and a decision that re-attributes a person's work @@ -20,6 +28,12 @@ import { itemKey, parseAccountKey } from "@/lib/identities/account-key"; * * Opened by the `?acct=` in the URL — never by click state alone — so a link * an operator shares lands their colleague on the same case. + * + * A decision prunes its row from the list at once (see `useCorrection`), and + * that list feeds this window: taken literally, the operator's own success + * would yank the candidates, the outcome alert and the prev/next footer out + * from under them. The window therefore HOLDS what it knew about the open + * case — the row and its position — and lets the list move underneath. */ export function CaseDialog({ acct, @@ -36,13 +50,41 @@ export function CaseDialog({ onClose: () => void; }) { const { t } = useTranslation(); + const popupRef = useRef(null); const ref = parseAccountKey(acct); - const queueItem = items.find((i) => itemKey(i) === acct); + + const live = items.find((i) => itemKey(i) === acct); + const liveAt = acct ? ordered.indexOf(acct) : -1; + + // Held via state adjusted during render (the sanctioned previous-render + // pattern), so the freshest row and position stick for the open case. + const [held, setHeld] = useState({}); + const fresh: HeldCase = { + acct, + item: live ?? (held.acct === acct ? held.item : undefined), + at: liveAt >= 0 ? liveAt : held.acct === acct ? held.at : undefined, + }; + if (held.acct !== fresh.acct || held.item !== fresh.item || held.at !== fresh.at) { + setHeld(fresh); + } + const queueItem = fresh.item; + const heading = - queueItem?.email?.trim() || queueItem?.username?.trim() || ref?.account_id; - const at = acct ? ordered.indexOf(acct) : -1; + queueItem?.email?.trim() || + queueItem?.username?.trim() || + queueItem?.display_name?.trim() || + ref?.account_id; + // The caller vouching for the account (a queue row, a search hit) is what + // separates a real account from a mistyped link; the vouching survives the + // row being pruned, since the account did not stop existing. + const observed = queueItem != null || liveAt >= 0; + + // When the open row was pruned, everything after it shifted left — so the + // held index now points at the NEXT account, which is where a conveyor + // should go, and one before it is still the previous one. + const at = liveAt >= 0 ? liveAt : (fresh.at ?? -1); const previous = at > 0 ? ordered[at - 1] : undefined; - const next = at >= 0 ? ordered[at + 1] : undefined; + const next = liveAt >= 0 ? ordered[at + 1] : at >= 0 ? ordered[at] : undefined; return ( " — the wrong first thing to hear), + // and `initialFocus={false}` does not move focus AT ALL, stranding it + // in the aria-hidden page behind the dialog. + tabIndex={-1} + initialFocus={popupRef} > {heading} @@ -84,7 +129,12 @@ export function CaseDialog({ outcome, an open confirmation), and a cached binding renders the next case synchronously — unkeyed, that state would follow. */} {ref ? ( - + ) : null} {/* Working a backlog is a conveyor: the next case is one press away, without a trip back through the list. */} diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index dd2c26ae6..80cb42614 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -465,6 +465,37 @@ describe("IdentitiesView", () => { expect(portalRouter.search.acct).toContain("a2"); }); + // A decision prunes its row from the list at once. The window must hold + // what it knew — the case and its position — or the operator's own success + // kills the outcome they are reading and the Next button they are about to + // press. + it("keeps the open case and the conveyor when the decided row is pruned", async () => { + attention.q.data = { + items: [ + item({ account_id: "a1", email: "ann@example.com" }), + item({ account_id: "a2", email: "bob@example.com" }), + ], + rates: RATES, + }; + const view = render(); + + await userEvent.click(screen.getByRole("button", { name: /ann@example\.com/i })); + // The verb landed: the server said "decided", the cache dropped the row. + attention.q.data = { + items: [item({ account_id: "a2", email: "bob@example.com" })], + rates: RATES, + }; + view.rerender(); + + const dialog = screen.getByRole("dialog"); + // The case still names what was decided, not a stale-link apology. + expect(within(dialog).getByText(/ann@example\.com/)).toBeInTheDocument(); + expect(within(dialog).queryByText(/link may be stale/i)).not.toBeInTheDocument(); + // And the conveyor still moves: the row that shifted into this slot is next. + await userEvent.click(within(dialog).getByRole("button", { name: /next account/i })); + expect(portalRouter.search.acct).toContain("a2"); + }); + // Modes are ways IN to the same decisions; the mode rides in the URL so a // link opens the one it was sent from. it("switches modes through the URL, dropping the account selected in the old one", async () => { diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 7176f1394..4ee0dc695 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -324,9 +324,14 @@ function Queue({ items: everything }: { items: AttentionItem[] }) { // Closing the window puts the operator back on the row they opened, not at // the top of the page — the queue is worked in one pass. const returnFocus = (key: string) => { - listRef.current - ?.querySelector(`[data-queue-row="${CSS.escape(key)}"]`) - ?.focus(); + const row = + listRef.current?.querySelector( + `[data-queue-row="${CSS.escape(key)}"]`, + ) ?? + // The row an operator just decided is pruned by the time the window + // closes — fall to the top of the list rather than to nowhere. + listRef.current?.querySelector("[data-queue-row]"); + row?.focus(); }; // The queue is a list, so it moves like one. Enter and Space open a row; @@ -410,11 +415,24 @@ function QueueFilter() { const setSearch = useSetPortalSearch(); const [query, setQuery] = useState(filter ?? ""); const debounced = useDebouncedValue(query, FILTER_DEBOUNCE_MS); + // What this component last wrote to the URL. Distinguishes its own write + // landing (ignore) from an external change — Back, a pasted link — which + // must reach the input, or the box shows a filter the list stopped using. + const lastWritten = useRef(filter ?? ""); useEffect(() => { + lastWritten.current = debounced.trim(); setSearch({ filter: debounced.trim() || undefined }, { replace: true }); }, [debounced, setSearch]); + useEffect(() => { + const external = filter ?? ""; + if (external !== lastWritten.current) { + lastWritten.current = external; + setQuery(external); + } + }, [filter]); + return (
    diff --git a/src/frontend/src/components/portal/person-accounts-view.tsx b/src/frontend/src/components/portal/person-accounts-view.tsx index 6cc7dcc24..4ac857585 100644 --- a/src/frontend/src/components/portal/person-accounts-view.tsx +++ b/src/frontend/src/components/portal/person-accounts-view.tsx @@ -15,7 +15,11 @@ import { useState } from "react"; import { useTranslation } from "react-i18next"; import { UserSearch } from "lucide-react"; -import type { PersonAccountEntry, PersonSummary } from "@/api/identity-client"; +import type { + AttentionItem, + PersonAccountEntry, + PersonSummary, +} from "@/api/identity-client"; import { CaseDialog } from "@/components/portal/case-dialog"; import { PersonCell } from "@/components/portal/person-cell"; import { PersonPicker } from "@/components/portal/person-picker"; @@ -105,6 +109,18 @@ function PersonAccounts({ const entries = accounts.data.accounts; const ordered = entries.map((entry) => accountKey(entry)); + // Queue-shaped rows for the window: the voucher that each account exists + // (they are read from the person's own bindings), plus the person as the + // one candidate so the current binding renders as a card, not a bare id. + const asCases: AttentionItem[] = entries.map((entry) => ({ + kind: "member", + source: entry.source, + source_id: entry.source_id, + account_id: entry.account_id, + email: entry.email, + username: entry.username, + candidates: [card ?? { person_id: personId }], + })); return ( @@ -133,7 +149,7 @@ function PersonAccounts({ setAcct(null)} From e43ecfb8544fa68e2531f569c2b28499a11ba501 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 07:59:03 +0300 Subject: [PATCH 18/19] frontend+stand: the review's smaller catches, and the tests that were missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accessibility, from the review's live pass: - the person picker nested the card's copy
    @@ -255,8 +258,8 @@ function HistoryRow({ opened for — how long this has stood. Neither replaces the other. */} {formatUtcInstant(entry.recorded_at, "d MMM yyyy, HH:mm")} - - {formatUtcAge(entry.recorded_at)} + + {` (${formatUtcAge(entry.recorded_at)})`}
    diff --git a/src/frontend/src/components/portal/identities-view.test.tsx b/src/frontend/src/components/portal/identities-view.test.tsx index 80cb42614..f6d7c7516 100644 --- a/src/frontend/src/components/portal/identities-view.test.tsx +++ b/src/frontend/src/components/portal/identities-view.test.tsx @@ -215,8 +215,8 @@ describe("IdentitiesView", () => { expect(screen.getByText(/1 case · 3 accounts/i)).toBeInTheDocument(); // The candidates are stated once for the case, so each row has to say // which of them it would be taking the account from. - expect(screen.getByText(/now Ann Lee/i)).toBeInTheDocument(); - expect(screen.getAllByText(/now Bob Park/i)).toHaveLength(2); + expect(screen.getByText(/held by Ann Lee/i)).toBeInTheDocument(); + expect(screen.getAllByText(/held by Bob Park/i)).toHaveLength(2); // Each account still has its own row: a decision is taken per account. expect(screen.getAllByRole("button", { name: /dev42@example\.com/i })).toHaveLength(3); }); diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 4ee0dc695..4bd044abc 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -258,9 +258,11 @@ function Tile({ {label} + {/* Focusable, or the hint — which carries the tile's actual + meaning — exists for mouse users only. */} } - aria-label={label} + render={} + aria-label={hint} > @@ -650,8 +652,24 @@ function CaseBlock({ }} aria-pressed={selected} // Without a label the name is computed from everything inside, - // which reads out before the account is even named. - aria-label={`${label} ${item.source}`} + // which reads out before the account is even named. With one, + // it must still carry what a sighted operator sees at a glance: + // who holds it, its status, what the source says it is. + aria-label={[ + label, + item.source, + boundTo + ? t("identities.queue.bound_to", { + name: personDisplayName(boundTo), + }) + : null, + item.status?.trim().toLowerCase() === "active" + ? null + : item.status, + description || null, + ] + .filter(Boolean) + .join(", ")} className={cn( "cursor-pointer rounded-md border p-3 text-start select-text", selected diff --git a/src/frontend/src/components/portal/person-picker.tsx b/src/frontend/src/components/portal/person-picker.tsx index 8549677ed..ef75efe1b 100644 --- a/src/frontend/src/components/portal/person-picker.tsx +++ b/src/frontend/src/components/portal/person-picker.tsx @@ -14,6 +14,7 @@ import { useTranslation } from "react-i18next"; import type { PersonSummary } from "@/api/identity-client"; import { PersonCell } from "@/components/portal/person-cell"; +import { personDisplayName } from "@/lib/identities/person-display"; import { Input } from "@/components/ui/input"; import { Spinner } from "@/components/ui/spinner"; import { useDebouncedValue } from "@/hooks/use-debounced-value"; @@ -73,13 +74,32 @@ export function PersonPicker({
      {results.map((person) => (
    • - +
      • ))}
    diff --git a/src/frontend/src/locales/en/translation.json b/src/frontend/src/locales/en/translation.json index 99f88f7ac..019a667aa 100644 --- a/src/frontend/src/locales/en/translation.json +++ b/src/frontend/src/locales/en/translation.json @@ -403,10 +403,10 @@ }, "identities": { "title": "Identities", - "subtitle": "Accounts the resolver could not decide — review and resolve them.", + "subtitle": "Who each account belongs to — review what the resolver could not decide, and correct what it did.", "rates": { "decisions": "Needs a decision", - "decisions_hint": "Cases waiting for you. This is the queue below — the only number here you can act on.", + "decisions_hint": "Accounts waiting for you. This is the queue below — the only number here you can act on.", "observed": "Observed", "observed_hint": "Every account a connector currently reports, closed ones excluded.", "bound": "Bound to a person", @@ -414,7 +414,7 @@ "pending": "Unbound · has an address", "pending_hint": "Waiting for the resolver, not for you: an address is present, so the next run binds it. Only the contested ones reach the queue.", "no_evidence": "Unbound · nothing to match on", - "no_evidence_hint": "No address, so automation has no key to match on. Only a person can bind these — they are in the queue.", + "no_evidence_hint": "No address, so automation has no key to match on. Only an operator can bind these — they are in the queue.", "excluded": "Excluded", "excluded_hint": "Declared not a person — a bot, CI or service account. Dropped from every person metric." }, @@ -422,7 +422,7 @@ "contested": "Contested — more than one person claims the account", "binding_conflict": "Binding conflicts — evidence disagrees with the current binding", "provisioned_at_login": "Given a person at first sign-in — not yet confirmed", - "no_evidence": "No address to match on — only a person can bind these", + "no_evidence": "No address to match on — only an operator can bind these", "other": "Needs review" }, "queue": { @@ -441,7 +441,7 @@ "previous_case": "‹ Previous account", "next_case": "Next account ›", "reports_to": "reports to {{manager}}", - "bound_to": "now {{name}}", + "bound_to": "held by {{name}}", "case_people_one": "{{count}} person", "case_people_other": "{{count}} people", "case_accounts_one": "{{count}} account", @@ -475,7 +475,8 @@ "by": "by", "by_name": "by {{name}}", "accounts_touched_one": "{{count}} account in this call", - "accounts_touched_other": "{{count}} accounts in this call" + "accounts_touched_other": "{{count}} accounts in this call", + "call": "the call" }, "actions": { "bind": "Bind", @@ -486,14 +487,15 @@ "detach_confirm": "Detach", "exclude": "Exclude (bot / CI)", "exclude_confirm": "Exclude", - "assign_other": "Assign to someone else" + "assign_other": "Assign to someone else", + "assign_person": "Assign to a person" }, "dialogs": { "bind_title": "Bind this account?", "confirm_title": "Confirm the current binding?", "bind_description": "This account moves to {{name}}. Its activity counts towards them from here on — another bind moves it back.", "merge_title": "Merge two persons?", - "merge_description": "Every account of the currently bound person moves to {{name}}. Merging back is another merge — nothing is lost, but metrics re-attribute.", + "merge_description": "Every account of {{source}} moves to {{target}}. Merging back is another merge — nothing is lost, but metrics re-attribute.", "merge_preview_loading": "Counting the accounts that would move…", "merge_preview_failed": "Could not count the accounts that would move.", "merge_preview_one": "{{count}} account moves:", diff --git a/src/frontend/src/queries/identity-resolution.test.tsx b/src/frontend/src/queries/identity-resolution.test.tsx new file mode 100644 index 000000000..f3baa746a --- /dev/null +++ b/src/frontend/src/queries/identity-resolution.test.tsx @@ -0,0 +1,135 @@ +/** + * The verb hooks' cache behavior. What matters: the accounts the SERVER + * reported as decided leave the cached attention queue at once — under the + * REAL query key, sessionScope suffix included, since a prefix drift here + * ships green and the operator keeps staring at rows they already decided + * until the slow refetch lands. A refused account keeps its row, and both + * the resolution and person-search cache families are invalidated. + */ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderHook, waitFor } from "@testing-library/react"; +import { createElement, type ReactNode } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import * as identityClient from "@/api/identity-client"; + +vi.mock("@/api/identity-client"); + +const session = vi.hoisted(() => ({ value: { scope: "tenant-a" } as unknown })); +vi.mock("@/auth/use-auth", () => ({ + useAuth: () => ({ session: session.value }), +})); +vi.mock("@/auth/session-scope", () => ({ + sessionAuthorizationScope: (s: unknown) => + s == null ? null : (s as { scope: string }).scope, +})); + +import { useBindAccount } from "./identity-resolution"; + +const bindAccount = vi.mocked(identityClient.bindAccount); + +const ATTENTION_KEY = ["identity", "resolution", "attention", "tenant-a"]; + +const REF = { + source: "github", + source_id: "01900000-0000-7000-8000-00000000aa01", + account_id: "a1", +}; + +function item(account_id: string): identityClient.AttentionItem { + return { + kind: "contested", + source: REF.source, + source_id: REF.source_id, + account_id, + email: `${account_id}@example.com`, + username: null, + candidates: [], + }; +} + +function outcome(status: string): identityClient.CorrectionResponse { + return { + applied: status === "applied" ? 1 : 0, + already_decided: 0, + items: [{ ...REF, outcome: status }], + }; +} + +function harness() { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + const wrapper = ({ children }: { children: ReactNode }) => + createElement(QueryClientProvider, { client: queryClient }, children); + return { queryClient, wrapper }; +} + +beforeEach(() => { + vi.resetAllMocks(); + session.value = { scope: "tenant-a" }; +}); + +describe("useBindAccount cache behavior", () => { + it("drops the decided row from the cached queue under the real key", async () => { + const { queryClient, wrapper } = harness(); + queryClient.setQueryData(ATTENTION_KEY, { + items: [item("a1"), item("a2")], + rates: { observed: 2, bound: 0, pending: 2, no_evidence: 0, excluded: 0 }, + }); + bindAccount.mockResolvedValueOnce(outcome("applied")); + + const { result } = renderHook(() => useBindAccount(), { wrapper }); + result.current.mutate({ + account: { source: REF.source, source_id: REF.source_id, id: REF.account_id }, + person_id: "01900000-0000-7000-8000-0000000000b0", + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + const cached = queryClient.getQueryData( + ATTENTION_KEY, + ); + expect(cached?.items.map((i) => i.account_id)).toEqual(["a2"]); + }); + + // A refusal changed nothing on the server, so the row must survive the + // prune — removing it would show a queue that dealt with something the + // server declined. + it("keeps a refused row in the cached queue", async () => { + const { queryClient, wrapper } = harness(); + queryClient.setQueryData(ATTENTION_KEY, { + items: [item("a1")], + rates: { observed: 1, bound: 0, pending: 1, no_evidence: 0, excluded: 0 }, + }); + bindAccount.mockResolvedValueOnce(outcome("refused")); + + const { result } = renderHook(() => useBindAccount(), { wrapper }); + result.current.mutate({ + account: { source: REF.source, source_id: REF.source_id, id: REF.account_id }, + person_id: "01900000-0000-7000-8000-0000000000b0", + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + const cached = queryClient.getQueryData( + ATTENTION_KEY, + ); + expect(cached?.items).toHaveLength(1); + }); + + it("invalidates the resolution reads and the person-search cache", async () => { + const { queryClient, wrapper } = harness(); + const invalidate = vi.spyOn(queryClient, "invalidateQueries"); + bindAccount.mockResolvedValueOnce(outcome("applied")); + + const { result } = renderHook(() => useBindAccount(), { wrapper }); + result.current.mutate({ + account: { source: REF.source, source_id: REF.source_id, id: REF.account_id }, + person_id: "01900000-0000-7000-8000-0000000000b0", + }); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + const keys = invalidate.mock.calls.map((c) => c[0]?.queryKey); + expect(keys).toContainEqual(["identity", "resolution"]); + expect(keys).toContainEqual(["identity", "persons", "search"]); + }); +}); diff --git a/src/frontend/vitest.config.ts b/src/frontend/vitest.config.ts index 12086822a..675df65bd 100644 --- a/src/frontend/vitest.config.ts +++ b/src/frontend/vitest.config.ts @@ -28,6 +28,11 @@ export default defineConfig({ }, }, test: { + // A timezone with no UTC-coinciding offset, ever. CI runners live in UTC, + // where "parse a zone-less timestamp as UTC" and "parse it as local" are + // the same function — every zone-handling test passes vacuously. Pinning + // a non-UTC zone makes those tests able to fail. + env: { TZ: "Pacific/Kiritimati" }, // Coverage is a GLOBAL option in Vitest — with `projects` it must live at // the root `test` level; a `coverage` block nested inside a project is // ignored (which silently dropped our `cobertura` reporter and left CI's diff --git a/tests/stand/api/identity/test_resolution.py b/tests/stand/api/identity/test_resolution.py index fa223bbbf..7b59e5868 100644 --- a/tests/stand/api/identity/test_resolution.py +++ b/tests/stand/api/identity/test_resolution.py @@ -1,6 +1,7 @@ """The operator correction surface — the manual-resolution routes on the deployed path. GET /v1/resolution/attention 200 rates arithmetic · 403 realm admin + GET /v1/resolution/accounts 200 search + holder · 400 short q · 403 realm admin GET /v1/resolution/accounts/{source}/{sid}/{aid} 200 binding + history (round trip) GET /v1/resolution/persons/{id}/accounts 200 for a seeded person POST /v1/resolution/bind 200 applied → already_decided (round trip) · 400 excluded sentinel @@ -31,12 +32,14 @@ from .. import scratch from ..schemas import ( AccountBindingResponse, + AccountSearchResponse, AttentionResponse, CorrectionResponse, PersonAccountsResponse, ) ATTENTION = identity_path("/v1/resolution/attention") +ACCOUNT_SEARCH = identity_path("/v1/resolution/accounts") #: The reserved excluded-person sentinel (`Uuid::from_u128(u128::MAX)` in the #: service). Once any account was ever excluded it exists in the journal, so @@ -73,6 +76,64 @@ def test_the_realm_admin_is_refused_the_queue(realm_admin_session: PersonaSessio assert response.status_code == 403, f"{response.status_code} {response.text[:300]}" +@pytest.mark.requires_seed("ceo") +@pytest.mark.security +def test_the_realm_admin_is_refused_the_account_search( + realm_admin_session: PersonaSession, +) -> None: + """The gate runs before `q` validation: a non-admin with no query at all + still learns nothing but 403 — a 400 would reveal the gate ran second.""" + response = realm_admin_session.client.get(ACCOUNT_SEARCH) + assert response.status_code == 403, f"{response.status_code} {response.text[:300]}" + + +@pytest.mark.requires_seed("admin_operator") +@pytest.mark.reliability +def test_a_short_account_search_needle_is_refused( + admin_operator_session: PersonaSession, +) -> None: + """Under three characters the needle would scan the whole fold to answer + with everything; the service refuses rather than obliging.""" + response = admin_operator_session.client.get(ACCOUNT_SEARCH, params={"q": "ab"}) + assert response.status_code == 400, f"{response.status_code} {response.text[:300]}" + + +@pytest.mark.requires_seed("admin_operator", "dev_lead") +@pytest.mark.reliability +def test_account_search_finds_a_seeded_account_and_names_its_holder( + admin_operator_session: PersonaSession, stand_manifest: Manifest +) -> None: + """`GET /v1/resolution/accounts?q=` answers with the account AND whose it + is — the mode exists for an operator holding a value, not a person. The + seeded lead's address is observed by the roster connector and bound by the + seed, so searching it must return at least one account holding a hydrated + person; every match must echo the needle in one of its searched values. + """ + lead = stand_manifest.fixture("dev_lead") + needle = lead.email + + response = admin_operator_session.client.get(ACCOUNT_SEARCH, params={"q": needle}) + assert response.status_code == 200, f"{response.status_code} {response.text[:300]}" + + found = response.parse(AccountSearchResponse) + assert found.items, f"a seeded stand finds no account for {needle!r}" + lowered = needle.lower() + for match in found.items: + carried = [ + value + for value in (match.email, match.username, match.display_name, match.account_id) + if value is not None + ] + assert any(lowered in value.lower() for value in carried), ( + f"match carries none of the searched values: {match!r}" + ) + bound = [match for match in found.items if match.person is not None] + assert bound, "the seeded address resolves to a bound account, so a holder must appear" + assert any( + (match.person.email or "").lower() == lowered for match in bound + ), f"no holder card carries the searched address: {bound!r}" + + @pytest.mark.requires_seed("admin_operator") @pytest.mark.reliability def test_the_queue_answers_with_coherent_tenant_wide_rates( diff --git a/tests/stand/api/schemas/__init__.py b/tests/stand/api/schemas/__init__.py index 27477df93..55660b1de 100644 --- a/tests/stand/api/schemas/__init__.py +++ b/tests/stand/api/schemas/__init__.py @@ -65,6 +65,7 @@ ) from .identity import ( AccountBindingResponse, + AccountSearchResponse, AttentionResponse, CorrectionResponse, MeResponse, @@ -117,6 +118,7 @@ "EXTRACTOR_REJECTION_CONTENT_TYPE", "PROBLEM_CONTENT_TYPE", "AccountBindingResponse", + "AccountSearchResponse", "AttentionResponse", "CorrectionResponse", "CustomMetric", From 1d5d0bd555a5fefe60bffc216f0940eefcbae296 Mon Sep 17 00:00:00 2001 From: Sergei Mozhaev Date: Sat, 15 Aug 2026 08:23:38 +0300 Subject: [PATCH 19/19] identity: a bound account never reads as unbound in the search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ways the account search lied about a holder, both from external review: A holder with no card attributes — a login-minted stub — missed the card hydration entirely, so the response carried `person: null` and the row said "bound to nobody" about a bound account. The holder is now back-filled as an id-only card, the same rule every other surface applies. An excluded account came out the same way: `person: null`, indistinguishable from never-bound — presenting an operator's recorded exclusion as an invitation to bind the bot and undo it. The response now says `excluded` explicitly and the row renders it as its own state, not as an absence. Also from the same review: the cache-prune key in `cases.ts` used literal NUL bytes as separators, which made git treat the TypeScript file as binary — it now uses the shared `accountKey`, which is collision-safe by URI-encoding; a comment in the query layer stated latency as a fact about real deployments rather than a property of dataset size; and two files gained a blank line at EOF. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Sergei Mozhaev --- .../backend/identity-resolution/openapi.json | 7 ++++- .../identity-resolution/src/api/resolution.rs | 25 +++++++++++++----- src/frontend/src/api/identity-client.ts | 7 ++++- .../src/components/portal/account-detail.tsx | 1 - .../portal/account-search-view.test.tsx | 13 +++++++++ .../components/portal/account-search-view.tsx | 7 ++++- .../src/components/portal/identities-view.tsx | 1 - src/frontend/src/lib/identities/cases.ts | Bin 3925 -> 3827 bytes src/frontend/src/locales/en/translation.json | 3 ++- .../src/queries/identity-resolution.ts | 10 +++---- tests/stand/api/schemas/identity.py | 1 + 11 files changed, 58 insertions(+), 17 deletions(-) diff --git a/docs/components/backend/identity-resolution/openapi.json b/docs/components/backend/identity-resolution/openapi.json index 026e2e6ab..db16cb99d 100644 --- a/docs/components/backend/identity-resolution/openapi.json +++ b/docs/components/backend/identity-resolution/openapi.json @@ -66,6 +66,10 @@ "null" ] }, + "excluded": { + "description": "The account is deliberately excluded from person metrics (a bot, CI, a\nservice account). Without this an exclusion — an operator's recorded\ndecision — would read as \"bound to nobody\" and invite undoing it.", + "type": "boolean" + }, "person": { "oneOf": [ { @@ -73,7 +77,7 @@ }, { "$ref": "#/components/schemas/PersonSummaryResponse", - "description": "The person holding it, hydrated. Absent means nobody holds it yet." + "description": "The person holding it, hydrated. Absent means nobody holds it yet —\nexcept when `excluded` is set, which is a different fact entirely." } ] }, @@ -95,6 +99,7 @@ "source", "source_id", "account_id", + "excluded", "bound_by_operator" ], "type": "object" diff --git a/src/backend/services/identity-resolution/src/api/resolution.rs b/src/backend/services/identity-resolution/src/api/resolution.rs index dd0ddb17c..fdb75784c 100644 --- a/src/backend/services/identity-resolution/src/api/resolution.rs +++ b/src/backend/services/identity-resolution/src/api/resolution.rs @@ -882,8 +882,13 @@ pub struct AccountMatchResponse { pub email: Option, pub username: Option, pub display_name: Option, - /// The person holding it, hydrated. Absent means nobody holds it yet. + /// The person holding it, hydrated. Absent means nobody holds it yet — + /// except when `excluded` is set, which is a different fact entirely. pub person: Option, + /// The account is deliberately excluded from person metrics (a bot, CI, a + /// service account). Without this an exclusion — an operator's recorded + /// decision — would read as "bound to nobody" and invite undoing it. + pub excluded: bool, /// `true` when a person decided this binding rather than automation. pub bound_by_operator: bool, } @@ -970,12 +975,20 @@ pub async fn search_accounts( email: account.email, username: account.username, display_name: account.description.display_name, - person: binding - .and_then(|b| cards.get(&b.person_id).cloned()) - .map(|card| PersonSummaryResponse { - provisional: provisional.contains(&card.person_id), + // A holder with no card attributes (a login-minted stub) is + // still a holder: back-fill an id-only card rather than + // presenting a bound account as unbound. + person: binding.filter(|b| b.person_id != EXCLUDED_PERSON).map(|b| { + let card = cards + .get(&b.person_id) + .cloned() + .unwrap_or_else(|| PersonCard::empty(b.person_id)); + PersonSummaryResponse { + provisional: provisional.contains(&b.person_id), ..PersonSummaryResponse::from(card) - }), + } + }), + excluded: binding.is_some_and(|b| b.person_id == EXCLUDED_PERSON), bound_by_operator: binding.is_some_and(KnownBinding::is_operator_authored), } }) diff --git a/src/frontend/src/api/identity-client.ts b/src/frontend/src/api/identity-client.ts index af5cba77c..8b0d4bea6 100644 --- a/src/frontend/src/api/identity-client.ts +++ b/src/frontend/src/api/identity-client.ts @@ -186,8 +186,13 @@ export interface AccountMatch { email?: string | null; username?: string | null; display_name?: string | null; - /** The person holding it; absent = nobody holds it yet. */ + /** The person holding it; absent = nobody holds it yet — unless + * `excluded` is set, which is a different fact entirely. */ person?: PersonSummary | null; + /** Deliberately excluded from person metrics (bot / CI). An operator's + * recorded decision — not "bound to nobody", and not an invitation to + * bind. Optional so an older backend reads as never-excluded. */ + excluded?: boolean; bound_by_operator: boolean; } diff --git a/src/frontend/src/components/portal/account-detail.tsx b/src/frontend/src/components/portal/account-detail.tsx index 581d1a3d8..7205b30bc 100644 --- a/src/frontend/src/components/portal/account-detail.tsx +++ b/src/frontend/src/components/portal/account-detail.tsx @@ -313,4 +313,3 @@ function SectionLabel({ children }: { children: React.ReactNode }) { ); } - diff --git a/src/frontend/src/components/portal/account-search-view.test.tsx b/src/frontend/src/components/portal/account-search-view.test.tsx index 4533fcc9e..02a7b641f 100644 --- a/src/frontend/src/components/portal/account-search-view.test.tsx +++ b/src/frontend/src/components/portal/account-search-view.test.tsx @@ -167,6 +167,19 @@ describe("AccountSearchView", () => { ).toBeInTheDocument(); }); + // An exclusion is an operator's recorded decision; presenting it as + // "bound to nobody" invites binding the bot and undoing that decision. + it("shows an excluded account as excluded, not as unbound", () => { + hooks.search.data = { + items: [match({ person: null, excluded: true, bound_by_operator: true })], + truncated: false, + }; + render(); + + expect(screen.getByText(/excluded — bot \/ CI/i)).toBeInTheDocument(); + expect(screen.queryByText(/bound to nobody/i)).not.toBeInTheDocument(); + }); + it("says a cut list was cut", () => { hooks.search.data = { items: [match()], truncated: true }; render(); diff --git a/src/frontend/src/components/portal/account-search-view.tsx b/src/frontend/src/components/portal/account-search-view.tsx index f50b4e792..cfd40a230 100644 --- a/src/frontend/src/components/portal/account-search-view.tsx +++ b/src/frontend/src/components/portal/account-search-view.tsx @@ -174,9 +174,14 @@ function AccountRow({ {/* Whose it is — the answer the mode exists for. Unbound is an answer - too, and a different one from "nobody has decided yet". */} + too, and exclusion is a third one: an operator's recorded decision, + which "bound to nobody" would invite undoing. */} {item.person ? ( + ) : item.excluded ? ( + + {t("identities.accounts.excluded")} + ) : ( {t("identities.accounts.unbound")} diff --git a/src/frontend/src/components/portal/identities-view.tsx b/src/frontend/src/components/portal/identities-view.tsx index 4bd044abc..b11c0f51c 100644 --- a/src/frontend/src/components/portal/identities-view.tsx +++ b/src/frontend/src/components/portal/identities-view.tsx @@ -710,4 +710,3 @@ function CaseBlock({ ); } - diff --git a/src/frontend/src/lib/identities/cases.ts b/src/frontend/src/lib/identities/cases.ts index 4ee328c33fb28a8d0179f6b9965468edc62c5d48..3832501fcb433867c234f4115aa77fe94db5c294 100644 GIT binary patch delta 79 zcmcaA_gQv>G&6T%a&mrYUWs>VrOsw4=0;8~guvu;JZ_V3ad62grlclkrlh9mWh54B TAmlYNOHy++H8(f%USR|PeaIZM delta 183 zcmew?dsS|OH1lS4<_1n( return useMutation({ mutationFn: run, // The journal stays the truth, and the refetch below is what reconciles - // to it — but the attention read folds every observed account, so on a - // real tenant it takes seconds, and until it lands the operator is still - // looking at the row they just decided. Dropping the accounts the SERVER - // reported as decided is not a guess about the new state; a `refused` - // account keeps its row, and everything else follows from the refetch. + // to it — but the attention read folds every observed account, so its + // latency grows with the dataset, and until it lands the operator is + // still looking at the row they just decided. Dropping the accounts the + // SERVER reported as decided is not a guess about the new state; a + // `refused` account keeps its row, and the rest follows from the refetch. onSuccess: (result) => { client.setQueriesData( { queryKey: [...RESOLUTION_KEY, "attention"] }, diff --git a/tests/stand/api/schemas/identity.py b/tests/stand/api/schemas/identity.py index 15dd03fcd..de8c1f3a9 100644 --- a/tests/stand/api/schemas/identity.py +++ b/tests/stand/api/schemas/identity.py @@ -475,6 +475,7 @@ class AccountMatchResponse(BaseModel): bound_by_operator: bool = Field(..., description='`true` when a person decided this binding rather than automation.') display_name: str | None = None email: str | None = None + excluded: bool = Field(..., description='The account is deliberately excluded from person metrics (a bot, CI, a\nservice account). Without this an exclusion — an operator\'s recorded\ndecision — would read as "bound to nobody" and invite undoing it.') person: PersonSummaryResponse | None = None source: str source_id: UUID