diff --git a/backend/__tests__/service/byo-next-free-name.test.js b/backend/__tests__/service/byo-next-free-name.test.js index 7a07df8be..385b75085 100644 --- a/backend/__tests__/service/byo-next-free-name.test.js +++ b/backend/__tests__/service/byo-next-free-name.test.js @@ -33,6 +33,7 @@ describe('BYO next-free-name lookup', () => { beforeEach(async () => { await AgentInstallation.deleteMany({}); await Pod.deleteMany({}); + await User.deleteMany({ isBot: true }); }); const pod = async (name, memberIds, publicRead = false) => Pod.create({ @@ -105,6 +106,108 @@ describe('BYO next-free-name lookup', () => { expect(row.params.name).toBe('sam-codex'); }); + const webhookInstall = (target, name, installedBy = owner._id, extra = {}) => AgentInstallation.create({ + podId: target._id, agentName: name, instanceId: 'default', version: '1.0.0', installedBy, status: 'active', + config: { runtime: { runtimeType: 'webhook' } }, ...extra, + }); + const agentUserWithKey = (name) => User.create({ + username: name, email: `${name}@agents.test`, password: 'password123', isBot: true, + botMetadata: { agentName: name, instanceId: 'default' }, + agentRuntimeTokens: [{ tokenHash: `hash-${name}`, label: 'BYO MCP — initial issue', createdAt: new Date() }], + }); + const suggest = async (base, podId) => { + const response = await request(app) + .get(`/api/registry/agents/next-free-name?base=${base}${podId ? `&podId=${podId}` : ''}`) + .set('Authorization', `Bearer ${token}`); + expect(response.status).toBe(200); + return response.body.name; + }; + + it('hands back the caller’s own BYO install whose key was never issued, so a reopen does not mint a second', async () => { + // Path 1: the install succeeded, then the runtime-tokens request failed or + // was abandoned. No agent user exists yet, or one exists with no key. + const first = await pod('one', [owner._id]); + await webhookInstall(first, 'sam-claude'); + expect(await suggest('sam-claude', first._id)).toBe('sam-claude'); + + await User.create({ + username: 'sam-claude', email: 'sam-claude@agents.test', password: 'password123', isBot: true, + botMetadata: { agentName: 'sam-claude', instanceId: 'default' }, agentRuntimeTokens: [], + }); + expect(await suggest('sam-claude', first._id)).toBe('sam-claude'); + }); + + it('hands the name back only for the pod being installed into', async () => { + // Elsewhere the name would put the same identity into a second pod. + const first = await pod('one', [owner._id]); + const second = await pod('two', [owner._id]); + await webhookInstall(first, 'sam-claude'); + expect(await suggest('sam-claude', second._id)).toBe('sam-claude-2'); + expect(await suggest('sam-claude')).toBe('sam-claude-2'); + }); + + it('keeps the name taken when the agent already holds a key from another pod', async () => { + // Path 2: runtime-tokens answered existing:true and the person backed out at + // "replace the running key". That agent has a working key, so its name is + // not free, and suggesting it would steer the next click into a replacement. + const first = await pod('one', [owner._id]); + const second = await pod('two', [owner._id]); + await webhookInstall(first, 'sam-claude'); + await webhookInstall(second, 'sam-claude'); + await agentUserWithKey('sam-claude'); + expect(await suggest('sam-claude', second._id)).toBe('sam-claude-2'); + }); + + it('keeps the name taken when the one install in this pod already has a keyed agent', async () => { + // The agent-user key check alone decides this: the install is the caller's + // own webhook install in the selected pod, with no key on the row. + const first = await pod('one', [owner._id]); + await webhookInstall(first, 'sam-claude'); + await agentUserWithKey('sam-claude'); + expect(await suggest('sam-claude', first._id)).toBe('sam-claude-2'); + }); + + it('keeps the name taken when the keyless own install is on a non-default instance', async () => { + // The install route derives the default instance for these clicks, so + // handing this name back would create a second installation beside it. + const first = await pod('one', [owner._id]); + await webhookInstall(first, 'sam-claude', owner._id, { instanceId: 'work' }); + expect(await suggest('sam-claude', first._id)).toBe('sam-claude-2'); + }); + + it('the connect card gets the same keyless-own-install exemption as the page', async () => { + // Without the exemption both would skip sam-claude-2 and land on -3. + const here = await pod('here', [owner._id]); + const elsewhere = await pod('elsewhere', [owner._id]); + await install(elsewhere, 'sam-claude'); + await webhookInstall(here, 'sam-claude-2'); + expect(await suggest('sam-claude', here._id)).toBe('sam-claude-2'); + + const result = await proposeAction({ + podId: String(here._id), + agentName: 'scout', + instanceId: 'default', + displayName: 'Scout', + actionType: 'connect_local_agent', + params: { name: 'sam-claude', adapter: 'claude' }, + summary: "Connect your own agent as 'sam-claude'", + ownerUserId: owner._id, + }); + expect(result.ok).toBe(true); + const row = await ApprovalAction.findById(result.approvalId).lean(); + expect(row.params.name).toBe('sam-claude-2'); + }); + + it('keeps names taken that are not the caller’s own unissued webhook install', async () => { + const shared = await pod('shared', [owner._id, other._id]); + await webhookInstall(shared, 'sam-claude', other._id); // someone else's + await install(shared, 'sam-claude-2'); // not a webhook install + await webhookInstall(shared, 'sam-claude-3', owner._id, { // a legacy key on the installation + runtimeTokens: [{ tokenHash: 'legacy', label: 'legacy', createdAt: new Date() }], + }); + expect(await suggest('sam-claude', shared._id)).toBe('sam-claude-4'); + }); + it('requires authentication and a valid base', async () => { expect((await request(app).get('/api/registry/agents/next-free-name?base=sam-claude')).status).toBe(401); const invalid = await request(app) diff --git a/backend/__tests__/service/runtimeTokenSiblingLegacyKey.test.js b/backend/__tests__/service/runtimeTokenSiblingLegacyKey.test.js new file mode 100644 index 000000000..e84352e4c --- /dev/null +++ b/backend/__tests__/service/runtimeTokenSiblingLegacyKey.test.js @@ -0,0 +1,71 @@ +// A legacy runtime key can sit on another installation of the same identity +// (#2917). Issuing must treat it as existing, so no second live key is minted, +// and must only record the caller as its owner when the caller installed that +// sibling. Real mongod: the sibling lookup relies on a case-insensitive +// collation, which a mocked model cannot exercise. +const mongoose = require('mongoose'); +const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../utils/testUtils'); + +process.env.PG_HOST = ''; +process.env.NODE_ENV = 'test'; + +const Pod = require('../../models/Pod'); +const User = require('../../models/User'); +const AgentCredential = require('../../models/AgentCredential'); +const { AgentInstallation } = require('../../models/AgentRegistry'); +const { issueRuntimeTokenForAgent } = require('../../routes/registry/tokens'); + +describe('issueRuntimeTokenForAgent with a legacy key on a sibling installation', () => { + let caller; + let stranger; + let podA; + let podB; + + beforeAll(async () => { await setupMongoDb(); }); + afterAll(async () => { await closeMongoDb(); }); + + beforeEach(async () => { + await clearMongoDb(); + caller = await User.create({ username: 'sib-caller', email: 'sib-caller@test.com', password: 'password123' }); + stranger = await User.create({ username: 'sib-stranger', email: 'sib-stranger@test.com', password: 'password123' }); + podA = await Pod.create({ name: 'a', type: 'chat', createdBy: caller._id, members: [caller._id] }); + podB = await Pod.create({ name: 'b', type: 'chat', createdBy: stranger._id, members: [stranger._id] }); + }); + + const installOf = (pod, instanceId, installedBy, runtimeTokens = []) => AgentInstallation.create({ + podId: pod._id, agentName: 'sam-claude', instanceId, version: '1.0.0', installedBy, status: 'active', + config: { runtime: { runtimeType: 'webhook' } }, runtimeTokens, + }); + const agentUser = () => User.create({ + username: 'sam-claude-bot', email: 'sam-claude-bot@agents.test', password: 'password123', isBot: true, + botMetadata: { agentName: 'sam-claude', instanceId: 'work' }, agentRuntimeTokens: [], + }); + const legacy = (tokenHash) => [{ tokenHash, label: 'legacy', createdAt: new Date() }]; + + it('finds a sibling whose instanceId differs only in case', async () => { + await installOf(podB, 'Work', caller._id, legacy('legacy-mixed-case')); + const installation = await installOf(podA, 'work', caller._id); + const result = await issueRuntimeTokenForAgent(await agentUser(), 'BYO', installation, { ownerUserId: caller._id }); + expect(result).toMatchObject({ existing: true }); + expect(result.token).toBeUndefined(); + }); + + it('records the caller as owner only of a sibling key the caller installed', async () => { + await installOf(podB, 'work', caller._id, legacy('own-legacy')); + const installation = await installOf(podA, 'work', caller._id); + const result = await issueRuntimeTokenForAgent(await agentUser(), 'BYO', installation, { ownerUserId: caller._id }); + expect(result).toMatchObject({ existing: true }); + const row = await AgentCredential.findOne({ tokenHash: 'own-legacy' }).lean(); + expect(String(row.ownerUserId)).toBe(String(caller._id)); + }); + + it('blocks a second mint on another person\'s sibling key without claiming it', async () => { + await installOf(podB, 'work', stranger._id, legacy('stranger-legacy')); + const installation = await installOf(podA, 'work', caller._id); + const result = await issueRuntimeTokenForAgent(await agentUser(), 'BYO', installation, { ownerUserId: caller._id }); + expect(result).toMatchObject({ existing: true }); + expect(result.token).toBeUndefined(); + expect(await AgentCredential.exists({ tokenHash: 'stranger-legacy' })).toBeNull(); + expect(await AgentCredential.countDocuments({ ownerUserId: new mongoose.Types.ObjectId(String(caller._id)) })).toBe(0); + }); +}); diff --git a/backend/__tests__/unit/routes/registry.runtime-tokens.test.js b/backend/__tests__/unit/routes/registry.runtime-tokens.test.js index 1a5ae6cf2..103e66a4c 100644 --- a/backend/__tests__/unit/routes/registry.runtime-tokens.test.js +++ b/backend/__tests__/unit/routes/registry.runtime-tokens.test.js @@ -233,6 +233,45 @@ describe('agent runtime tokens', () => { expect(res.body.token).toBeUndefined(); }); + it('returns {existing:true} when another installation of the identity holds the only legacy key', async () => { + // BYO now issues after "already installed". A key on a sibling installation + // is invisible to the issue itself, so minting would leave two live keys. + Pod.findById.mockReturnValue({ + lean: jest.fn().mockResolvedValue({ _id: 'pod-1', createdBy: 'user-1', members: ['user-1'] }), + }); + const installation = { + agentName: 'Sam-Claude', podId: 'pod-1', instanceId: 'default', displayName: 'sam-claude', + status: 'active', runtimeTokens: [], save: jest.fn().mockResolvedValue(true), + }; + const agentUser = { agentRuntimeTokens: [], save: jest.fn().mockResolvedValue(true) }; + AgentInstallation.findOne.mockResolvedValue(installation); + let siblingQuery; + AgentInstallation.find.mockImplementation((query) => { + if (query['runtimeTokens.0']) siblingQuery = query; + const rows = query['runtimeTokens.0'] + ? [{ runtimeTokens: [{ tokenHash: 'legacy-hash', label: 'legacy', createdAt: new Date() }] }] + : []; + return { + collation: jest.fn(function collation() { return this; }), + select: jest.fn(() => ({ lean: jest.fn().mockResolvedValue(rows) })), + }; + }); + AgentIdentityService.getOrCreateAgentUser.mockResolvedValue(agentUser); + AgentIdentityService.ensureAgentInPod.mockResolvedValue(true); + + const res = await request(app) + .post('/api/registry/pods/pod-1/agents/sam-claude/runtime-tokens') + .send({ label: 'BYO MCP — initial issue' }); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ existing: true }); + expect(res.body.token).toBeUndefined(); + expect(agentUser.save).not.toHaveBeenCalled(); + expect(siblingQuery).toEqual({ + agentName: 'Sam-Claude', instanceId: 'default', 'runtimeTokens.0': { $exists: true }, + }); + }); + it('with force:true, clears existing tokens and mints a fresh one (detach + reattach race fix)', async () => { // ADR-005 detach + reattach: CLI deletes its local token file, but the // server-side hashed copy on the agent User row persists. Without diff --git a/backend/routes/registry/catalog.ts b/backend/routes/registry/catalog.ts index 1a88c9908..df14683e0 100644 --- a/backend/routes/registry/catalog.ts +++ b/backend/routes/registry/catalog.ts @@ -51,13 +51,14 @@ catalogRouter.get('/agents/next-free-name', catalogRateLimit, auth, async (req: } const memberId = new mongoose.Types.ObjectId(String(userId)); const base = req.query.base; + const selectedPodId = typeof req.query.podId === 'string' ? req.query.podId : ''; if (typeof base !== 'string' || base.length < 1 || base.length > 64 || !/^[a-z0-9-]+$/.test(base)) { return res.status(400).json({ error: 'Invalid agent name' }); } // The shared rule: the connect card proposes the same name (TASK-329). - const name = await nextFreeByoName(memberId, base); + const name = await nextFreeByoName(memberId, base, { podId: selectedPodId }); return res.json({ name }); } catch (error) { console.error('Error suggesting BYO agent name:', error); diff --git a/backend/routes/registry/tokens.ts b/backend/routes/registry/tokens.ts index 00ada1ef0..97720a118 100644 --- a/backend/routes/registry/tokens.ts +++ b/backend/routes/registry/tokens.ts @@ -289,13 +289,40 @@ const issueRuntimeTokenForAgent = async (agentUser: any, label: any, installatio await refuseDeletingOwner(); if (await runsInProcess(agentUser, installation)) throw new InProcessAgentTokenRefused(); // Check if agent already has a runtime token (reuse existing) - const existingToken = agentUser.agentRuntimeTokens?.[0] || installation?.runtimeTokens?.[0]; + // A legacy key can also sit on another installation of the same identity. + // Minting beside it would leave two live keys, so it counts as existing and + // the caller must ask for an explicit (forced) replacement, which revokes + // every copy first. + const siblingLegacyToken = async () => { + if (!installation?.agentName) return null; + // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports + const { AgentInstallation } = require('../../models/AgentRegistry'); + const siblings = await AgentInstallation.find({ + agentName: installation.agentName, + instanceId: installation.instanceId || 'default', + 'runtimeTokens.0': { $exists: true }, + }).collation({ locale: 'en', strength: 2 }).select('runtimeTokens installedBy').lean(); + const sibling = (siblings || []).find((entry: any) => entry.runtimeTokens?.length); + return sibling ? { token: sibling.runtimeTokens[0], installedBy: sibling.installedBy } : null; + }; + let existingToken = agentUser.agentRuntimeTokens?.[0] || installation?.runtimeTokens?.[0]; + // A sibling's key still blocks a second mint, whoever installed it. But only + // a sibling this issuer installed may record the issuer as the key's owner: + // another person's key backfilled under the caller would be revoked when the + // caller's account is deleted. + let backfillOwner = Boolean(existingToken); + if (!existingToken) { + const sibling = await siblingLegacyToken(); + existingToken = sibling?.token || null; + backfillOwner = Boolean(sibling && issuer?.ownerUserId + && String(sibling.installedBy) === String(issuer.ownerUserId)); + } if (existingToken) { // Backfill a credential row for the pre-substrate token so the existing // fleet becomes listable and cascade-revocable (#1312: without // this the collection stays near-empty while coverage looks fine). // Provenance is unknown, so no parent lineage is claimed. - if (issuer?.ownerUserId) { + if (issuer?.ownerUserId && backfillOwner) { // eslint-disable-next-line global-require, @typescript-eslint/no-require-imports const AgentCredential = require('../../models/AgentCredential'); await AgentCredential.updateOne( diff --git a/backend/services/approvalActionService.ts b/backend/services/approvalActionService.ts index e23dbbe7c..dcadd10b0 100644 --- a/backend/services/approvalActionService.ts +++ b/backend/services/approvalActionService.ts @@ -489,7 +489,9 @@ export const proposeAction = async (options: ProposeOptions): Promise-2" makes the next click mint a second install and orphans + * the first. Only in the pod being installed into: elsewhere the name would + * put the same identity into a second pod. No pod given, no exemption. And + * only an install on the `default` instance: that is the instance the install + * route derives when the display name is the agent's own name, which is how + * both callers install. A keyless install on any other instance keeps its + * name taken, or the next click would create a second installation beside it. */ -export const nextFreeByoName = async (userId: unknown, base: string): Promise => { +export const nextFreeByoName = async ( + userId: unknown, + base: string, + { podId }: { podId?: string } = {}, +): Promise => { const pods = await Pod.find({ members: userId, type: { $nin: ['agent-room', 'agent-dm', 'agent-admin'] }, @@ -19,8 +37,24 @@ export const nextFreeByoName = async (userId: unknown, base: string): Promise pod._id) }, status: 'active', - }).select('agentName').lean() : []; - const taken = new Set(installations.map((row: any) => String(row.agentName).toLowerCase())); + }).select('agentName instanceId installedBy config runtimeTokens podId').lean() : []; + const selectedPodId = podId ? String(podId) : ''; + const unissued = await Promise.all(installations.map(async (row: any) => { + if (!selectedPodId || String(row.podId) !== selectedPodId) return false; + if (String(row.installedBy) !== String(userId)) return false; + if (row.config?.runtime?.runtimeType !== 'webhook') return false; + if (row.runtimeTokens?.length) return false; + if ((row.instanceId || 'default') !== 'default') return false; + const username = AgentIdentityService.buildAgentUsername( + AgentIdentityService.resolveAgentType(row.agentName), + 'default', + ); + const agentUser: any = await User.findOne({ username }).select('agentRuntimeTokens').lean(); + return !agentUser?.agentRuntimeTokens?.length; + })); + const taken = new Set(installations + .filter((_row: any, index: number) => !unissued[index]) + .map((row: any) => String(row.agentName).toLowerCase())); let name = base; for (let suffix = 2; taken.has(name); suffix += 1) { name = `${base.slice(0, 64 - String(suffix).length - 1)}-${suffix}`; diff --git a/frontend/src/v2/__tests__/V2AgentBYOHosted.test.tsx b/frontend/src/v2/__tests__/V2AgentBYOHosted.test.tsx index 45220a6f4..52b9bb3cf 100644 --- a/frontend/src/v2/__tests__/V2AgentBYOHosted.test.tsx +++ b/frontend/src/v2/__tests__/V2AgentBYOHosted.test.tsx @@ -96,6 +96,22 @@ describe('V2AgentBYO — Run it here', () => { // instance has said whether it can host, the page must not offer an act at // all: an enabled "Install + generate token" there chose a runtime for the // person (Sprint Review's measurement on #2190). + // Only a BYO install may be handed back its own unkeyed webhook name. A + // hosted install given that name would hit "already installed". + test('asks for a name in the pod only once bring-your-own is chosen', async () => { + mockGet({ configured: true }); + renderPage(); + const lookups = () => axios.get.mock.calls.map(([url]) => url) + .filter((url) => url.startsWith('/api/registry/agents/next-free-name?')); + + fireEvent.click(await screen.findByTestId('byo-mode-hosted')); + await waitFor(() => expect(lookups().length).toBeGreaterThan(0)); + expect(lookups().some((url) => url.includes('podId='))).toBe(false); + + fireEvent.click(screen.getByTestId('byo-mode-byo')); + await waitFor(() => expect(lookups().some((url) => url.includes('podId=p1'))).toBe(true)); + }); + test('offers no act while it is not yet known whether there is a choice', async () => { let answerAvailability; let answerMachines; diff --git a/frontend/src/v2/__tests__/V2AgentBYOMemory.test.tsx b/frontend/src/v2/__tests__/V2AgentBYOMemory.test.tsx index ca8ad3b8f..ecf3a6d20 100644 --- a/frontend/src/v2/__tests__/V2AgentBYOMemory.test.tsx +++ b/frontend/src/v2/__tests__/V2AgentBYOMemory.test.tsx @@ -142,7 +142,7 @@ describe('V2AgentBYO memory import', () => { const manyPods = Array.from({ length: 20 }, (_, i) => ({ _id: `p${i + 1}`, name: `Workspace ${i + 1}`, type: 'chat' })); axios.get.mockImplementation((url) => { if (url === '/api/pods') return Promise.resolve({ data: manyPods }); - if (url === '/api/registry/agents/next-free-name?base=sam-claude') return Promise.resolve({ data: { name: 'sam-claude-3' } }); + if (url === '/api/registry/agents/next-free-name?base=sam-claude&podId=p1') return Promise.resolve({ data: { name: 'sam-claude-3' } }); return Promise.resolve({ data: {} }); }); @@ -153,6 +153,28 @@ describe('V2AgentBYO memory import', () => { expect(axios.get.mock.calls.filter(([url]) => /\/api\/registry\/pods\/.*\/agents$/.test(url))).toHaveLength(0); }); + test('picking the pod that holds the unkeyed install takes back the suggested suffix', async () => { + // The prefill reads the real location, so a deep link needs pushState. + window.history.pushState({}, '', '/?name=sam-claude&pod=a1&adapter=claude'); + try { + const twoPods = [{ _id: 'a1', name: 'One', type: 'chat' }, { _id: 'a2', name: 'Two', type: 'chat' }]; + axios.get.mockImplementation((url) => { + if (url === '/api/pods') return Promise.resolve({ data: twoPods }); + if (url === '/api/registry/agents/next-free-name?base=sam-claude&podId=a1') return Promise.resolve({ data: { name: 'sam-claude-2' } }); + if (url === '/api/registry/agents/next-free-name?base=sam-claude&podId=a2') return Promise.resolve({ data: { name: 'sam-claude' } }); + return Promise.resolve({ data: {} }); + }); + + renderPage('/?name=sam-claude&pod=a1&adapter=claude'); + const field = screen.getByPlaceholderText('my-mcp-agent'); + await waitFor(() => expect(field).toHaveValue('sam-claude-2')); + fireEvent.change(await screen.findByRole('combobox', { name: /pod it joins/i }), { target: { value: 'a2' } }); + await waitFor(() => expect(field).toHaveValue('sam-claude')); + } finally { + window.history.pushState({}, '', '/'); + } + }); + test('a failed name lookup blocks the connect command until retry succeeds', async () => { let retry = false; axios.get.mockImplementation((url) => { @@ -172,10 +194,11 @@ describe('V2AgentBYO memory import', () => { expect(screen.getByRole('button', { name: /get my connect command/i })).toBeEnabled(); }); - test('already-installed agent requires explicit key replacement for lost-token recovery', async () => { + test('already-installed agent that holds a key requires explicit key replacement', async () => { axios.get.mockResolvedValueOnce({ data: [{ _id: 'p1', name: 'Workspace', type: 'chat' }] }); axios.post .mockRejectedValueOnce({ response: { data: { error: 'Agent already installed in this pod' } } }) + .mockResolvedValueOnce({ data: { existing: true } }) .mockResolvedValueOnce({ data: { token: AGENT_TOKEN } }); renderPage(); @@ -184,11 +207,35 @@ describe('V2AgentBYO memory import', () => { fireEvent.click(screen.getByRole('button', { name: /get my connect command/i })); await screen.findByText(/already has a key/i); - expect(axios.post.mock.calls.some(([url]) => /runtime-tokens/.test(url))).toBe(false); + const asked = axios.post.mock.calls.filter(([url]) => /runtime-tokens/.test(url)); + expect(asked).toHaveLength(1); + expect(asked[0][1].force).toBeUndefined(); fireEvent.click(screen.getByRole('button', { name: /replace the running agent.s key/i })); await screen.findByText(/your connect command for/i); expect(screen.getByText(AGENT_TOKEN)).toBeInTheDocument(); - expect(axios.post.mock.calls.find(([url]) => /runtime-tokens/.test(url))[1]).toMatchObject({ force: true }); + expect(axios.post.mock.calls.filter(([url]) => /runtime-tokens/.test(url))[1][1]).toMatchObject({ force: true }); + }); + + test('already-installed agent whose key was never issued gets it, with no second install and no replacement', async () => { + // An earlier click installed, then its key request failed or was abandoned. + // next-free-name hands that name back; this click finishes the issue. + axios.get.mockResolvedValueOnce({ data: [{ _id: 'p1', name: 'Workspace', type: 'chat' }] }); + axios.post + .mockRejectedValueOnce({ response: { data: { error: 'Agent already installed in this pod' } } }) + .mockResolvedValueOnce({ data: { token: AGENT_TOKEN } }); + + renderPage(); + await waitFor(() => expect(screen.getByText('Workspace (chat)')).toBeInTheDocument()); + 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); + expect(screen.getByText(AGENT_TOKEN)).toBeInTheDocument(); + expect(screen.queryByText(/already has a key/i)).toBeNull(); + const asked = axios.post.mock.calls.filter(([url]) => /runtime-tokens/.test(url)); + expect(asked).toHaveLength(1); + expect(asked[0][1].force).toBeUndefined(); + expect(axios.post.mock.calls.filter(([url]) => url === '/api/registry/install')).toHaveLength(1); }); test('a token shared from another pod is not silently replaced after install', async () => { diff --git a/frontend/src/v2/components/V2AgentBYO.tsx b/frontend/src/v2/components/V2AgentBYO.tsx index e3ce24540..95caeccef 100644 --- a/frontend/src/v2/components/V2AgentBYO.tsx +++ b/frontend/src/v2/components/V2AgentBYO.tsx @@ -316,6 +316,7 @@ const V2AgentBYO: React.FC = () => { } }, [defaultAgentName, nameTouched]); + const lastSuggestedName = useRef(null); useEffect(() => { if (!choicesKnown) return undefined; if (prefill.reconnect && prefill.name) { @@ -326,16 +327,30 @@ const V2AgentBYO: React.FC = () => { const base = sanitizeAgentName(prefill.name || defaultAgentName); setNamesReady(false); setNameLookupError(false); - api.get<{ name?: string }>(`/api/registry/agents/next-free-name?base=${encodeURIComponent(base)}`) + // The pod lets the server hand back this person's own install there whose + // key never arrived, instead of "-2". BYO only: that install is a + // webhook, so another mode handed its name would hit "already installed". + // Not with a persona: its display name derives a non-default instance, + // and the server only hands back installs on the default one. + const inPod = mode === 'byo' && podId && !personaCard ? `&podId=${encodeURIComponent(podId)}` : ''; + api.get<{ name?: string }>(`/api/registry/agents/next-free-name?base=${encodeURIComponent(base)}${inPod}`) .then((result) => { if (cancelled) return; if (!result?.name) throw new Error('Empty name suggestion'); - setName((current) => (!nameTouched || current === prefill.name ? result.name! : current)); + // A name the page suggested earlier is still the page's to replace, + // e.g. "-2" before the person picked the pod holding . + // Read before the update: React runs the updater later. + const previous = lastSuggestedName.current; + lastSuggestedName.current = result.name; + setName((current) => ( + !nameTouched || current === prefill.name || current === previous + ? result.name! : current)); setNamesReady(true); }) .catch(() => { if (!cancelled) setNameLookupError(true); }); return () => { cancelled = true; }; - }, [api, choicesKnown, defaultAgentName, nameLookupRetry, nameTouched, prefill.name, prefill.reconnect]); + }, [api, choicesKnown, defaultAgentName, mode, nameLookupRetry, nameTouched, personaCard, podId, prefill.name, + prefill.reconnect]); const [placed, setPlaced] = useState<{ agentName: string; machineId: string; machineName: string } | null>(null); const [placedState, setPlacedState] = useState<'waiting' | 'running' | 'slow'>('waiting'); const [reconnectRequest, setReconnectRequest] = useState<{ @@ -708,6 +723,11 @@ const V2AgentBYO: React.FC = () => { } setSubmitting(true); try { + // "Already installed" is the lost-token recovery path. The issue below + // never revokes a running key: an agent that holds one answers + // existing:true and the page asks for an explicit replacement. One that + // never got its key (an earlier click's issue failed or was abandoned) + // gets it now, instead of a second install under "-2". try { await api.post('/api/registry/install', { agentName: cleanName, @@ -719,13 +739,9 @@ const V2AgentBYO: React.FC = () => { ...(personaCard ? { description: personaCard.card } : {}), }); } catch (installErr) { - // A repeat install is the lost-token recovery path. Stop before - // revoking the running key and ask for an explicit replacement. const msg = (installErr as { response?: { data?: { error?: string } } }) ?.response?.data?.error || ''; if (!/already installed/i.test(msg)) throw installErr; - setPendingReplacement({ agentName: cleanName, podId }); - return; } // An agent can have a token from another pod. A nonrotating issue // returns existing:true in that case, so no key is revoked here. @@ -1255,7 +1271,7 @@ const V2AgentBYO: React.FC = () => { { setErrorField(null); setNameTouched(true); setPendingReplacement(null); setName(e.target.value); }} + onChange={(e) => { setErrorField(null); setNameTouched(true); setPendingReplacement(null); lastSuggestedName.current = null; setName(e.target.value); }} placeholder={DEFAULT_AGENT_NAME} className="v2-byo__input" />