-
Notifications
You must be signed in to change notification settings - Fork 22
feat(agents): copy-to-clipboard session ID pill button in agents table #454
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 6 commits
dd9cc2f
83f4b88
5bc7d01
f8cdc68
482576c
f4f4ace
da7129b
19475c6
78682d0
effafc4
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 |
|---|---|---|
|
|
@@ -2210,10 +2210,23 @@ function FleetSessionsPanel({ slots, taskFallbackEntries = [], onOpenWorkspace, | |
| const [sessionScope, setSessionScope] = useState(FLEET_SESSION_SCOPE.all); | ||
| const [sessionSearch, setSessionSearch] = useState(""); | ||
| const [selectedEntryKey, setSelectedEntryKey] = useState(null); | ||
| const [copiedSessionId, setCopiedSessionId] = useState(""); | ||
| const [logText, setLogText] = useState("(no logs yet)"); | ||
| const logRef = useRef(null); | ||
| const allSessions = sessionsData.value || []; | ||
|
|
||
| const copySessionId = (sessionId) => { | ||
| if (!sessionId) return; | ||
| setCopiedSessionId(sessionId); | ||
| navigator.clipboard | ||
| .writeText(sessionId) | ||
| .then(() => showToast("Session ID copied", "success")) | ||
| .catch(() => { | ||
| setCopiedSessionId(""); | ||
| showToast("Copy failed", "error"); | ||
| }); | ||
| }; | ||
|
|
||
| /* Stabilise entries with useMemo so the reference only changes when the | ||
| underlying data actually changes – prevents infinite render loops that | ||
| previously caused "insertBefore" DOM errors. */ | ||
|
|
@@ -2506,6 +2519,23 @@ function FleetSessionsPanel({ slots, taskFallbackEntries = [], onOpenWorkspace, | |
| <span class=${`fleet-slot-state-badge ${isFleetEntryActive(entry) ? "active" : "historic"}`}> | ||
| ${entryStatus || "unknown"} | ||
| </span> | ||
| ${sessionId | ||
| ? html`<button | ||
| type="button" | ||
| class="fleet-session-id-pill" | ||
| data-session-id=${sessionId} | ||
| data-copied=${copiedSessionId === sessionId ? "true" : "false"} | ||
| aria-label=${`Copy session ID ${sessionId}`} | ||
| onClick=${() => copySessionId(sessionId)} | ||
| onAnimationEnd=${(event) => { | ||
| if (event?.target !== event?.currentTarget) return; | ||
| if (copiedSessionId === sessionId) setCopiedSessionId(""); | ||
| }} | ||
| > | ||
| <span class="fleet-session-id-pill-text mono">${sessionId.slice(0, 8)}</span> | ||
| <span class="fleet-session-id-pill-icon" aria-hidden="true">${copiedSessionId === sessionId ? "✓" : ICONS.copy}</span> | ||
|
Comment on lines
+2534
to
+2547
|
||
| </button>` | ||
| : null} | ||
| ${entry.slot?.branch | ||
| ? html`<span class="fleet-slot-meta-branch">${entry.slot.branch}</span>` | ||
| : entry.session?.branch | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -31,6 +31,16 @@ const sessionListSourceFiles = [ | |||||
|
|
||||||
| for (const { relPath, source } of sourceFiles) { | ||||||
| describe(`FleetSessionsPanel render stability (${relPath})`, () => { | ||||||
| it("renders a keyboard-accessible session id pill with copy feedback state", () => { | ||||||
| expect(source).toContain("fleet-session-id-pill"); | ||||||
| expect(source).toContain("type=\"button\""); | ||||||
| expect(source).toContain("aria-label=${`Copy session ID ${sessionId}`}\"); | ||||||
|
||||||
| expect(source).toContain("aria-label=${`Copy session ID ${sessionId}`}\"); | |
| expect(source).toContain("aria-label={`Copy session ID ${sessionId}`}"); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import { afterEach, describe, expect, it, vi } from "vitest"; | ||
| import { h } from "preact"; | ||
| import { cleanup, fireEvent, render } from "@testing-library/preact"; | ||
|
Check failure on line 3 in tests/ui-agents-session-pill.test.mjs
|
||
| import htm from "htm"; | ||
|
|
||
| const html = htm.bind(h); | ||
|
|
||
| vi.mock("../ui/modules/telegram.js", () => ({ | ||
| haptic: vi.fn(), | ||
| showConfirm: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("../ui/modules/api.js", () => ({ | ||
| apiFetch: vi.fn(() => Promise.resolve({ data: [] })), | ||
| sendCommandToChat: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("../ui/modules/state.js", async () => { | ||
| const actual = await vi.importActual("../ui/modules/state.js"); | ||
| return { | ||
| ...actual, | ||
| executorData: { | ||
| value: { | ||
| data: { | ||
| slots: [ | ||
| { | ||
| taskId: "task-1", | ||
| taskTitle: "Agent task", | ||
| branch: "feature/test", | ||
| status: "busy", | ||
| sessionId: "12345678-1234-1234-1234-1234567890ab", | ||
| startedAt: "2026-03-21T00:00:00.000Z", | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| }, | ||
| showToast: vi.fn(), | ||
| scheduleRefresh: vi.fn(), | ||
| refreshTab: vi.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| vi.mock("../ui/components/session-list.js", () => ({ | ||
| loadSessions: vi.fn(), | ||
| loadSessionMessages: vi.fn(), | ||
| selectedSessionId: { value: null }, | ||
| sessionsData: { | ||
| value: [ | ||
| { | ||
| id: "12345678-1234-1234-1234-1234567890ab", | ||
| taskId: "task-1", | ||
| title: "Agent task", | ||
| branch: "feature/test", | ||
| status: "active", | ||
| createdAt: "2026-03-21T00:00:00.000Z", | ||
| lastActiveAt: "2026-03-21T00:00:10.000Z", | ||
| }, | ||
| ], | ||
| }, | ||
| sessionMessages: { value: [] }, | ||
| sessionMessagesSessionId: { value: null }, | ||
| })); | ||
|
|
||
| vi.mock("../ui/components/chat-view.js", () => ({ ChatView: () => html`<div />` })); | ||
| vi.mock("../ui/components/diff-viewer.js", () => ({ DiffViewer: () => html`<div />` })); | ||
|
|
||
| describe("agents session ID pill", () => { | ||
| afterEach(() => { | ||
| cleanup(); | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it("copies the full session id and clears copied state after animation end", async () => { | ||
| const clipboardWrite = vi.fn().mockResolvedValue(); | ||
| Object.defineProperty(globalThis, "navigator", { | ||
| value: { clipboard: { writeText: clipboardWrite } }, | ||
| configurable: true, | ||
| }); | ||
|
||
|
|
||
| const mod = await import("../ui/tabs/agents.js"); | ||
| const FleetSessionsTab = mod.FleetSessionsTab; | ||
| const { findByRole } = render(html`<${FleetSessionsTab} />`); | ||
| const pill = await findByRole("button", { name: /copy session id 12345678-1234-1234-1234-1234567890ab/i }); | ||
|
|
||
| await fireEvent.click(pill); | ||
| expect(clipboardWrite).toHaveBeenCalledWith("12345678-1234-1234-1234-1234567890ab"); | ||
| expect(pill.getAttribute("data-copied")).toBe("true"); | ||
|
|
||
| fireEvent.animationEnd(pill); | ||
| expect(pill.getAttribute("data-copied")).toBe("false"); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1408,6 +1408,18 @@ export function AgentsTab() { | |||||||||||||||||||||||||||||||||||||||||||||||||
| }, [slots, fleetSearch]); | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const allSessions = sessionsData.value || []; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| const copySessionId = (sessionId) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
github-code-quality[bot] marked this conversation as resolved.
Fixed
Show fixed
Hide fixed
github-code-quality[bot] marked this conversation as resolved.
Fixed
Show fixed
Hide fixed
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!sessionId) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setCopiedSessionId(sessionId); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| navigator.clipboard | ||||||||||||||||||||||||||||||||||||||||||||||||||
| .writeText(sessionId) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| .then(() => showToast("Session ID copied", "success")) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| .catch(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| setCopiedSessionId(""); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| showToast("Copy failed", "error"); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| const copySessionId = (sessionId) => { | |
| if (!sessionId) return; | |
| setCopiedSessionId(sessionId); | |
| navigator.clipboard | |
| .writeText(sessionId) | |
| .then(() => showToast("Session ID copied", "success")) | |
| .catch(() => { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| }); | |
| }; |
Outdated
Copilot
AI
Mar 26, 2026
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.
copySessionId calls navigator.clipboard.writeText without checking that the Clipboard API is available. In unsupported/insecure contexts this can throw synchronously (bypassing the Promise .catch) and will also leave data-copied stuck. Add a navigator?.clipboard?.writeText guard (consistent with other clipboard code in this file) and reset copiedSessionId / toast appropriately when unavailable.
| navigator.clipboard | |
| .writeText(sessionId) | |
| .then(() => showToast("Session ID copied", "success")) | |
| .catch(() => { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| }); | |
| const writeText = navigator?.clipboard?.writeText; | |
| if (!writeText) { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| return; | |
| } | |
| try { | |
| writeText.call(navigator.clipboard, sessionId) | |
| .then(() => showToast("Session ID copied", "success")) | |
| .catch(() => { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| }); | |
| } catch (_err) { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| } |
Copilot
AI
Mar 26, 2026
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.
The session-id pill is rendered as a native <button> inside an MUI <Button> (which renders as a <button> by default). Nested buttons are invalid HTML and browsers will implicitly close the outer button, breaking click/keyboard behavior and accessibility. Refactor so the row container is not a <button> when it contains this pill (e.g., make the row a <div>/ListItemButton with appropriate keyboard handling, or move copy handling onto a non-nested element).
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.
copySessionIdcallsnavigator.clipboard.writeTextwithout checking that the Clipboard API is available. In unsupported/insecure contexts this can throw synchronously (bypassing the Promise.catch) and will also leavedata-copiedstuck. Add anavigator?.clipboard?.writeTextguard (consistent with other clipboard code in this file) and resetcopiedSessionId/ toast appropriately when unavailable.