feat(agents): copy-to-clipboard session ID pill button in agents table#454
feat(agents): copy-to-clipboard session ID pill button in agents table#454
Conversation
|
Bosun CI signal: Bosun-created PR currently has failing checks.
|
There was a problem hiding this comment.
Pull request overview
Adds a “session ID pill” UI affordance to the Agents/Fleet sessions list so users can copy a session ID to the clipboard with visual feedback, along with styling and regression tests.
Changes:
- Render a session-id pill button in
FleetSessionsPanelthat copies the full session id and shows a copied state. - Add pill styling + copied animation to shared UI CSS (mirrored in
ui/andsite/ui/). - Add/extend tests to cover copy behavior and enforce the render/markup pattern.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/tabs/agents.js | Adds clipboard-copy handler + renders the session-id pill in FleetSessionsPanel. |
| site/ui/tabs/agents.js | Mirrors the same pill + copy behavior for the site/ UI build. |
| ui/styles.css | Adds .fleet-session-id-pill styles and copied animation. |
| site/ui/styles.css | Mirrors the same pill styling for the site/ UI build. |
| tests/ui-agents-session-pill.test.mjs | New test validating clipboard copy and copied-state reset via animation end. |
| tests/fleet-tab-render.test.mjs | Adds a source-level assertion ensuring the pill markup/accessibility hooks are present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const copySessionId = (sessionId) => { | ||
| if (!sessionId) return; | ||
| setCopiedSessionId(sessionId); | ||
| navigator.clipboard | ||
| .writeText(sessionId) | ||
| .then(() => showToast("Session ID copied", "success")) | ||
| .catch(() => { | ||
| setCopiedSessionId(""); | ||
| showToast("Copy failed", "error"); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
copySessionId references setCopiedSessionId, but AgentsTab does not define a copiedSessionId state (and this helper is not used anywhere in the file). This will throw if called and is likely leftover/unfinished; either add the missing useState + UI usage, or remove this block from AgentsTab.
| const copySessionId = (sessionId) => { | |
| if (!sessionId) return; | |
| setCopiedSessionId(sessionId); | |
| navigator.clipboard | |
| .writeText(sessionId) | |
| .then(() => showToast("Session ID copied", "success")) | |
| .catch(() => { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| }); | |
| }; |
| ? 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> |
There was a problem hiding this comment.
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).
| navigator.clipboard | ||
| .writeText(sessionId) | ||
| .then(() => showToast("Session ID copied", "success")) | ||
| .catch(() => { | ||
| setCopiedSessionId(""); | ||
| showToast("Copy failed", "error"); | ||
| }); |
There was a problem hiding this comment.
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"); | |
| } |
| 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}`}\"); |
There was a problem hiding this comment.
This assertion line has a syntax error: the string literal is not closed due to an extra backslash/quote (...${sessionId}}"). This will prevent the test file from parsing/running. Fix the string so it’s a valid JS literal and matches the intended source substring.
| expect(source).toContain("aria-label=${`Copy session ID ${sessionId}`}\"); | |
| expect(source).toContain("aria-label={`Copy session ID ${sessionId}`}"); |
| const clipboardWrite = vi.fn().mockResolvedValue(); | ||
| Object.defineProperty(globalThis, "navigator", { | ||
| value: { clipboard: { writeText: clipboardWrite } }, | ||
| configurable: true, | ||
| }); |
There was a problem hiding this comment.
This test overwrites globalThis.navigator but never restores the original value, which can leak into other tests. Capture the prior navigator and restore it in afterEach, or use vi.stubGlobal('navigator', ...) / vi.unstubAllGlobals() to keep test isolation.
| const copySessionId = (sessionId) => { | ||
| if (!sessionId) return; | ||
| setCopiedSessionId(sessionId); | ||
| navigator.clipboard | ||
| .writeText(sessionId) | ||
| .then(() => showToast("Session ID copied", "success")) | ||
| .catch(() => { | ||
| setCopiedSessionId(""); | ||
| showToast("Copy failed", "error"); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
copySessionId references setCopiedSessionId, but AgentsTab does not define a copiedSessionId state (and this helper is not used anywhere in the file). This will throw if called and is likely leftover/unfinished; either add the missing useState + UI usage, or remove this block from AgentsTab.
| const copySessionId = (sessionId) => { | |
| if (!sessionId) return; | |
| setCopiedSessionId(sessionId); | |
| navigator.clipboard | |
| .writeText(sessionId) | |
| .then(() => showToast("Session ID copied", "success")) | |
| .catch(() => { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| }); | |
| }; |
| ? 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> |
There was a problem hiding this comment.
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).
| setCopiedSessionId(sessionId); | ||
| navigator.clipboard | ||
| .writeText(sessionId) | ||
| .then(() => showToast("Session ID copied", "success")) | ||
| .catch(() => { | ||
| setCopiedSessionId(""); | ||
| showToast("Copy failed", "error"); | ||
| }); |
There was a problem hiding this comment.
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.
| setCopiedSessionId(sessionId); | |
| navigator.clipboard | |
| .writeText(sessionId) | |
| .then(() => showToast("Session ID copied", "success")) | |
| .catch(() => { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| }); | |
| // Guard against unsupported / insecure contexts where the Clipboard API | |
| // is unavailable or cannot be used safely. | |
| if (!navigator?.clipboard?.writeText) { | |
| setCopiedSessionId(""); | |
| showToast("Clipboard not available", "error"); | |
| return; | |
| } | |
| setCopiedSessionId(sessionId); | |
| try { | |
| navigator.clipboard | |
| .writeText(sessionId) | |
| .then(() => showToast("Session ID copied", "success")) | |
| .catch(() => { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| }); | |
| } catch (err) { | |
| setCopiedSessionId(""); | |
| showToast("Copy failed", "error"); | |
| } |
…ard-session-id-pill-bu
…ard-session-id-pill-bu
|
Bosun PR classification: Bosun-created.
|
Co-authored-by: bosun-ve[bot] <262908237+bosun-ve[bot]@users.noreply.github.com>
Co-authored-by: bosun-ve[bot] <262908237+bosun-ve[bot]@users.noreply.github.com>
Task-ID: ac13245d-3271-47a3-89b6-ea83f7456eb2\n\nAutomated PR for task ac13245d-3271-47a3-89b6-ea83f7456eb2\n\n---\n\nBosun-Origin: created