Skip to content

Commit 10f6cbe

Browse files
authored
feat: assign unassigned targets to gateways and preserve targets on removal (#410)
* feat: assign unassigned targets to gateways and preserve targets on removal * test: add unit tests for unassigned target assignment and gateway removal - getUnassignedTargets: returns targets, empty when no config, empty when field missing - createGatewayFromWizard: moves selected targets to new gateway, removes from unassigned - removeGateway: preserves targets as unassigned on removal, no-op for empty gateways - previewRemoveGateway: shows 'will become unassigned' warning * style: fix formatting and merge duplicate imports * docs: add comment explaining unassigned targets preservation
1 parent 497d7ca commit 10f6cbe

10 files changed

Lines changed: 416 additions & 38 deletions

File tree

src/cli/operations/mcp/__tests__/create-mcp.test.ts

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { SKIP_FOR_NOW } from '../../../tui/screens/mcp/types.js';
2-
import type { AddGatewayTargetConfig } from '../../../tui/screens/mcp/types.js';
3-
import { createExternalGatewayTarget } from '../create-mcp.js';
2+
import type { AddGatewayConfig, AddGatewayTargetConfig } from '../../../tui/screens/mcp/types.js';
3+
import { createExternalGatewayTarget, createGatewayFromWizard, getUnassignedTargets } from '../create-mcp.js';
44
import { afterEach, describe, expect, it, vi } from 'vitest';
55

66
const { mockReadMcpSpec, mockWriteMcpSpec, mockConfigExists, mockReadProjectSpec } = vi.hoisted(() => ({
@@ -131,3 +131,75 @@ describe('createExternalGatewayTarget', () => {
131131
expect(target.outboundAuth).toEqual({ type: 'API_KEY', credentialName: 'my-cred' });
132132
});
133133
});
134+
135+
describe('getUnassignedTargets', () => {
136+
afterEach(() => vi.clearAllMocks());
137+
138+
it('returns unassigned targets from mcp spec', async () => {
139+
mockConfigExists.mockReturnValue(true);
140+
mockReadMcpSpec.mockResolvedValue({
141+
agentCoreGateways: [],
142+
unassignedTargets: [{ name: 't1' }, { name: 't2' }],
143+
});
144+
145+
const result = await getUnassignedTargets();
146+
expect(result).toHaveLength(2);
147+
expect(result[0]!.name).toBe('t1');
148+
});
149+
150+
it('returns empty array when no mcp config exists', async () => {
151+
mockConfigExists.mockReturnValue(false);
152+
expect(await getUnassignedTargets()).toEqual([]);
153+
});
154+
155+
it('returns empty array when unassignedTargets field is missing', async () => {
156+
mockConfigExists.mockReturnValue(true);
157+
mockReadMcpSpec.mockResolvedValue({ agentCoreGateways: [] });
158+
expect(await getUnassignedTargets()).toEqual([]);
159+
});
160+
});
161+
162+
describe('createGatewayFromWizard with selectedTargets', () => {
163+
afterEach(() => vi.clearAllMocks());
164+
165+
function makeGatewayConfig(overrides: Partial<AddGatewayConfig> = {}): AddGatewayConfig {
166+
return {
167+
name: 'new-gateway',
168+
authorizerType: 'AWS_IAM',
169+
...overrides,
170+
} as AddGatewayConfig;
171+
}
172+
173+
it('moves selected targets to new gateway and removes from unassigned', async () => {
174+
mockConfigExists.mockReturnValue(true);
175+
mockReadMcpSpec.mockResolvedValue({
176+
agentCoreGateways: [],
177+
unassignedTargets: [
178+
{ name: 'target-a', targetType: 'mcpServer' },
179+
{ name: 'target-b', targetType: 'mcpServer' },
180+
{ name: 'target-c', targetType: 'mcpServer' },
181+
],
182+
});
183+
184+
await createGatewayFromWizard(makeGatewayConfig({ selectedTargets: ['target-a', 'target-c'] }));
185+
186+
const written = mockWriteMcpSpec.mock.calls[0]![0];
187+
const gateway = written.agentCoreGateways.find((g: { name: string }) => g.name === 'new-gateway');
188+
expect(gateway.targets).toHaveLength(2);
189+
expect(gateway.targets[0]!.name).toBe('target-a');
190+
expect(gateway.targets[1]!.name).toBe('target-c');
191+
expect(written.unassignedTargets).toHaveLength(1);
192+
expect(written.unassignedTargets[0]!.name).toBe('target-b');
193+
});
194+
195+
it('creates gateway with empty targets when no selectedTargets', async () => {
196+
mockConfigExists.mockReturnValue(true);
197+
mockReadMcpSpec.mockResolvedValue({ agentCoreGateways: [] });
198+
199+
await createGatewayFromWizard(makeGatewayConfig());
200+
201+
const written = mockWriteMcpSpec.mock.calls[0]![0];
202+
const gateway = written.agentCoreGateways.find((g: { name: string }) => g.name === 'new-gateway');
203+
expect(gateway.targets).toHaveLength(0);
204+
});
205+
});

src/cli/operations/mcp/create-mcp.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,22 @@ function buildAuthorizerConfiguration(config: AddGatewayConfig): AgentCoreGatewa
8080
};
8181
}
8282

83+
/**
84+
* Get list of unassigned targets from MCP spec.
85+
*/
86+
export async function getUnassignedTargets(): Promise<AgentCoreGatewayTarget[]> {
87+
try {
88+
const configIO = new ConfigIO();
89+
if (!configIO.configExists('mcp')) {
90+
return [];
91+
}
92+
const mcpSpec = await configIO.readMcpSpec();
93+
return mcpSpec.unassignedTargets ?? [];
94+
} catch {
95+
return [];
96+
}
97+
}
98+
8399
/**
84100
* Get list of existing gateway names from project spec.
85101
*/
@@ -160,15 +176,34 @@ export async function createGatewayFromWizard(config: AddGatewayConfig): Promise
160176
throw new Error(`Gateway "${config.name}" already exists.`);
161177
}
162178

179+
// Collect selected unassigned targets
180+
const selectedTargets: AgentCoreGatewayTarget[] = [];
181+
if (config.selectedTargets && config.selectedTargets.length > 0) {
182+
const unassignedTargets = mcpSpec.unassignedTargets ?? [];
183+
for (const targetName of config.selectedTargets) {
184+
const target = unassignedTargets.find(t => t.name === targetName);
185+
if (target) {
186+
selectedTargets.push(target);
187+
}
188+
}
189+
}
190+
163191
const gateway: AgentCoreGateway = {
164192
name: config.name,
165193
description: config.description,
166-
targets: [],
194+
targets: selectedTargets,
167195
authorizerType: config.authorizerType,
168196
authorizerConfiguration: buildAuthorizerConfiguration(config),
169197
};
170198

171199
mcpSpec.agentCoreGateways.push(gateway);
200+
201+
// Remove selected targets from unassigned targets
202+
if (config.selectedTargets && config.selectedTargets.length > 0) {
203+
const selected = config.selectedTargets;
204+
mcpSpec.unassignedTargets = (mcpSpec.unassignedTargets ?? []).filter(t => !selected.includes(t.name));
205+
}
206+
172207
await configIO.writeMcpSpec(mcpSpec);
173208

174209
return { name: config.name };
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
import { previewRemoveGateway, removeGateway } from '../remove-gateway.js';
2+
import { afterEach, describe, expect, it, vi } from 'vitest';
3+
4+
const { mockReadMcpSpec, mockWriteMcpSpec, mockConfigExists } = vi.hoisted(() => ({
5+
mockReadMcpSpec: vi.fn(),
6+
mockWriteMcpSpec: vi.fn(),
7+
mockConfigExists: vi.fn(),
8+
}));
9+
10+
vi.mock('../../../../lib/index.js', () => ({
11+
ConfigIO: class {
12+
configExists = mockConfigExists;
13+
readMcpSpec = mockReadMcpSpec;
14+
writeMcpSpec = mockWriteMcpSpec;
15+
},
16+
}));
17+
18+
describe('removeGateway', () => {
19+
afterEach(() => vi.clearAllMocks());
20+
21+
it('moves gateway targets to unassignedTargets on removal, preserving existing', async () => {
22+
mockReadMcpSpec.mockResolvedValue({
23+
agentCoreGateways: [
24+
{ name: 'gw-to-remove', targets: [{ name: 'target-1' }, { name: 'target-2' }] },
25+
{ name: 'other-gw', targets: [] },
26+
],
27+
unassignedTargets: [{ name: 'already-unassigned' }],
28+
});
29+
30+
const result = await removeGateway('gw-to-remove');
31+
32+
expect(result.ok).toBe(true);
33+
const written = mockWriteMcpSpec.mock.calls[0]![0];
34+
expect(written.agentCoreGateways).toHaveLength(1);
35+
expect(written.agentCoreGateways[0]!.name).toBe('other-gw');
36+
expect(written.unassignedTargets).toHaveLength(3);
37+
expect(written.unassignedTargets[0]!.name).toBe('already-unassigned');
38+
expect(written.unassignedTargets[1]!.name).toBe('target-1');
39+
expect(written.unassignedTargets[2]!.name).toBe('target-2');
40+
});
41+
42+
it('does not modify unassignedTargets when gateway has no targets', async () => {
43+
mockReadMcpSpec.mockResolvedValue({
44+
agentCoreGateways: [{ name: 'empty-gw', targets: [] }],
45+
});
46+
47+
const result = await removeGateway('empty-gw');
48+
49+
expect(result.ok).toBe(true);
50+
const written = mockWriteMcpSpec.mock.calls[0]![0];
51+
expect(written.agentCoreGateways).toHaveLength(0);
52+
expect(written.unassignedTargets).toBeUndefined();
53+
});
54+
});
55+
56+
describe('previewRemoveGateway', () => {
57+
afterEach(() => vi.clearAllMocks());
58+
59+
it('shows "will become unassigned" warning when gateway has targets', async () => {
60+
mockReadMcpSpec.mockResolvedValue({
61+
agentCoreGateways: [{ name: 'my-gw', targets: [{ name: 't1' }, { name: 't2' }] }],
62+
});
63+
64+
const preview = await previewRemoveGateway('my-gw');
65+
66+
expect(preview.summary.some(s => s.includes('2 target(s) will become unassigned'))).toBe(true);
67+
});
68+
});

src/cli/operations/remove/remove-gateway.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export async function previewRemoveGateway(gatewayName: string): Promise<Removal
3434
const schemaChanges: SchemaChange[] = [];
3535

3636
if (gateway.targets.length > 0) {
37-
summary.push(`Note: ${gateway.targets.length} target(s) behind this gateway will become orphaned`);
37+
summary.push(`Note: ${gateway.targets.length} target(s) will become unassigned`);
3838
}
3939

4040
// Compute schema changes
@@ -52,9 +52,17 @@ export async function previewRemoveGateway(gatewayName: string): Promise<Removal
5252
* Compute the MCP spec after removing a gateway.
5353
*/
5454
function computeRemovedGatewayMcpSpec(mcpSpec: AgentCoreMcpSpec, gatewayName: string): AgentCoreMcpSpec {
55+
const gatewayToRemove = mcpSpec.agentCoreGateways.find(g => g.name === gatewayName);
56+
const targetsToPreserve = gatewayToRemove?.targets ?? [];
57+
5558
return {
5659
...mcpSpec,
5760
agentCoreGateways: mcpSpec.agentCoreGateways.filter(g => g.name !== gatewayName),
61+
// Preserve gateway's targets as unassigned so the user doesn't lose them.
62+
// Only add the field if there are targets to preserve or unassignedTargets already exists.
63+
...(targetsToPreserve.length > 0 || mcpSpec.unassignedTargets
64+
? { unassignedTargets: [...(mcpSpec.unassignedTargets ?? []), ...targetsToPreserve] }
65+
: {}),
5866
};
5967
}
6068

src/cli/tui/hooks/useCreateMcp.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
getAvailableAgents,
66
getExistingGateways,
77
getExistingToolNames,
8+
getUnassignedTargets,
89
} from '../../operations/mcp/create-mcp';
910
import type { AddGatewayConfig, AddGatewayTargetConfig } from '../screens/mcp/types';
1011
import { useCallback, useEffect, useState } from 'react';
@@ -117,3 +118,22 @@ export function useExistingToolNames() {
117118

118119
return { toolNames, refresh };
119120
}
121+
122+
export function useUnassignedTargets() {
123+
const [targets, setTargets] = useState<string[]>([]);
124+
125+
useEffect(() => {
126+
async function load() {
127+
const result = await getUnassignedTargets();
128+
setTargets(result.map(t => t.name));
129+
}
130+
void load();
131+
}, []);
132+
133+
const refresh = useCallback(async () => {
134+
const result = await getUnassignedTargets();
135+
setTargets(result.map(t => t.name));
136+
}, []);
137+
138+
return { targets, refresh };
139+
}

src/cli/tui/screens/mcp/AddGatewayFlow.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { SelectableItem } from '../../components';
33
import { HELP_TEXT } from '../../constants';
44
import { useListNavigation } from '../../hooks';
55
import { useAgents, useAttachGateway, useGateways } from '../../hooks/useAttach';
6-
import { useCreateGateway, useExistingGateways } from '../../hooks/useCreateMcp';
6+
import { useCreateGateway, useExistingGateways, useUnassignedTargets } from '../../hooks/useCreateMcp';
77
import { AddSuccessScreen } from '../add/AddSuccessScreen';
88
import { AddGatewayScreen } from './AddGatewayScreen';
99
import type { AddGatewayConfig } from './types';
@@ -55,6 +55,7 @@ export function AddGatewayFlow({
5555
}: AddGatewayFlowProps) {
5656
const { createGateway, reset: resetCreate } = useCreateGateway();
5757
const { gateways: existingGateways, refresh: refreshGateways } = useExistingGateways();
58+
const { targets: unassignedTargets } = useUnassignedTargets();
5859
const [flow, setFlow] = useState<FlowState>({ name: 'mode-select' });
5960

6061
// Bind flow hooks
@@ -157,6 +158,7 @@ export function AddGatewayFlow({
157158
<AddGatewayScreen
158159
existingGateways={existingGateways}
159160
availableAgents={availableAgents}
161+
unassignedTargets={unassignedTargets}
160162
onComplete={handleCreateComplete}
161163
onExit={onBack}
162164
/>
@@ -183,6 +185,7 @@ export function AddGatewayFlow({
183185
<AddGatewayScreen
184186
existingGateways={existingGateways}
185187
availableAgents={availableAgents}
188+
unassignedTargets={unassignedTargets}
186189
onComplete={handleCreateComplete}
187190
onExit={() => setFlow({ name: 'mode-select' })}
188191
/>

0 commit comments

Comments
 (0)