Repository navigation
feat(gui): Integrations shows the clients on this machine first #3391
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
89c7005
163841a
c7eb197
4c4a157
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import { useEffect, useRef, useState, type KeyboardEvent } from "react"; | ||
| import { useCallback, useEffect, useId, useRef, useState, type KeyboardEvent } from "react"; | ||
| import { navigateHash, normalizeHashPath } from "../hash-routing"; | ||
| import { useT } from "../i18n/shared"; | ||
| import { useDataSurface } from "../data-surface"; | ||
| import ClientMark from "../components/ClientMark"; | ||
| import { INTEGRATION_MARKS } from "../components/integration-marks"; | ||
| import ApiKeys from "./ApiKeys"; | ||
|
|
@@ -12,6 +13,7 @@ import FileIntegrationPage, { | |
| type FileIntegrationClientId, | ||
| } from "./integrations/FileIntegrationPage"; | ||
| import { FILE_CLIENTS, TABS, type IntegrationTab } from "./integrations/integration-tabs"; | ||
| import { loadIntegrationStates, type IntegrationStatus } from "./integrations/integration-api"; | ||
|
|
||
| function readIntegrationTab(hash = window.location.hash): IntegrationTab { | ||
| const raw = normalizeHashPath(hash); | ||
|
|
@@ -56,6 +58,35 @@ export default function Integrations({ apiBase, machineApiBase = apiBase, connec | |
| const [machineSyncing, setMachineSyncing] = useState(false); | ||
| if (tabRefs.current === null) tabRefs.current = new Map(); | ||
|
|
||
| /* | ||
| * Eighteen tabs, most of them clients that are not installed on this machine, is the | ||
| * page's largest noise source. Tabs for uninstalled file clients hide behind one | ||
| * "more" button; everything the operator can actually act on stays in the strip. | ||
| * The state comes from the same keyed resource the overview reads, so this is not a | ||
| * second fetch. Until it settles every tab is primary — a strip must never flash-hide. | ||
| */ | ||
| const fetchStates = useCallback( | ||
| async (signal: AbortSignal) => (await loadIntegrationStates(apiBase, signal)).clients, | ||
| [apiBase], | ||
| ); | ||
| const statesResource = useDataSurface<IntegrationStatus[]>( | ||
| `integration-states:${apiBase}`, | ||
| [apiBase], | ||
| fetchStates, | ||
| { isEmpty: rows => rows.length === 0, sessionCacheKey: `ocx.integrations.states.v1:${apiBase}` }, | ||
| ); | ||
| const statesSettled = statesResource.state.kind !== "cold" && statesResource.state.kind !== "retrying-cold"; | ||
| const installedFileClients = new Set((statesResource.state.data ?? []).filter(c => c.installed).map(c => c.clientId)); | ||
| const isSecondary = (id: IntegrationTab) => | ||
| statesSettled && FILE_CLIENTS.has(id as FileIntegrationClientId) && !installedFileClients.has(id as FileIntegrationClientId); | ||
|
Comment on lines
+78
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the initial Useful? React with 👍 / 👎. |
||
| const secondaryCount = TABS.filter(d => isSecondary(d.id)).length; | ||
| const [moreOpen, setMoreOpen] = useState(false); | ||
| const tablistId = useId(); | ||
| // The selected tab can never be hidden: a deep link to an uninstalled client opens the | ||
| // overflow, and the button is disabled while such a tab is selected. | ||
| const selectedIsSecondary = isSecondary(tab); | ||
| const showSecondary = moreOpen || selectedIsSecondary; | ||
|
|
||
| useEffect(() => { | ||
| if (!connected) return; | ||
| const controller = new AbortController(); | ||
|
|
@@ -115,12 +146,16 @@ export default function Integrations({ apiBase, machineApiBase = apiBase, connec | |
|
|
||
| const handleTabKeyDown = (event: KeyboardEvent<HTMLButtonElement>) => { | ||
| const index = TABS.findIndex(candidate => candidate.id === tab); | ||
| let nextIndex: number | null = null; | ||
| if (event.key === "ArrowLeft") nextIndex = (index - 1 + TABS.length) % TABS.length; | ||
| else if (event.key === "ArrowRight") nextIndex = (index + 1) % TABS.length; | ||
| else if (event.key === "Home") nextIndex = 0; | ||
| else if (event.key === "End") nextIndex = TABS.length - 1; | ||
| if (nextIndex === null) return; | ||
| // Arrows walk the VISIBLE tabs only; a hidden tab is not a stop. | ||
| const visible = TABS.map((d, i) => ({ d, i })).filter(({ d }) => showSecondary || !isSecondary(d.id)); | ||
| const pos = visible.findIndex(({ i }) => i === index); | ||
| let nextPos: number | null = null; | ||
| if (event.key === "ArrowLeft") nextPos = (pos - 1 + visible.length) % visible.length; | ||
| else if (event.key === "ArrowRight") nextPos = (pos + 1) % visible.length; | ||
| else if (event.key === "Home") nextPos = 0; | ||
| else if (event.key === "End") nextPos = visible.length - 1; | ||
| if (nextPos === null) return; | ||
| const nextIndex = visible[nextPos]!.i; | ||
| event.preventDefault(); | ||
| selectTab(TABS[nextIndex].id, true); | ||
| }; | ||
|
|
@@ -130,7 +165,6 @@ export default function Integrations({ apiBase, machineApiBase = apiBase, connec | |
| <div className="page-head"> | ||
| <h2>{t("nav.integrations")}</h2> | ||
| </div> | ||
| <p className="page-sub">{t("integrations.subtitle")}</p> | ||
| {connected && ( | ||
| <section className="notice" aria-label={t("connection.clients.title")}> | ||
| <strong>{t("connection.clients.title")}</strong> | ||
|
|
@@ -139,7 +173,7 @@ export default function Integrations({ apiBase, machineApiBase = apiBase, connec | |
| </section> | ||
| )} | ||
|
|
||
| <div className="page-tabs" role="tablist" aria-label={t("integrations.tabsLabel")}> | ||
| <div className="page-tabs" role="tablist" aria-label={t("integrations.tabsLabel")} id={tablistId}> | ||
| {TABS.map(definition => ( | ||
| <button | ||
| key={definition.id} | ||
|
|
@@ -154,6 +188,7 @@ export default function Integrations({ apiBase, machineApiBase = apiBase, connec | |
| aria-controls={panelDomId(definition.id)} | ||
| tabIndex={tab === definition.id ? 0 : -1} | ||
| className={`page-tab${tab === definition.id ? " page-tab--active" : ""}`} | ||
| hidden={!showSecondary && isSecondary(definition.id)} | ||
| onClick={() => selectTab(definition.id, true)} | ||
| onKeyDown={handleTabKeyDown} | ||
| > | ||
|
|
@@ -164,6 +199,18 @@ export default function Integrations({ apiBase, machineApiBase = apiBase, connec | |
| </button> | ||
| ))} | ||
| </div> | ||
| {secondaryCount > 0 && ( | ||
| <button | ||
| type="button" | ||
| className="btn btn-ghost btn-sm page-tabs-more" | ||
| aria-expanded={showSecondary} | ||
| aria-controls={tablistId} | ||
| disabled={selectedIsSecondary} | ||
| onClick={() => setMoreOpen(open => !open)} | ||
| > | ||
| {t(showSecondary ? "integrations.fewerClients" : "integrations.moreClients", { count: secondaryCount })} | ||
| </button> | ||
| )} | ||
|
|
||
| {TABS.map(definition => { | ||
| if (!mounted.has(definition.id)) return null; | ||
|
|
@@ -177,7 +224,7 @@ export default function Integrations({ apiBase, machineApiBase = apiBase, connec | |
| hidden={!active} | ||
| > | ||
| {definition.id === "overview" && ( | ||
| <IntegrationsOverview apiBase={apiBase} active={active} /> | ||
| <IntegrationsOverview apiBase={apiBase} active={active} statesResource={statesResource} /> | ||
| )} | ||
| {definition.id === "keys" && <ApiKeys apiBase={apiBase} active={active} />} | ||
| {definition.id === "codex" && ( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { useCallback, useEffect, useRef, useState } from "react"; | ||
| import { useDataSurface } from "../../data-surface"; | ||
| import { useDataSurface, type DataSurfaceResource } from "../../data-surface"; | ||
| import { DataSurfaceSkeleton } from "../../components/data-surface"; | ||
| import { navigateHash } from "../../hash-routing"; | ||
| import { useT } from "../../i18n/shared"; | ||
|
|
@@ -169,9 +169,16 @@ function OverviewCard({ | |
| export default function IntegrationsOverview({ | ||
| apiBase, | ||
| active = true, | ||
| statesResource, | ||
| }: { | ||
| apiBase: string; | ||
| active?: boolean; | ||
| /** | ||
| * The file-client state list, owned by the Integrations page (it also drives which | ||
| * tabs are primary). Lifted rather than subscribed twice so there is exactly one | ||
| * owner of the fetch regardless of tab timing. | ||
| */ | ||
| statesResource: DataSurfaceResource<IntegrationStatus[]>; | ||
| }) { | ||
| const t = useT(); | ||
| const [bulkPending, setBulkPending] = useState(false); | ||
|
|
@@ -191,10 +198,6 @@ export default function IntegrationsOverview({ | |
| if (trigger.isConnected) trigger.focus(); | ||
| }, [pendingToggle]); | ||
|
|
||
| const fetchStates = useCallback( | ||
| async (signal: AbortSignal) => (await loadIntegrationStates(apiBase, signal)).clients, | ||
| [apiBase], | ||
| ); | ||
| const fetchHistory = useCallback( | ||
| async (signal: AbortSignal) => (await loadIntegrationJournal(apiBase, undefined, signal)).operations, | ||
| [apiBase], | ||
|
|
@@ -235,12 +238,6 @@ export default function IntegrationsOverview({ | |
| [apiBase], | ||
| ); | ||
|
|
||
| const statesResource = useDataSurface<IntegrationStatus[]>( | ||
| `integration-states:${apiBase}`, | ||
| [apiBase], | ||
| fetchStates, | ||
| { isEmpty: rows => rows.length === 0, enabled: active, sessionCacheKey: `ocx.integrations.states.v1:${apiBase}` }, | ||
| ); | ||
| const historyResource = useDataSurface<IntegrationJournalRow[]>( | ||
| `integration-journal-all:${apiBase}`, | ||
| [apiBase], | ||
|
|
@@ -334,6 +331,23 @@ export default function IntegrationsOverview({ | |
| nativeSettled, | ||
| }); | ||
| const counts = countOverviewRows(rows); | ||
| // Installed (or applied, or not a file client at all) rows are the grid; the rest fold. | ||
| const presentRows = rows.filter(row => row.installed || row.applied || row.status === null); | ||
| const presentIds = new Set(presentRows.map(row => row.id)); | ||
| const absentRows = rows.filter(row => !presentIds.has(row.id)); | ||
|
Comment on lines
+334
to
+337
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If a client is uninstalled after opencodex has written its configuration, the management API can report AGENTS.md reference: gui/AGENTS.md:L9-L10 Useful? React with 👍 / 👎. |
||
| const renderCard = (row: (typeof rows)[number]) => ( | ||
| <OverviewCard | ||
| key={row.id} | ||
| row={row} | ||
| pending={cardPending !== null} | ||
| result={cardResults[row.id] ?? null} | ||
| onOpen={() => navigateHash(row.hash)} | ||
| onToggle={row.toggle ? () => requestToggle(row, !(row.toggleOn ?? row.applied)) : null} | ||
| onOverwrite={row.status !== null && row.status.state === "conflict" && row.installed | ||
| ? () => setPendingOverwrite(row) | ||
| : null} | ||
| /> | ||
| ); | ||
|
|
||
| /* | ||
| * `refresh()` on the resource layer is deliberately fire-and-forget: it | ||
|
|
@@ -416,7 +430,6 @@ export default function IntegrationsOverview({ | |
| : { tone: "err", text: t("integrations.bulk.partial", { clients: failed.join("; ") }) }); | ||
| }; | ||
|
|
||
| const lastChange = history[0]?.at; | ||
|
|
||
| /* | ||
| * The card carries its own switch. Sending the user to a sub-page to flip | ||
|
|
@@ -538,10 +551,6 @@ export default function IntegrationsOverview({ | |
| <strong>{counts.unknown}</strong> | ||
| </div> | ||
| )} | ||
| <div className="integration-summary-cell"> | ||
| <span className="integration-summary-label">{t("integrations.summary.lastChange")}</span> | ||
| <strong>{lastChange ? new Date(lastChange).toLocaleString() : t("integrations.status.unknown")}</strong> | ||
| </div> | ||
| <button | ||
| type="button" | ||
| className="btn btn-ghost" | ||
|
|
@@ -593,21 +602,23 @@ export default function IntegrationsOverview({ | |
| <p className="page-sub">{t("common.loading")}</p> | ||
| ) | ||
| ) : ( | ||
| <ul className="integration-cards"> | ||
| {rows.map(row => ( | ||
| <OverviewCard | ||
| key={row.id} | ||
| row={row} | ||
| pending={cardPending !== null} | ||
| result={cardResults[row.id] ?? null} | ||
| onOpen={() => navigateHash(row.hash)} | ||
| onToggle={row.toggle ? () => requestToggle(row, !(row.toggleOn ?? row.applied)) : null} | ||
| onOverwrite={row.status !== null && row.status.state === "conflict" && row.installed | ||
| ? () => setPendingOverwrite(row) | ||
| : null} | ||
| /> | ||
| ))} | ||
| </ul> | ||
| <> | ||
| <ul className="integration-cards"> | ||
| {presentRows.map(row => renderCard(row))} | ||
| </ul> | ||
| {/* | ||
| Clients that are not on this machine are inventory, not decisions. They stay | ||
| discoverable behind one disclosure instead of doubling the grid. | ||
| */} | ||
| {absentRows.length > 0 && ( | ||
| <details className="integration-cards-more"> | ||
| <summary className="muted text-label">{t("integrations.notInstalled", { count: absentRows.length })}</summary> | ||
| <ul className="integration-cards"> | ||
| {absentRows.map(row => renderCard(row))} | ||
| </ul> | ||
| </details> | ||
| )} | ||
| </> | ||
| )} | ||
| {clientsSettled && installedFileClients.length === 0 && ( | ||
| <div className="integration-empty"> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep file-client tabs visible when the install-state request fails.
failed-coldcurrently counts as settled. The empty fallback at line 79 then makes every file client secondary. If/api/client-integrationsfails before its first successful response, the page hides all file-client tabs without showing a load error.Exclude
failed-coldfromstatesSettled. Keepfailed-with-stalesettled because it has prior data. Add a regression test for an initial failed response.As per coding guidelines, “Keep dashboard behavior aligned with the management API and provider configuration model.” As per path instructions, “Check that GUI state changes stay consistent with the management API responses.”
🤖 Prompt for AI Agents
Sources: Coding guidelines, Path instructions