a73x

081c27b8

feat(mcp): a VM lands on the host you name

a73x   2026-08-06 13:40

Commit message
feat(mcp): a VM lands on the host you name

vm_create's host parameter resolves against the whole fleet: an exact
match on host id or name places the VM there, wherever it sits in the
listing. An offline match is refused — nothing would converge the create
until that host came back — and a name that matches nothing errors with
the fleet's hosts and their online state, so the model can correct itself
without a second call. Naming no host still takes the first online one.
internal/mcpserver/tools.go
Old New
@@ -21,6 +21,7 @@ type api interface {
21 ListVMs(ctx context.Context) ([]client.VM, error) 21 ListVMs(ctx context.Context) ([]client.VM, error)
22 CreateVM(ctx context.Context, req client.CreateVMRequest) (client.CreateVMResponse, error) 22 CreateVM(ctx context.Context, req client.CreateVMRequest) (client.CreateVMResponse, error)
23 DeleteVM(ctx context.Context, id string) error 23 DeleteVM(ctx context.Context, id string) error
24 ListHosts(ctx context.Context) ([]client.Host, error)
24 FirstOnlineHost(ctx context.Context) (client.Host, error) 25 FirstOnlineHost(ctx context.Context) (client.Host, error)
25 CreateExposure(ctx context.Context, vmID string, guestPort, hostPort int64) (client.Exposure, error) 26 CreateExposure(ctx context.Context, vmID string, guestPort, hostPort int64) (client.Exposure, error)
26 ListExposures(ctx context.Context, vmID string) ([]client.Exposure, error) 27 ListExposures(ctx context.Context, vmID string) ([]client.Exposure, error)
@@ -136,11 +137,11 @@ func (t *Tools) VMCreate(ctx context.Context, in VMCreateIn) (VMCreateOut, error
136 req.DiskGB = 20 137 req.DiskGB = 20
137 } 138 }
138 if in.Host != "" { 139 if in.Host != "" {
139 vm, err := t.resolveHost(ctx, in.Host) 140 hostID, err := t.resolveHost(ctx, in.Host)
140 if err != nil { 141 if err != nil {
141 return VMCreateOut{}, err 142 return VMCreateOut{}, err
142 } 143 }
143 req.HostID = vm 144 req.HostID = hostID
144 } else { 145 } else {
145 h, err := t.API.FirstOnlineHost(ctx) 146 h, err := t.API.FirstOnlineHost(ctx)
146 if err != nil { 147 if err != nil {
@@ -260,18 +261,43 @@ func (t *Tools) waitReady(ctx context.Context, id, name string, deadline time.Ti
260 return "", fmt.Errorf("vm %s (%s) not ready after %s (control-plane never listed successfully; last error: %v); it may still come up — check vm_info, do not assume failure", id, name, t.waitTimeout(), lastErr) 261 return "", fmt.Errorf("vm %s (%s) not ready after %s (control-plane never listed successfully; last error: %v); it may still come up — check vm_info, do not assume failure", id, name, t.waitTimeout(), lastErr)
261 } 262 }
262 263
264 // resolveHost turns a caller-named host into the id to place on: an exact match
265 // on host id or host name, anywhere in the fleet. An offline match is refused
266 // rather than accepted — nothing would converge the create until that host came
267 // back, so the VM would simply sit there. Errors name the hosts that DO exist
268 // and whether each is online, so the model can correct itself without a second
269 // call.
263 func (t *Tools) resolveHost(ctx context.Context, name string) (string, error) { 270 func (t *Tools) resolveHost(ctx context.Context, name string) (string, error) {
264 // v1 places every VM on the first online host; the api seam exposes only 271 hosts, err := t.API.ListHosts(ctx)
265 // FirstOnlineHost. A caller-named host must therefore match it. Extend the
266 // seam with real host-by-name lookup when multi-host placement is needed.
267 h, err := t.API.FirstOnlineHost(ctx)
268 if err != nil { 272 if err != nil {
269 return "", err 273 return "", err
270 } 274 }
271 if h.Name != name && h.ID != name { 275 for _, h := range hosts {
272 return "", fmt.Errorf("unknown host %q (v1 places on the first online host %q; pass no host to use it)", name, h.Name) 276 if h.ID != name && h.Name != name {
277 continue
278 }
279 if !h.Online {
280 return "", fmt.Errorf("host %q is offline (fleet: %s)", name, describeHosts(hosts))
281 }
282 return h.ID, nil
283 }
284 return "", fmt.Errorf("no host with id or name %q (fleet: %s)", name, describeHosts(hosts))
285 }
286
287 // describeHosts renders the fleet as "name (online)" entries for an error.
288 func describeHosts(hosts []client.Host) string {
289 if len(hosts) == 0 {
290 return "none"
291 }
292 parts := make([]string, 0, len(hosts))
293 for _, h := range hosts {
294 state := "offline"
295 if h.Online {
296 state = "online"
297 }
298 parts = append(parts, fmt.Sprintf("%s (%s)", h.Name, state))
273 } 299 }
274 return h.ID, nil 300 return strings.Join(parts, ", ")
275 } 301 }
276 302
277 func (t *Tools) sshCommand(ctx context.Context, name string) string { 303 func (t *Tools) sshCommand(ctx context.Context, name string) string {
internal/mcpserver/tools_test.go
Old New
@@ -27,6 +27,17 @@ type fakeToolsAPI struct {
27 // exposures the fake serves per VM id, and the exposure ids revoked. 27 // exposures the fake serves per VM id, and the exposure ids revoked.
28 exposures map[string][]client.Exposure 28 exposures map[string][]client.Exposure
29 revoked []string 29 revoked []string
30 // hosts the fake's fleet reports; nil means the one-host fleet.
31 hosts []client.Host
32 }
33
34 // fleet is the fake's host list, defaulted so tests that don't care about
35 // placement get a single online host.
36 func (f *fakeToolsAPI) fleet() []client.Host {
37 if f.hosts != nil {
38 return f.hosts
39 }
40 return []client.Host{{ID: "h1", Name: "mewtwo", Online: true}}
30 } 41 }
31 42
32 func (f *fakeToolsAPI) ListVMs(ctx context.Context) ([]client.VM, error) { 43 func (f *fakeToolsAPI) ListVMs(ctx context.Context) ([]client.VM, error) {
@@ -56,8 +67,16 @@ func (f *fakeToolsAPI) DeleteVM(ctx context.Context, id string) error {
56 f.deleted = append(f.deleted, id) 67 f.deleted = append(f.deleted, id)
57 return nil 68 return nil
58 } 69 }
70 func (f *fakeToolsAPI) ListHosts(ctx context.Context) ([]client.Host, error) {
71 return f.fleet(), nil
72 }
59 func (f *fakeToolsAPI) FirstOnlineHost(ctx context.Context) (client.Host, error) { 73 func (f *fakeToolsAPI) FirstOnlineHost(ctx context.Context) (client.Host, error) {
60 return client.Host{ID: "h1", Name: "mewtwo", Online: true}, nil 74 for _, h := range f.fleet() {
75 if h.Online {
76 return h, nil
77 }
78 }
79 return client.Host{}, fmt.Errorf("no online hosts")
61 } 80 }
62 81
63 // CreateExposure mirrors the control plane: host port 0 is allocated from the 82 // CreateExposure mirrors the control plane: host port 0 is allocated from the
@@ -275,6 +294,77 @@ func TestCreateNoWaitReturnsImmediately(t *testing.T) {
275 assert.Empty(t, run.execs) 294 assert.Empty(t, run.execs)
276 } 295 }
277 296
297 // mixedFleet is a two-host fleet with a Mac second and a decommissioned-looking
298 // host offline, the shape placement has to get right.
299 func mixedFleet() []client.Host {
300 return []client.Host{
301 {ID: "h1", Name: "onyx", Online: true},
302 {ID: "h2", Name: "Squirtle.local", Online: true},
303 {ID: "h3", Name: "charmander", Online: false},
304 }
305 }
306
307 func TestCreatePlacesOnTheNamedHost(t *testing.T) {
308 api := &fakeToolsAPI{hosts: mixedFleet()}
309 tl := newTestTools(api, &fakeRunner{})
310 no := false
311
312 _, err := tl.VMCreate(t.Context(), VMCreateIn{Host: "Squirtle.local", Wait: &no})
313 require.NoError(t, err)
314 require.Len(t, api.created, 1)
315 assert.Equal(t, "h2", api.created[0].HostID, "a named host is reachable past the first online one")
316 }
317
318 func TestCreatePlacesOnAHostNamedByID(t *testing.T) {
319 api := &fakeToolsAPI{hosts: mixedFleet()}
320 tl := newTestTools(api, &fakeRunner{})
321 no := false
322
323 _, err := tl.VMCreate(t.Context(), VMCreateIn{Host: "h2", Wait: &no})
324 require.NoError(t, err)
325 require.Len(t, api.created, 1)
326 assert.Equal(t, "h2", api.created[0].HostID)
327 }
328
329 func TestCreateRefusesAnOfflineHost(t *testing.T) {
330 api := &fakeToolsAPI{hosts: mixedFleet()}
331 tl := newTestTools(api, &fakeRunner{})
332 no := false
333
334 _, err := tl.VMCreate(t.Context(), VMCreateIn{Host: "charmander", Wait: &no})
335 require.Error(t, err)
336 assert.ErrorContains(t, err, `host "charmander" is offline`)
337 assert.Empty(t, api.created, "an offline host would strand the create")
338 }
339
340 func TestCreateUnknownHostNamesTheFleet(t *testing.T) {
341 api := &fakeToolsAPI{hosts: mixedFleet()}
342 tl := newTestTools(api, &fakeRunner{})
343 no := false
344
345 _, err := tl.VMCreate(t.Context(), VMCreateIn{Host: "mewtwo", Wait: &no})
346 require.Error(t, err)
347 assert.ErrorContains(t, err, `no host with id or name "mewtwo"`)
348 assert.ErrorContains(t, err, "onyx (online)", "the error names what the fleet DOES have")
349 assert.ErrorContains(t, err, "Squirtle.local (online)")
350 assert.ErrorContains(t, err, "charmander (offline)", "with each host's state")
351 assert.Empty(t, api.created)
352 }
353
354 func TestCreateWithoutAHostTakesTheFirstOnlineOne(t *testing.T) {
355 // The first host is offline, so the default lands on the second.
356 hosts := mixedFleet()
357 hosts[0].Online = false
358 api := &fakeToolsAPI{hosts: hosts}
359 tl := newTestTools(api, &fakeRunner{})
360 no := false
361
362 _, err := tl.VMCreate(t.Context(), VMCreateIn{Wait: &no})
363 require.NoError(t, err)
364 require.Len(t, api.created, 1)
365 assert.Equal(t, "h2", api.created[0].HostID)
366 }
367
278 func TestCreateWaitTimeoutDoesNotDestroy(t *testing.T) { 368 func TestCreateWaitTimeoutDoesNotDestroy(t *testing.T) {
279 api := &fakeToolsAPI{phases: []string{"creating"}} // never ready 369 api := &fakeToolsAPI{phases: []string{"creating"}} // never ready
280 run := &fakeRunner{} 370 run := &fakeRunner{}