fix: resolve final workbench re-review findings

.superpowers/sdd/final-fix-3-report.md

@@ -0,0 +1,32 @@
+# Final Fix 3 Report
+
+## Status
+
+Completed the two final re-review fixes for the Discord-style project workbench UI.
+
+## Fixes
+
+- Made ObjectInspector and CustomLinkChannel clipboard handlers await `navigator.clipboard.writeText`, show success only after resolution, and show distinct unavailable or failure feedback otherwise.
+- Added focused ObjectInspector coverage for delayed success, rejection, and a missing clipboard API, plus CustomLinkChannel unavailable-clipboard coverage.
+- Tightened ServerLogin validation for malformed HTTP slash forms, credentials, missing/repeated/invalid hostname labels, whitespace including encoded whitespace, and unsupported protocols while preserving localhost, IPv4, domain, port, and protocol-less normalization.
+- Added unit and Playwright coverage for the requested malformed server-address cases.
+
+## TDD Evidence
+
+1. Added the regression tests before production changes.
+2. Ran the targeted test set and observed 10 expected failures: six malformed addresses were accepted and clipboard actions reported false success.
+3. Implemented the minimal validation and asynchronous clipboard handling.
+4. Re-ran the targeted test set successfully: 3 files, 26 tests passed.
+
+## Verification
+
+From `apps/web`:
+
+- `npm test -- --run` passed: 8 files, 40 tests.
+- `npm run build` passed.
+- `npx playwright test` passed: 6 tests.
+- `git diff --check` passed.
+
+## Concerns
+
+None. Existing untracked SDD artifacts were left untouched.

apps/web/e2e/project-workbench.spec.ts

@@ -25,17 +25,19 @@ test('login and project channel workbench flow', async ({ page }) => {
   await expect(page.getByText('Open external channel')).toBeVisible();
 });
 
-test('rejects malformed server addresses', async ({ page }) => {
+for (const server of ['https://user:secret@example.com', 'http:////example.com', 'http://example..com', 'http://-bad.com']) {
+  test(`rejects malformed server address: ${server}`, async ({ page }) => {
     await page.goto('/');
 
-  await page.getByLabel('Server IP or domain').fill('https://user:secret@example.com');
+    await page.getByLabel('Server IP or domain').fill(server);
     await page.getByLabel('Email or username').fill('david@example.com');
     await page.getByLabel('Password').fill('secret');
     await page.getByRole('button', { name: 'Log in' }).click();
 
     await expect(page.getByText('Enter a valid HTTP or HTTPS server address.')).toBeVisible();
     await expect(page.getByLabel('Server IP or domain')).toBeVisible();
-});
+  });
+}
 
 test('mobile workbench keeps project navigation and inspector compact', async ({ page }) => {
   await page.setViewportSize({ width: 390, height: 844 });

apps/web/src/features/auth/ServerLogin.svelte

@@ -18,10 +18,19 @@
     }
 
     const candidate = /^https?:\/\//i.test(value) ? value : `http://${value}`;
+    if (!/^https?:\/\/(?![\\/])/i.test(candidate) || hasCredentials(candidate)) {
+      return { error: 'Enter a valid HTTP or HTTPS server address.' };
+    }
 
     try {
       const parsed = new URL(candidate);
-      if (!parsed.hostname || parsed.username || parsed.password || !['http:', 'https:'].includes(parsed.protocol)) {
+      if (
+        !parsed.hostname ||
+        parsed.username ||
+        parsed.password ||
+        !['http:', 'https:'].includes(parsed.protocol) ||
+        !isValidHostname(parsed.hostname)
+      ) {
         return { error: 'Enter a valid HTTP or HTTPS server address.' };
       }
       return { apiBase: candidate };
@@ -49,6 +58,21 @@
     return /^[a-z][a-z\d+.-]*:(?!\d)/i.test(value);
   }
 
+  function hasCredentials(value: string): boolean {
+    const authority = value.replace(/^https?:\/\//i, '').split(/[/?#]/, 1)[0];
+    return authority.includes('@');
+  }
+
+  function isValidHostname(hostname: string): boolean {
+    if (hostname === 'localhost') return true;
+    if (/^\[[\da-f:.]+\]$/i.test(hostname)) return true;
+    if (/^\d+(?:\.\d+){3}$/.test(hostname)) {
+      return hostname.split('.').every((segment) => Number(segment) <= 255);
+    }
+
+    return hostname.split('.').every((label) => /^[a-z\d](?:[a-z\d-]{0,61}[a-z\d])?$/i.test(label));
+  }
+
   function submitLogin() {
     error = '';
     const normalized = normalizeServer(server);

apps/web/src/features/auth/ServerLogin.test.ts

@@ -40,6 +40,24 @@ describe('ServerLogin', () => {
     });
   });
 
+  it.each([
+    ['localhost', 'http://localhost'],
+    ['localhost:8080', 'http://localhost:8080'],
+    ['192.168.1.20:3000', 'http://192.168.1.20:3000'],
+    ['example.com:8443', 'http://example.com:8443'],
+    ['https://example.com', 'https://example.com'],
+  ])('accepts and normalizes a valid server value: %s', async (server, apiBase) => {
+    const onLogin = vi.fn();
+    render(ServerLogin, { props: { apiBase: 'http://localhost:8080', onLogin } });
+
+    await fireEvent.input(screen.getByLabelText('Server IP or domain'), { target: { value: server } });
+    await fireEvent.input(screen.getByLabelText('Email or username'), { target: { value: 'david@example.com' } });
+    await fireEvent.input(screen.getByLabelText('Password'), { target: { value: 'secret' } });
+    await fireEvent.click(screen.getByRole('button', { name: 'Log in' }));
+
+    expect(onLogin).toHaveBeenCalledWith({ apiBase, account: 'david@example.com' });
+  });
+
   it('shows an error when account or password is missing', async () => {
     render(ServerLogin, { props: { apiBase: 'http://localhost:8080' } });
 
@@ -57,6 +75,12 @@ describe('ServerLogin', () => {
     ['https://user:secret@example.com', 'Enter a valid HTTP or HTTPS server address.'],
     ['http://', 'Enter a valid HTTP or HTTPS server address.'],
     ['ftp://example.com', 'Enter a valid HTTP or HTTPS server address.'],
+    ['http:////example.com', 'Enter a valid HTTP or HTTPS server address.'],
+    ['http://example..com', 'Enter a valid HTTP or HTTPS server address.'],
+    ['http://-bad.com', 'Enter a valid HTTP or HTTPS server address.'],
+    ['http://bad-.com', 'Enter a valid HTTP or HTTPS server address.'],
+    ['http://bad_host.com', 'Enter a valid HTTP or HTTPS server address.'],
+    ['http://bad!.com', 'Enter a valid HTTP or HTTPS server address.'],
   ])('rejects an invalid server value: %s', async (server, error) => {
     const onLogin = vi.fn();
     render(ServerLogin, { props: { apiBase: 'http://localhost:8080', onLogin } });

apps/web/src/features/workbench/ObjectInspector.svelte

@@ -4,17 +4,31 @@
   export let item: InspectorItem | null;
   let activeTab: 'discussion' | 'properties' | 'more' = 'discussion';
   let copyFeedback = '';
+  let copyFailed = false;
   let currentItem: InspectorItem | null = null;
 
   $: if (item !== currentItem) {
     currentItem = item;
     copyFeedback = '';
+    copyFailed = false;
   }
 
-  function copyLink() {
+  async function copyLink() {
     const link = new URL(`?item=${encodeURIComponent(item?.title ?? '')}`, window.location.href).toString();
-    void navigator.clipboard?.writeText(link);
+    if (!navigator.clipboard?.writeText) {
+      copyFailed = true;
+      copyFeedback = 'Clipboard access is unavailable.';
+      return;
+    }
+
+    try {
+      await navigator.clipboard.writeText(link);
+      copyFailed = false;
       copyFeedback = 'Link copied to clipboard.';
+    } catch {
+      copyFailed = true;
+      copyFeedback = 'Unable to copy link.';
+    }
   }
 </script>
 
@@ -42,7 +56,7 @@
     {:else}
       <button type="button" on:click={copyLink}>Copy link</button>
       {#if copyFeedback}
-        <p class="copy-feedback" role="status">{copyFeedback}</p>
+        <p class:copy-error={copyFailed} class="copy-feedback" role="status">{copyFeedback}</p>
       {/if}
       <button type="button" aria-label="Archive is unavailable in this preview" title="Archive is unavailable in this preview" disabled>Archive</button>
     {/if}

apps/web/src/features/workbench/ObjectInspector.test.ts

@@ -0,0 +1,73 @@
+import '@testing-library/jest-dom/vitest';
+import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/svelte';
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import ObjectInspector from './ObjectInspector.svelte';
+
+const item = {
+  title: 'Review navigation',
+  type: 'Task',
+  description: 'Check the project navigation flow.',
+  properties: [],
+};
+const originalClipboard = Object.getOwnPropertyDescriptor(navigator, 'clipboard');
+
+function setClipboard(writeText: Clipboard['writeText']) {
+  Object.defineProperty(navigator, 'clipboard', {
+    configurable: true,
+    value: { writeText } as Clipboard,
+  });
+}
+
+afterEach(() => {
+  cleanup();
+  if (originalClipboard) {
+    Object.defineProperty(navigator, 'clipboard', originalClipboard);
+  } else {
+    Reflect.deleteProperty(navigator, 'clipboard');
+  }
+});
+
+describe('ObjectInspector', () => {
+  async function openMoreActions() {
+    await fireEvent.click(screen.getByRole('button', { name: 'More' }));
+  }
+
+  it('shows copy success only after clipboard writing resolves', async () => {
+    let resolveWrite: (() => void) | undefined;
+    const writeText = vi.fn(
+      () =>
+        new Promise<void>((resolve) => {
+          resolveWrite = resolve;
+        }),
+    );
+    setClipboard(writeText);
+    render(ObjectInspector, { props: { item } });
+
+    await openMoreActions();
+    await fireEvent.click(screen.getByRole('button', { name: 'Copy link' }));
+
+    expect(screen.queryByRole('status')).not.toBeInTheDocument();
+    resolveWrite?.();
+    await waitFor(() => expect(screen.getByRole('status')).toHaveTextContent('Link copied to clipboard.'));
+  });
+
+  it('reports a copy failure when clipboard writing rejects', async () => {
+    setClipboard(vi.fn().mockRejectedValue(new Error('Permission denied')));
+    render(ObjectInspector, { props: { item } });
+
+    await openMoreActions();
+    await fireEvent.click(screen.getByRole('button', { name: 'Copy link' }));
+
+    await waitFor(() => expect(screen.getByRole('status')).toHaveTextContent('Unable to copy link.'));
+  });
+
+  it('reports unavailable clipboard access without showing success', async () => {
+    Reflect.deleteProperty(navigator, 'clipboard');
+    render(ObjectInspector, { props: { item } });
+
+    await openMoreActions();
+    await fireEvent.click(screen.getByRole('button', { name: 'Copy link' }));
+
+    expect(screen.getByRole('status')).toHaveTextContent('Clipboard access is unavailable.');
+  });
+});

apps/web/src/features/workbench/ProjectWorkbench.test.ts

@@ -1,6 +1,6 @@
 import '@testing-library/jest-dom/vitest';
-import { fireEvent, render, screen, within } from '@testing-library/svelte';
+import { fireEvent, render, screen, waitFor, within } from '@testing-library/svelte';
-import { describe, expect, it } from 'vitest';
+import { describe, expect, it, vi } from 'vitest';
 import ProjectWorkbench from './ProjectWorkbench.svelte';
 
 describe('ProjectWorkbench', () => {
@@ -88,6 +88,11 @@ describe('ProjectWorkbench', () => {
   });
 
   it('provides Copy link feedback and disables unavailable archive actions', async () => {
+    const originalClipboard = Object.getOwnPropertyDescriptor(navigator, 'clipboard');
+    Object.defineProperty(navigator, 'clipboard', {
+      configurable: true,
+      value: { writeText: vi.fn().mockResolvedValue(undefined) } as Clipboard,
+    });
     render(ProjectWorkbench, { props: { currentUser: { account: 'david@example.com' } } });
     const inspector = screen.getByLabelText('Object inspector');
 
@@ -96,7 +101,13 @@ describe('ProjectWorkbench', () => {
     await fireEvent.click(within(inspector).getByRole('button', { name: 'More' }));
     await fireEvent.click(within(inspector).getByRole('button', { name: 'Copy link' }));
 
-    expect(within(inspector).getByRole('status')).toHaveTextContent('Link copied to clipboard.');
+    await waitFor(() => expect(within(inspector).getByRole('status')).toHaveTextContent('Link copied to clipboard.'));
     expect(within(inspector).getByRole('button', { name: 'Archive is unavailable in this preview' })).toBeDisabled();
+
+    if (originalClipboard) {
+      Object.defineProperty(navigator, 'clipboard', originalClipboard);
+    } else {
+      Reflect.deleteProperty(navigator, 'clipboard');
+    }
   });
 });

apps/web/src/features/workbench/channels/CustomLinkChannel.svelte

@@ -3,13 +3,22 @@
 
   export let channel: WorkbenchChannel;
   let copyStatus = '';
+  let copyFailed = false;
 
   async function copyUrl() {
+    if (!navigator.clipboard?.writeText) {
+      copyFailed = true;
+      copyStatus = 'Copy unavailable in this browser.';
+      return;
+    }
+
     try {
-      await navigator.clipboard?.writeText(channel.url ?? '');
+      await navigator.clipboard.writeText(channel.url ?? '');
+      copyFailed = false;
       copyStatus = 'URL copied.';
     } catch {
-      copyStatus = 'Copy unavailable in this browser.';
+      copyFailed = true;
+      copyStatus = 'Unable to copy URL.';
     }
   }
 </script>
@@ -20,6 +29,6 @@
   <a class="custom-link" href={channel.url} target="_blank" rel="noreferrer">Open external channel</a>
   <button class="copy-url-button" type="button" on:click={copyUrl}>Copy URL</button>
   {#if copyStatus}
-    <p class="copy-feedback" aria-live="polite">{copyStatus}</p>
+    <p class:copy-error={copyFailed} class="copy-feedback" aria-live="polite">{copyStatus}</p>
   {/if}
 </section>

apps/web/src/features/workbench/channels/CustomLinkChannel.test.ts

@@ -0,0 +1,36 @@
+import '@testing-library/jest-dom/vitest';
+import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/svelte';
+import { afterEach, describe, expect, it } from 'vitest';
+import CustomLinkChannel from './CustomLinkChannel.svelte';
+
+const channel = {
+  id: 'project-custom-link',
+  projectID: 1,
+  type: 'custom_link' as const,
+  title: 'Roadmap Board',
+  icon: 'link',
+  url: 'https://example.com/roadmap',
+  sortOrder: 1,
+};
+const originalClipboard = Object.getOwnPropertyDescriptor(navigator, 'clipboard');
+
+afterEach(() => {
+  cleanup();
+  if (originalClipboard) {
+    Object.defineProperty(navigator, 'clipboard', originalClipboard);
+  } else {
+    Reflect.deleteProperty(navigator, 'clipboard');
+  }
+});
+
+describe('CustomLinkChannel', () => {
+  it('reports unavailable clipboard access without showing URL copied', async () => {
+    Reflect.deleteProperty(navigator, 'clipboard');
+    render(CustomLinkChannel, { props: { channel } });
+
+    await fireEvent.click(screen.getByRole('button', { name: 'Copy URL' }));
+
+    await waitFor(() => expect(screen.getByText('Copy unavailable in this browser.')).toBeInTheDocument());
+    expect(screen.queryByText('URL copied.')).not.toBeInTheDocument();
+  });
+});

apps/web/src/index.css

@@ -253,6 +253,7 @@ button:disabled { cursor: not-allowed; opacity: 0.55; }
 .custom-link { color: var(--primary-strong); font-weight: 600; width: max-content; }
 .copy-url-button { width: max-content; }
 .copy-feedback { margin: 0; color: var(--success); font-size: 0.8125rem; }
+.copy-feedback.copy-error { color: var(--danger); }
 
 .object-inspector {
   align-content: start;