diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index 31befbee2d..d33af655a8 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -1722,8 +1722,7 @@ "toolCodex": "the Codex CLI", "unavailableClaude": "Claude Code can’t answer @mentions from {{os}} yet: Commonly runs it in a sandbox, and on {{os}} that sandbox can’t run it. Connect Claude Code from your own sessions with the command below, or choose “Run it here” for a hosted agent.", "unavailableCodex": "The Codex CLI can’t answer @mentions from {{os}} yet. Choose “Run it here” for a hosted agent, or run the command on a Mac. Linux is not tested yet.", - "switchToCodex": "I use Codex instead", - "switchToClaude": "I use Claude Code instead" + "useDifferentRunner": "Use a different adapter" }, "mode": { "label": "Where should it run?", diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index 51975b246d..de522bc6a6 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -1782,8 +1782,7 @@ "unavailableClaude": "Claude Code 在 {{os}} 上暂时不能回复 @提及：Commonly 要把它放进沙箱里运行，而 Commonly 用的沙箱在 {{os}} 上还做不到。你可以用下面的命令把 Commonly 的工具接进自己的 Claude Code 会话，或者选择“在这里运行”，改用托管智能体。", "unavailableCodex": "Codex CLI 在 {{os}} 上暂时不能回复 @提及。你可以选择“在这里运行”，改用托管智能体；也可以换到 Mac 机器上运行这条命令（Linux 尚未测试）。", "toolCodex": "Codex CLI", - "switchToCodex": "我用的是 Codex", - "switchToClaude": "我用的是 Claude Code" + "useDifferentRunner": "换一种适配器" }, "mode": { "label": "在哪里运行？", diff --git a/frontend/src/v2/__tests__/V2AgentBYOFromCard.test.tsx b/frontend/src/v2/__tests__/V2AgentBYOFromCard.test.tsx index ae638b33fb..82a03557d2 100644 --- a/frontend/src/v2/__tests__/V2AgentBYOFromCard.test.tsx +++ b/frontend/src/v2/__tests__/V2AgentBYOFromCard.test.tsx @@ -240,10 +240,55 @@ describe('the command page puts only what the paste needs above the copy', () => fireEvent.click(act); await screen.findByTestId('byo-listen'); expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Connect your Claude Code'); - fireEvent.click(screen.getByTestId('byo-listen-adapter-switch')); + // After issue there is no runner switch; the way back to the start is the only control. + expect(screen.queryByTestId('byo-runner')).toBeNull(); + fireEvent.click(screen.getByTestId('byo-use-different-runner')); + // Back from a card lands on the plain page: the picker is there, and the card's name is not carried over. + fireEvent.click(screen.getByTestId('byo-runner-codex')); + expect(screen.getByDisplayValue('sam-codex')).toBeInTheDocument(); + // The second issue needs its own install and token responses; the beforeEach ones are spent. + axios.post + .mockResolvedValueOnce({ data: { ok: true } }) + .mockResolvedValueOnce({ data: { token: 'cm_agent_test_token_codex' } }); + const again = await screen.findByRole('button', { name: /get my connect command/i }); + await waitFor(() => expect(again).toBeEnabled()); + fireEvent.click(again); + await screen.findByTestId('byo-listen'); expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Connect your Codex'); }); + test('a card shows no runner picker before the install either', async () => { + renderAt(CARD); + await screen.findByRole('button', { name: /get my connect command/i }); + expect(screen.queryByTestId('byo-runner')).toBeNull(); + expect(screen.queryByTestId('byo-runner-codex')).toBeNull(); + }); + + // CodeRabbit r4428846350: the lookup kept the card's name as its base, so + // the server's answer overwrote the Codex name the person had just picked. + test('a card, then a different adapter: the second install is named for the runner, not the card', async () => { + renderAt(CARD); + const act = await screen.findByRole('button', { name: /get my connect command/i }); + await waitFor(() => expect(act).toBeEnabled()); + fireEvent.click(act); + await screen.findByTestId('byo-listen'); + fireEvent.click(screen.getByTestId('byo-use-different-runner')); + axios.post + .mockResolvedValueOnce({ data: { ok: true } }) + .mockResolvedValueOnce({ data: { token: 'cm_agent_test_token_codex' } }); + fireEvent.click(screen.getByTestId('byo-runner-codex')); + await waitFor(() => expect(screen.getByDisplayValue('sam-codex')).toBeInTheDocument()); + const again = await screen.findByRole('button', { name: /get my connect command/i }); + await waitFor(() => expect(again).toBeEnabled()); + fireEvent.click(again); + await screen.findByTestId('byo-listen'); + const installs = axios.post.mock.calls.filter(([url]) => url === '/api/registry/install'); + expect(installs[installs.length - 1][1].agentName).toBe('sam-codex'); + const command = Array.from(document.querySelectorAll('.v2-byo__pre')) + .find((el) => /commonly agent run /.test(el.textContent || '')).textContent || ''; + expect(command.split('\n')[3]).toBe('commonly agent run sam-codex --adapter codex'); + }); + test('the key-once and keep-running warnings follow the command', async () => { renderAt(CARD); const act = await screen.findByRole('button', { name: /get my connect command/i }); diff --git a/frontend/src/v2/__tests__/V2AgentBYOListenSnippet.test.tsx b/frontend/src/v2/__tests__/V2AgentBYOListenSnippet.test.tsx index f3d4a89f00..74c9cd221d 100644 --- a/frontend/src/v2/__tests__/V2AgentBYOListenSnippet.test.tsx +++ b/frontend/src/v2/__tests__/V2AgentBYOListenSnippet.test.tsx @@ -47,7 +47,7 @@ const authValue = { updateProfile: jest.fn(), }; -const issueToken = async () => { +const issueToken = async (runner?: 'codex') => { axios.get.mockImplementation((url) => { if (url === '/api/pods') return Promise.resolve({ data: [{ _id: 'p1', name: 'Workspace', type: 'chat' }] }); if (url.startsWith('/api/registry/agents/next-free-name?')) return Promise.resolve({ data: { name: new URLSearchParams(url.split('?')[1]).get('base') } }); @@ -65,6 +65,9 @@ const issueToken = async () => { ); await waitFor(() => expect(screen.getByText('Workspace (chat)')).toBeInTheDocument()); await waitFor(() => expect(screen.getByRole('button', { name: /get my connect command/i })).toBeEnabled()); + if (runner) fireEvent.click(screen.getByTestId(`byo-runner-${runner}`)); + // The runner sets the default name, which re-checks it, so wait for the button again. + if (runner) await waitFor(() => expect(screen.getByRole('button', { name: /get my connect command/i })).toBeEnabled()); fireEvent.click(screen.getByRole('button', { name: /get my connect command/i })); await screen.findByText(/your connect command for/i); }; @@ -168,11 +171,37 @@ describe('BYO listener snippet', () => { expect(listenCommand().compareDocumentPosition(hint) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); }); - test('"I use Codex instead" gives a Codex command and says Codex is what it needs', async () => { - await issueToken(); - fireEvent.click(screen.getByTestId('byo-listen-adapter-switch')); - expect((listenCommand().textContent || '').split('\n')[3]).toMatch(/ --adapter codex$/); + test('picking Codex before the install gives a Codex command named for Codex, and no runner switch after issue', async () => { + await issueToken('codex'); + expect((listenCommand().textContent || '').split('\n')[3]).toMatch(/^commonly agent run sam-codex --adapter codex$/); expect(screen.getByTestId('byo-listen-needs')).toHaveTextContent(/the Codex CLI signed in/); + expect(screen.queryByTestId('byo-runner')).toBeNull(); + }); + + test('issue as Claude, go back, pick Codex, issue again: the second install is -codex on --adapter codex', async () => { + await issueToken(); + expect(axios.post.mock.calls.find(([url]) => url === '/api/registry/install')[1].agentName).toBe('sam-claude'); + fireEvent.click(screen.getByTestId('byo-use-different-runner')); + axios.post + .mockResolvedValueOnce({ data: { ok: true } }) + .mockResolvedValueOnce({ data: { token: 'cm_agent_test_token_codex' } }); + fireEvent.click(screen.getByTestId('byo-runner-codex')); + await waitFor(() => expect(screen.getByRole('button', { name: /get my connect command/i })).toBeEnabled()); + fireEvent.click(screen.getByRole('button', { name: /get my connect command/i })); + await screen.findByText(/your connect command for/i); + const installs = axios.post.mock.calls.filter(([url]) => url === '/api/registry/install'); + expect(installs[installs.length - 1][1].agentName).toBe('sam-codex'); + expect((listenCommand().textContent || '').split('\n')[3]).toMatch(/^commonly agent run sam-codex --adapter codex$/); + }); + + test('after issue, the way back returns to the start and leaves the install alone', async () => { + await issueToken(); + const postsBefore = axios.post.mock.calls.length; + fireEvent.click(screen.getByTestId('byo-use-different-runner')); + expect(screen.getByTestId('byo-runner-claude')).toBeInTheDocument(); + expect(screen.queryByTestId('byo-listen-needs')).toBeNull(); + expect(axios.post.mock.calls.length).toBe(postsBefore); + expect(axios.delete).not.toHaveBeenCalled(); }); test('a Claude Code listener says what it can reach and how --allow widens it; Codex says nothing of folders', async () => { @@ -183,14 +212,20 @@ describe('BYO listener snippet', () => { expect(reach).toHaveTextContent(/add --allow write to the last line\. Those commands can read beyond the folder\./); expect(reach).toHaveTextContent(/To let it fetch web pages, add --allow web\./); expect(reach).toHaveTextContent(/send what it reads off this computer/); - fireEvent.click(screen.getByTestId('byo-listen-adapter-switch')); + }); + + test('a Codex listener shows no Claude folder note', async () => { + await issueToken('codex'); expect(screen.queryByTestId('byo-listen-reach')).toBeNull(); }); test('the Claude Code MCP line shows for Claude Code only', async () => { await issueToken(); expect(screen.getByTestId('byo-mcp-claude')).toBeInTheDocument(); - fireEvent.click(screen.getByTestId('byo-listen-adapter-switch')); + }); + + test('a Codex install shows no Claude Code MCP line', async () => { + await issueToken('codex'); expect(screen.queryByTestId('byo-mcp-claude')).toBeNull(); }); @@ -274,7 +309,7 @@ describe('BYO listener snippet', () => { expect(screen.getByTestId('byo-mcp-claude')).not.toHaveTextContent(/command above/); }); - test('Linux with Claude Code or Codex: no listener command, and the bash MCP command; Codex says it is not tested yet', async () => { + test('Linux with Claude Code: no listener command, and the bash MCP command', async () => { setUserAgent(LINUX); await issueToken(); expect(screen.getByTestId('byo-setup-os-linux')).toHaveAttribute('aria-pressed', 'true'); @@ -284,7 +319,11 @@ describe('BYO listener snippet', () => { expect(screen.queryByTestId('byo-listen-npm-hint')).toBeNull(); expect(mcpBox()).toMatch(/-- npx -y @commonlyai\/mcp$/); expect(screen.getByTestId('byo-mcp-claude')).not.toHaveTextContent(/command above|Optional/); - fireEvent.click(screen.getByTestId('byo-listen-adapter-switch')); + }); + + test('Linux with Codex: no listener command, and Codex says it is not tested yet', async () => { + setUserAgent(LINUX); + await issueToken('codex'); expect(listener()).toBeUndefined(); expect(screen.getByTestId('byo-listen-unavailable')).toHaveTextContent(/The Codex CLI can’t answer @mentions from Linux yet\. .*Linux is not tested yet\./); expect(screen.queryByTestId('byo-listen-npm-hint')).toBeNull(); @@ -292,8 +331,7 @@ describe('BYO listener snippet', () => { test('Codex on Windows says so, and offers hosted or another machine', async () => { setUserAgent(WINDOWS); - await issueToken(); - fireEvent.click(screen.getByTestId('byo-listen-adapter-switch')); + await issueToken('codex'); expect(listener()).toBeUndefined(); expect(screen.getByTestId('byo-listen-unavailable')) .toHaveTextContent(/The Codex CLI can’t answer @mentions from Windows yet\. Choose “Run it here”/); diff --git a/frontend/src/v2/__tests__/V2AgentBYORunnerSwitchRekey.test.tsx b/frontend/src/v2/__tests__/V2AgentBYORunnerSwitchRekey.test.tsx new file mode 100644 index 0000000000..b6b4e2c888 --- /dev/null +++ b/frontend/src/v2/__tests__/V2AgentBYORunnerSwitchRekey.test.tsx @@ -0,0 +1,106 @@ +// @ts-nocheck +// Keep setup request assertions independent of funnel telemetry. +jest.mock('../utils/byoTelemetry', () => ({ recordByoStep: jest.fn() })); +// TASK-645 item 5, Vera's HOLD on #3019: the runner is the one choice that +// names the install, so a runner switch must not leave a key-replacement prompt +// for the other runner's name, and the picker must not move while an issue is in flight. +import React from 'react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import V2AgentBYO from '../components/V2AgentBYO'; +import { AuthContext } from '../../context/AuthContext'; + +jest.mock('../components/V2NavRail', () => () => null); +jest.mock('../hooks/useV2PodAttention', () => ({ + ...jest.requireActual('../hooks/useV2PodAttention'), + useV2PodAttention: () => ({ items: [], count: null, countByPod: {}, refresh: async () => {} }), +})); +jest.mock('axios', () => { + const mock = { + get: jest.fn(), + post: jest.fn(), + patch: jest.fn(), + delete: jest.fn(), + defaults: { baseURL: '', headers: { common: {} } }, + interceptors: { + request: { use: jest.fn(), eject: jest.fn() }, + response: { use: jest.fn(), eject: jest.fn() }, + }, + }; + return { __esModule: true, default: mock, ...mock }; +}); + +const axios = jest.requireMock('axios').default; + +const authValue = { + currentUser: { _id: 'u1', username: 'sam' }, + user: { _id: 'u1', username: 'sam' }, + token: 'user-jwt', + loading: false, + error: null, + isAuthenticated: true, + register: jest.fn(), + login: jest.fn(), + logout: jest.fn(), + updateProfile: jest.fn(), +}; + +const renderPage = () => render( + + + + + , +); + +const tokenCalls = () => axios.post.mock.calls.filter(([url]) => /runtime-tokens/.test(url)); + +// sam-claude already holds a key, so the first issue answers existing:true. +const mockAlreadyKeyed = (tokenReply) => { + axios.get.mockImplementation((url) => { + if (url === '/api/pods') return Promise.resolve({ data: [{ _id: 'p1', name: 'Workspace', type: 'chat' }] }); + if (url.startsWith('/api/registry/agents/next-free-name?')) return Promise.resolve({ data: { name: new URLSearchParams(url.split('?')[1]).get('base') } }); + return Promise.resolve({ data: {} }); + }); + axios.post.mockImplementation((url) => { + if (url === '/api/registry/install') return Promise.reject({ response: { data: { error: 'Agent already installed in this pod' } } }); + if (/runtime-tokens/.test(url)) return tokenReply(); + return Promise.resolve({ data: {} }); + }); +}; + +describe('BYO runner switch and key replacement (TASK-645 item 5, #3019 HOLD)', () => { + beforeEach(() => jest.clearAllMocks()); + + test('switching runner clears the replace-key prompt, so it cannot rotate the other runner’s key', async () => { + mockAlreadyKeyed(() => Promise.resolve({ data: { existing: true } })); + renderPage(); + await waitFor(() => expect(screen.getByRole('button', { name: /get my connect command/i })).toBeEnabled()); + fireEvent.click(screen.getByRole('button', { name: /get my connect command/i })); + await screen.findByTestId('byo-replace-key-confirmation'); + expect(screen.getByTestId('byo-replace-key-confirmation')).toHaveTextContent('sam-claude'); + + fireEvent.click(screen.getByTestId('byo-runner-codex')); + + expect(screen.queryByTestId('byo-replace-key-confirmation')).toBeNull(); + expect(screen.queryByRole('button', { name: /replace the running agent.s key/i })).toBeNull(); + // No force-rotation was sent for sam-claude. + expect(tokenCalls().some(([, body]) => body && body.force)).toBe(false); + }); + + test('the runner picker is disabled while an issue is in flight', async () => { + let release; + mockAlreadyKeyed(() => new Promise((resolve) => { release = resolve; })); + renderPage(); + await waitFor(() => expect(screen.getByRole('button', { name: /get my connect command/i })).toBeEnabled()); + fireEvent.click(screen.getByRole('button', { name: /get my connect command/i })); + await waitFor(() => expect(tokenCalls()).toHaveLength(1)); + + expect(screen.getByTestId('byo-runner-claude')).toBeDisabled(); + expect(screen.getByTestId('byo-runner-codex')).toBeDisabled(); + + release({ data: { existing: true } }); + await screen.findByTestId('byo-replace-key-confirmation'); + expect(screen.getByTestId('byo-runner-codex')).toBeEnabled(); + }); +}); diff --git a/frontend/src/v2/components/V2AgentBYO.tsx b/frontend/src/v2/components/V2AgentBYO.tsx index 7c1057cebb..9bae537b7d 100644 --- a/frontend/src/v2/components/V2AgentBYO.tsx +++ b/frontend/src/v2/components/V2AgentBYO.tsx @@ -239,6 +239,10 @@ const V2AgentBYO: React.FC = () => { // page shows the act and where it lands; the full page is one click away. // The key is still minted by the click, never on load: the click is consent. const [compactByo, setCompactByo] = useState(prefill.fromCard && Boolean(prefill.name)); + // A card fixes the name, and the approval already installed under it, so the + // runner belongs to the card too: no picker on a card. "Use a different + // runner" from a card opens the plain page with nothing carried over. + const [onCardStart, setOnCardStart] = useState(prefill.fromCard); const [hostedState, setHostedState] = useState<'starting' | 'running' | 'slow'>('starting'); // ADR-026 Phase 2: "on my computer" — install + placement request; the // daemon on the chosen machine adopts, provisions, and starts the agent. @@ -316,6 +320,10 @@ const V2AgentBYO: React.FC = () => { } }, [defaultAgentName, nameTouched]); + // A card's name is the lookup's base only while the card is on screen. + // Once the person goes back to the start, the runner names the install. + // Any other link keeps its ?name= prefill as before. + const namePrefill = prefill.fromCard && !onCardStart ? '' : prefill.name; const lastSuggestedName = useRef(null); useEffect(() => { if (!choicesKnown) return undefined; @@ -324,7 +332,7 @@ const V2AgentBYO: React.FC = () => { return undefined; } let cancelled = false; - const base = sanitizeAgentName(prefill.name || defaultAgentName); + const base = sanitizeAgentName(namePrefill || defaultAgentName); setNamesReady(false); setNameLookupError(false); // The pod lets the server hand back this person's own install there whose @@ -343,14 +351,14 @@ const V2AgentBYO: React.FC = () => { const previous = lastSuggestedName.current; lastSuggestedName.current = result.name; setName((current) => ( - !nameTouched || current === prefill.name || current === previous + !nameTouched || current === namePrefill || current === previous ? result.name! : current)); setNamesReady(true); }) .catch(() => { if (!cancelled) setNameLookupError(true); }); return () => { cancelled = true; }; - }, [api, choicesKnown, defaultAgentName, mode, nameLookupRetry, nameTouched, personaCard, podId, prefill.name, - prefill.reconnect]); + }, [api, choicesKnown, defaultAgentName, mode, namePrefill, nameLookupRetry, nameTouched, personaCard, podId, + prefill.reconnect, prefill.name]); const [placed, setPlaced] = useState<{ agentName: string; machineId: string; machineName: string } | null>(null); const [placedState, setPlacedState] = useState<'waiting' | 'running' | 'slow'>('waiting'); const [reconnectRequest, setReconnectRequest] = useState<{ @@ -1031,6 +1039,40 @@ const V2AgentBYO: React.FC = () => { : ''; const cursorInstallHref = cursorMcpInstallLink(apiUrl); + // The runner is picked before the install and never after it: the name + // follows the runner, so a switch after issue would leave a claude-named + // install under a codex listener. Going back to the start is the way to change it. + const backToStart = () => { + if (onCardStart) { + setOnCardStart(false); + setCompactByo(false); + setName(defaultNameFor('byo', listenAdapter)); + setNameTouched(false); + } + setIssued(null); + }; + const runnerPicker = mode === 'byo' && !issued && !onCardStart ? ( +

+ {t('agentByo.form.adapterLabel')}{' '} + {(['claude', 'codex'] as const).map((runner, i) => ( + + {i > 0 && ' · '} + + + ))} +

+ ) : null; + return ( { @@ -1131,6 +1173,7 @@ const V2AgentBYO: React.FC = () => { {/* Enabled once the pod list confirms the pod: a link's pod the person cannot install into is replaced by their own, and that is the line above the button, so the click never lands somewhere unshown. */} + {runnerPicker} {!isOtherOwner &&

{listenerAvailable && (