From 267caaa8e8ad102c1f0de24bd9b0dfedf57147d8 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 19:49:49 +0300 Subject: [PATCH 01/17] feat(sshx): add an Endpoint for the ssh target Signed-off-by: NovusEdge --- internal/sshx/endpoint.go | 30 ++++++++++++++++++++++++++++++ internal/sshx/endpoint_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 internal/sshx/endpoint.go create mode 100644 internal/sshx/endpoint_test.go diff --git a/internal/sshx/endpoint.go b/internal/sshx/endpoint.go new file mode 100644 index 0000000..556a4e0 --- /dev/null +++ b/internal/sshx/endpoint.go @@ -0,0 +1,30 @@ +package sshx + +import "github.com/novusedge/stoat/internal/config" + +// Endpoint is where ssh(1) and scp(1) reach a guest, and under what host key +// policy. A QEMU VM answers on a loopback forward, where an unchecked host key +// costs nothing. A cloud instance answers on a routable address, where the +// same setting is a machine-in-the-middle hole, so that endpoint names a +// per-VM known_hosts file instead. +type Endpoint struct { + Name string + Host string + Port int + User string + + // KnownHosts is the known_hosts file to pin against. Empty keeps the + // loopback policy: no host key checking at all. + KnownHosts string +} + +// LocalEndpoint is the endpoint for a VM reached through QEMU's user-mode +// port forward. +func LocalEndpoint(v *config.VM) Endpoint { + return Endpoint{ + Name: v.Name, + Host: "127.0.0.1", + Port: v.SSHPort, + User: User(v), + } +} diff --git a/internal/sshx/endpoint_test.go b/internal/sshx/endpoint_test.go new file mode 100644 index 0000000..4b1077f --- /dev/null +++ b/internal/sshx/endpoint_test.go @@ -0,0 +1,34 @@ +package sshx + +import ( + "testing" + + "github.com/novusedge/stoat/internal/config" +) + +func TestLocalEndpoint(t *testing.T) { + v := &config.VM{Name: "dev", SSHPort: 2222, SSHUser: "stoat"} + e := LocalEndpoint(v) + if e.Host != "127.0.0.1" { + t.Errorf("Host = %q, want 127.0.0.1", e.Host) + } + if e.Port != 2222 { + t.Errorf("Port = %d, want 2222", e.Port) + } + if e.User != "stoat" { + t.Errorf("User = %q, want stoat", e.User) + } + if e.Name != "dev" { + t.Errorf("Name = %q, want dev", e.Name) + } + if e.KnownHosts != "" { + t.Errorf("KnownHosts = %q, want empty for a loopback forward", e.KnownHosts) + } +} + +func TestLocalEndpointDefaultsUserToRoot(t *testing.T) { + e := LocalEndpoint(&config.VM{Name: "live", SSHPort: 2200}) + if e.User != "root" { + t.Errorf("User = %q, want root", e.User) + } +} From 3d1381ea1c1800aae61990b621b81b9609c8b277 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 19:55:45 +0300 Subject: [PATCH 02/17] refactor(sshx): take an explicit endpoint Args, CopyArgs and Wait read the host and port from an Endpoint. A loopback endpoint keeps the unchecked host key policy; a named known_hosts file pins instead. Signed-off-by: NovusEdge --- internal/cli/run_access.go | 2 +- internal/core/access.go | 2 +- internal/core/apply.go | 4 +-- internal/core/copy.go | 2 +- internal/sshx/endpoint_test.go | 50 ++++++++++++++++++++++++++++ internal/sshx/outputs.go | 2 +- internal/sshx/run.go | 2 +- internal/sshx/run_test.go | 2 +- internal/sshx/sharemount.go | 2 +- internal/sshx/sshx.go | 59 ++++++++++++++++++---------------- internal/sshx/sshx_test.go | 28 ++++++++-------- internal/tui/cloudinit.go | 2 +- 12 files changed, 106 insertions(+), 51 deletions(-) diff --git a/internal/cli/run_access.go b/internal/cli/run_access.go index c77b33b..f84989a 100644 --- a/internal/cli/run_access.go +++ b/internal/cli/run_access.go @@ -110,7 +110,7 @@ func runSSH(a *Args, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, "stoat: ssh:", err) return ExitFail } - argv := append([]string{"ssh"}, sshx.Args(v)...) + argv := append([]string{"ssh"}, sshx.Args(sshx.LocalEndpoint(v))...) if err := syscall.Exec(path, argv, os.Environ()); err != nil { fmt.Fprintln(stderr, "stoat: ssh:", err) return ExitFail diff --git a/internal/core/access.go b/internal/core/access.go index 9a51390..15fee1f 100644 --- a/internal/core/access.go +++ b/internal/core/access.go @@ -30,7 +30,7 @@ func SSHCommand(name string) ([]string, error) { if err != nil { return nil, err } - return append([]string{"ssh"}, sshx.Args(v)...), nil + return append([]string{"ssh"}, sshx.Args(sshx.LocalEndpoint(v))...), nil } // Which selects one of a VM's two log files. diff --git a/internal/core/apply.go b/internal/core/apply.go index 7d856d0..bf21401 100644 --- a/internal/core/apply.go +++ b/internal/core/apply.go @@ -286,7 +286,7 @@ func rebootAndWait(ctx context.Context, v *config.VM, recipe string) error { // `reboot` tears down the ssh session before the process can report an // exit status back to this host, so cmd.Run() returning an error here is // expected and not a failure signal; only the wait below is. - cmd := exec.CommandContext(ctx, "ssh", sshx.Args(v, "reboot")...) + cmd := exec.CommandContext(ctx, "ssh", sshx.Args(sshx.LocalEndpoint(v), "reboot")...) _ = cmd.Run() // The pre-reboot sshd can keep answering for a moment after the reboot @@ -344,7 +344,7 @@ func discoverCloudInitApplied(ctx context.Context, v *config.VM) ([]string, erro return nil, nil } script := fmt.Sprintf("for marker in %s/*; do case \"$marker\" in *.out) continue;; esac; [ -f \"$marker\" ] || continue; name=$(basename \"$marker\"); printf '===%%s\\n' \"$name\"; cat \"$marker.out\" 2>/dev/null; done", cloudinit.MarkerDir) - out, err := exec.CommandContext(ctx, "ssh", sshx.Args(v, script)...).Output() + out, err := exec.CommandContext(ctx, "ssh", sshx.Args(sshx.LocalEndpoint(v), script)...).Output() if err != nil { return nil, nil // marker dir missing or a transient ssh error; discover nothing } diff --git a/internal/core/copy.go b/internal/core/copy.go index 714e862..9e15adc 100644 --- a/internal/core/copy.go +++ b/internal/core/copy.go @@ -57,7 +57,7 @@ func doCopy(ctx context.Context, name, localPath, remotePath string, toRemote bo } var stderr bytes.Buffer - c := exec.CommandContext(ctx, "scp", sshx.CopyArgs(v, localPath, remotePath, toRemote)...) + c := exec.CommandContext(ctx, "scp", sshx.CopyArgs(sshx.LocalEndpoint(v), localPath, remotePath, toRemote)...) c.Stderr = &stderr err = c.Run() diff --git a/internal/sshx/endpoint_test.go b/internal/sshx/endpoint_test.go index 4b1077f..66b67bd 100644 --- a/internal/sshx/endpoint_test.go +++ b/internal/sshx/endpoint_test.go @@ -1,11 +1,14 @@ package sshx import ( + "strings" "testing" "github.com/novusedge/stoat/internal/config" ) +func joined(a []string) string { return strings.Join(a, " ") } + func TestLocalEndpoint(t *testing.T) { v := &config.VM{Name: "dev", SSHPort: 2222, SSHUser: "stoat"} e := LocalEndpoint(v) @@ -32,3 +35,50 @@ func TestLocalEndpointDefaultsUserToRoot(t *testing.T) { t.Errorf("User = %q, want root", e.User) } } + +func TestConnOptionsLoopbackKeepsUncheckedPolicy(t *testing.T) { + got := joined(connOptions(Endpoint{Host: "127.0.0.1", Port: 2222})) + if !strings.Contains(got, "StrictHostKeyChecking=no") { + t.Errorf("loopback options = %q, want StrictHostKeyChecking=no", got) + } + if !strings.Contains(got, "UserKnownHostsFile=/dev/null") { + t.Errorf("loopback options = %q, want UserKnownHostsFile=/dev/null", got) + } +} + +func TestConnOptionsPinsWhenKnownHostsSet(t *testing.T) { + got := joined(connOptions(Endpoint{Host: "34.1.2.3", Port: 22, KnownHosts: "/tmp/kh"})) + if !strings.Contains(got, "StrictHostKeyChecking=accept-new") { + t.Errorf("pinned options = %q, want StrictHostKeyChecking=accept-new", got) + } + if !strings.Contains(got, "UserKnownHostsFile=/tmp/kh") { + t.Errorf("pinned options = %q, want the named known_hosts file", got) + } + if strings.Contains(got, "/dev/null") { + t.Errorf("pinned options = %q, must not discard the host key", got) + } +} + +func TestArgsUsesEndpointHostAndPort(t *testing.T) { + got := joined(Args(Endpoint{Host: "34.1.2.3", Port: 22, User: "stoat"}, "uptime")) + if !strings.Contains(got, "-p 22") { + t.Errorf("args = %q, want -p 22", got) + } + if !strings.Contains(got, "stoat@34.1.2.3") { + t.Errorf("args = %q, want stoat@34.1.2.3", got) + } + if !strings.HasSuffix(got, "uptime") { + t.Errorf("args = %q, want the remote command last", got) + } +} + +func TestCopyArgsUsesEndpointHostAndPort(t *testing.T) { + e := Endpoint{Host: "34.1.2.3", Port: 22, User: "stoat"} + got := joined(CopyArgs(e, "/local", "/remote", true)) + if !strings.Contains(got, "-P 22") { + t.Errorf("copy args = %q, want -P 22", got) + } + if !strings.HasSuffix(got, "/local stoat@34.1.2.3:/remote") { + t.Errorf("copy args = %q, want local then remote", got) + } +} diff --git a/internal/sshx/outputs.go b/internal/sshx/outputs.go index 1faa0dc..1063da2 100644 --- a/internal/sshx/outputs.go +++ b/internal/sshx/outputs.go @@ -48,7 +48,7 @@ func collectOutputs(ctx context.Context, v *config.VM, name string, m recipes.Ma // it, so a multi-word script must travel as one already-quoted argv // element or only its first word ends up under the escalation prefix. remote := []string{"sh -c " + guest.ShQuote(script)} - out, err := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, remote)...)...).Output() + out, err := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, remote)...)...).Output() if err != nil { var ee *exec.ExitError if errors.As(err, &ee) { diff --git a/internal/sshx/run.go b/internal/sshx/run.go index e51b13b..e3ec838 100644 --- a/internal/sshx/run.go +++ b/internal/sshx/run.go @@ -44,7 +44,7 @@ func Run(ctx context.Context, v *config.VM, root bool, argv []string, stdin io.R remote = escalate(v, argv) } var out, errb bytes.Buffer - c := exec.CommandContext(ctx, "ssh", Args(v, Quote(remote))...) + c := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), Quote(remote))...) c.Stdin = stdin c.Stdout = &out c.Stderr = &errb diff --git a/internal/sshx/run_test.go b/internal/sshx/run_test.go index 6db10a0..58d629f 100644 --- a/internal/sshx/run_test.go +++ b/internal/sshx/run_test.go @@ -84,7 +84,7 @@ func TestRunDoesNotEscalateForRoot(t *testing.T) { // strings end in "'id'". Comparing the whole line against Args with the // bare quoted argv is the only check that catches a prefix Run should // not have added. - want := strings.Join(sshx.Args(v, sshx.Quote([]string{"id"})), " ") + want := strings.Join(sshx.Args(sshx.LocalEndpoint(v), sshx.Quote([]string{"id"})), " ") if got := calls.Calls()[0].Remote; got != want { t.Fatalf("ssh argv = %q, want %q (root must not escalate)", got, want) } diff --git a/internal/sshx/sharemount.go b/internal/sshx/sharemount.go index 786ea6f..5f47f73 100644 --- a/internal/sshx/sharemount.go +++ b/internal/sshx/sharemount.go @@ -109,7 +109,7 @@ func mountShares(ctx context.Context, v *config.VM, log io.Writer) { return } fmt.Fprintln(log, "\n=== mounting 9p shares ===") - cmd := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, []string{"sh", "-s"})...)...) + cmd := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, []string{"sh", "-s"})...)...) cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) } cmd.WaitDelay = recipeShutdownGrace cmd.Stdin = strings.NewReader(shareMountScript(tags, guestFstabPath)) diff --git a/internal/sshx/sshx.go b/internal/sshx/sshx.go index 1a7b176..446098d 100644 --- a/internal/sshx/sshx.go +++ b/internal/sshx/sshx.go @@ -58,23 +58,29 @@ func User(v *config.VM) string { // The port flag is not here. ssh takes "-p" and scp takes "-P" (capital, // since scp's lowercase -p means "preserve file times"), so each caller // supplies it itself. See CopyArgs. -func connOptions() []string { - return []string{ +func connOptions(e Endpoint) []string { + host := []string{ "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", + } + if e.KnownHosts != "" { + host = []string{ + "-o", "StrictHostKeyChecking=accept-new", + "-o", "UserKnownHostsFile=" + e.KnownHosts, + } + } + return append(host, "-o", "LogLevel=ERROR", "-o", "ConnectTimeout=5", "-o", "BatchMode=yes", "-i", keys.PrivatePath(), - } + ) } -// Args returns the argv (excluding argv[0]) for ssh into v. Host key checks -// are off on purpose: this is a loopback forward to a VM stoat just built, -// and live VMs are recreated constantly. -func Args(v *config.VM, extra ...string) []string { - a := append([]string{"-p", fmt.Sprint(v.SSHPort)}, connOptions()...) - a = append(a, User(v)+"@127.0.0.1") +// Args returns the argv (excluding argv[0]) for ssh to e. +func Args(e Endpoint, extra ...string) []string { + a := append([]string{"-p", fmt.Sprint(e.Port)}, connOptions(e)...) + a = append(a, e.User+"@"+e.Host) return append(a, extra...) } @@ -105,21 +111,20 @@ func preludeFor(v *config.VM, runtime string) string { } // CopyArgs returns the argv (excluding argv[0]) for scp between the host and -// v's guest. It shares every connection setting Args does (see connOptions) +// e's guest. It shares every connection setting Args does (see connOptions) // and differs only in the port flag, since scp's is capital -P. // -// toRemote picks the direction. true puts the guest spec -// ("user@127.0.0.1:remotePath") on the right, as scp's destination -// (core.CopyTo). false puts it on the left, as scp's source (core.CopyFrom). -// localPath is always a bare host path, never quoted or rewritten: it is -// scp's own argv element, not something a shell re-parses. +// toRemote picks the direction. true puts the guest spec on the right, as +// scp's destination (core.CopyTo). false puts it on the left, as scp's source +// (core.CopyFrom). localPath is always a bare host path, never quoted or +// rewritten: it is scp's own argv element, not something a shell re-parses. // // -q suppresses scp's interactive progress meter. This argv is built for // exec.CommandContext, never a terminal, so stray meter output would // otherwise get captured as if it were an error. -func CopyArgs(v *config.VM, localPath, remotePath string, toRemote bool) []string { - a := append([]string{"-P", fmt.Sprint(v.SSHPort), "-q"}, connOptions()...) - remoteSpec := User(v) + "@127.0.0.1:" + remotePath +func CopyArgs(e Endpoint, localPath, remotePath string, toRemote bool) []string { + a := append([]string{"-P", fmt.Sprint(e.Port), "-q"}, connOptions(e)...) + remoteSpec := e.User + "@" + e.Host + ":" + remotePath if toRemote { return append(a, localPath, remoteSpec) } @@ -140,9 +145,9 @@ func CopyArgs(v *config.VM, localPath, remotePath string, toRemote bool) []strin // expires elsewhere, sshx.Wait needs a caller-supplied timeout ceiling, and // duplicating the ~10-line dial is cheaper than reconciling those two // different contracts. -func Wait(ctx context.Context, v *config.VM, timeout time.Duration) error { +func Wait(ctx context.Context, e Endpoint, timeout time.Duration) error { deadline := time.Now().Add(timeout) - addr := fmt.Sprintf("127.0.0.1:%d", v.SSHPort) + addr := fmt.Sprintf("%s:%d", e.Host, e.Port) for { if err := ctx.Err(); err != nil { return err @@ -179,7 +184,7 @@ func Wait(ctx context.Context, v *config.VM, timeout time.Duration) error { case <-time.After(sleep): } } - return fmt.Errorf("%s: ssh not reachable on port %d after %s", v.Name, v.SSHPort, timeout) + return fmt.Errorf("%s: ssh not reachable at %s after %s", e.Name, addr, timeout) } // dialCtx dials addr, bounding the attempt by whichever of ctx or @@ -286,7 +291,7 @@ func cloudInitProbe(ctx context.Context, v *config.VM, log io.Writer) (cloudInit defer cancel() var out bytes.Buffer - ci := exec.CommandContext(probeCtx, "ssh", Args(v, argv...)...) + ci := exec.CommandContext(probeCtx, "ssh", Args(LocalEndpoint(v), argv...)...) ci.Cancel = func() error { return ci.Process.Signal(syscall.SIGTERM) } ci.WaitDelay = recipeShutdownGrace ci.Stdout = &out @@ -383,7 +388,7 @@ func RunCheck(ctx context.Context, v *config.VM, command string, timeout time.Du prelude = guest.Prelude(o, "sh") } body := prelude + "\n" + command + "\n" - cmd := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, []string{"sh", "-s"})...)...) + cmd := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, []string{"sh", "-s"})...)...) cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) } cmd.WaitDelay = healthShutdownGrace cmd.Stdin = strings.NewReader(body) @@ -416,7 +421,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { defer func() { _ = log.Close() }() fmt.Fprintf(log, "waiting for ssh on port %d…\n", v.SSHPort) - if err := Wait(ctx, v, WaitTimeout); err != nil { + if err := Wait(ctx, LocalEndpoint(v), WaitTimeout); err != nil { if ctx.Err() != nil { fmt.Fprintf(log, "CANCELLED: %v\n", err) } else { @@ -446,7 +451,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { if o, ok := guest.Lookup(v.OS); ok && strings.TrimSpace(o.Pkg.Setup) != "" { fmt.Fprintln(log, "refreshing the package index...") - st := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, []string{"sh", "-s"})...)...) + st := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, []string{"sh", "-s"})...)...) st.Cancel = func() error { return st.Process.Signal(syscall.SIGTERM) } st.WaitDelay = recipeShutdownGrace st.Stdin = strings.NewReader(guest.Prelude(o, "sh") + "stoat_pkg_setup\n") @@ -497,7 +502,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { if bootstrap := recipes.BootstrapScript(runtime, v.OS); bootstrap != "" { fmt.Fprintf(log, "ensuring %s is installed...\n", runtime) - bs := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, []string{"sh", "-s"})...)...) + bs := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, []string{"sh", "-s"})...)...) bs.Cancel = func() error { return bs.Process.Signal(syscall.SIGTERM) } bs.WaitDelay = recipeShutdownGrace bs.Stdin = strings.NewReader(guest.WithPrelude(bootstrap, preludeFor(v, "sh"))) @@ -513,7 +518,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { } } - cmd := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, recipes.InterpreterArgs(runtime))...)...) + cmd := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, recipes.InterpreterArgs(runtime))...)...) cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) } cmd.WaitDelay = recipeShutdownGrace cmd.Stdin = strings.NewReader(input) diff --git a/internal/sshx/sshx_test.go b/internal/sshx/sshx_test.go index 24679ce..8391483 100644 --- a/internal/sshx/sshx_test.go +++ b/internal/sshx/sshx_test.go @@ -18,7 +18,7 @@ import ( func TestArgs(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := strings.Join(Args(v), " ") + got := strings.Join(Args(LocalEndpoint(v)), " ") for _, want := range []string{ "-p 2201", @@ -40,7 +40,7 @@ func TestArgs(t *testing.T) { func TestArgsUsesConfiguredSSHUser(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x", SSHUser: "stoat"} - got := strings.Join(Args(v), " ") + got := strings.Join(Args(LocalEndpoint(v)), " ") if !strings.Contains(got, "stoat@127.0.0.1") { t.Errorf("expected stoat@127.0.0.1 in: %s", got) @@ -57,7 +57,7 @@ func TestArgsUsesConfiguredSSHUser(t *testing.T) { func TestCopyArgsUsesScpsPortFlagNotSSHs(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := CopyArgs(v, "/tmp/local", "/root/remote", true) + got := CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/remote", true) if !containsPair(got, "-P", "2201") { t.Errorf("missing -P 2201 in: %v", got) @@ -75,7 +75,7 @@ func TestCopyArgsUsesScpsPortFlagNotSSHs(t *testing.T) { func TestCopyArgsSharesConnOptionsWithArgs(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := strings.Join(CopyArgs(v, "/tmp/local", "/root/remote", true), " ") + got := strings.Join(CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/remote", true), " ") for _, want := range []string{ "-o StrictHostKeyChecking=no", @@ -96,12 +96,12 @@ func TestCopyArgsDirection(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - up := CopyArgs(v, "/tmp/local", "/root/remote", true) + up := CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/remote", true) if up[len(up)-2] != "/tmp/local" || up[len(up)-1] != "root@127.0.0.1:/root/remote" { t.Errorf("CopyTo argv = %v, want local then remote", up) } - down := CopyArgs(v, "/tmp/local", "/root/remote", false) + down := CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/remote", false) if down[len(down)-2] != "root@127.0.0.1:/root/remote" || down[len(down)-1] != "/tmp/local" { t.Errorf("CopyFrom argv = %v, want remote then local", down) } @@ -118,7 +118,7 @@ func TestCopyArgsDirection(t *testing.T) { func TestCopyArgsRemotePathIsNotShellQuoted(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := CopyArgs(v, "/tmp/local", "/root/my file.txt", true) + got := CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/my file.txt", true) want := "root@127.0.0.1:/root/my file.txt" if got[len(got)-1] != want { @@ -129,7 +129,7 @@ func TestCopyArgsRemotePathIsNotShellQuoted(t *testing.T) { func TestArgsExtraGoesAfterTarget(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := Args(v, "sh", "-s") + got := Args(LocalEndpoint(v), "sh", "-s") if got[len(got)-2] != "sh" || got[len(got)-1] != "-s" { t.Errorf("extra args must come last, got %v", got) } @@ -237,7 +237,7 @@ func TestWaitTimesOutWhenAcceptedButNoBanner(t *testing.T) { v := &config.VM{Name: "x", SSHPort: port, Dir: t.TempDir()} start := time.Now() - err := Wait(context.Background(), v, 500*time.Millisecond) + err := Wait(context.Background(), LocalEndpoint(v), 500*time.Millisecond) elapsed := time.Since(start) t.Logf("accept-without-banner: Wait took %s", elapsed) if err == nil { @@ -253,7 +253,7 @@ func TestWaitSucceedsOnceBannerArrives(t *testing.T) { v := &config.VM{Name: "x", SSHPort: port, Dir: t.TempDir()} start := time.Now() - err := Wait(context.Background(), v, 2*time.Second) + err := Wait(context.Background(), LocalEndpoint(v), 2*time.Second) elapsed := time.Since(start) t.Logf("accept-with-banner: Wait took %s", elapsed) if err != nil { @@ -287,7 +287,7 @@ func TestWaitSucceedsOnSlowBanner(t *testing.T) { v := &config.VM{Name: "x", SSHPort: port, Dir: t.TempDir()} start := time.Now() - err = Wait(context.Background(), v, 3*time.Second) + err = Wait(context.Background(), LocalEndpoint(v), 3*time.Second) elapsed := time.Since(start) t.Logf("slow-banner (500ms): Wait took %s", elapsed) if err != nil { @@ -299,7 +299,7 @@ func TestWaitTimesOutOnClosedPort(t *testing.T) { // Port 1 on loopback: reserved, nothing listens. v := &config.VM{Name: "x", SSHPort: 1, Dir: t.TempDir()} start := time.Now() - err := Wait(context.Background(), v, 300*time.Millisecond) + err := Wait(context.Background(), LocalEndpoint(v), 300*time.Millisecond) if err == nil { t.Fatal("Wait returned nil for a closed port") } @@ -352,7 +352,7 @@ func TestWaitCancelDuringRetrySleepReturnsPromptly(t *testing.T) { }() start := time.Now() - err := Wait(ctx, v, 10*time.Second) + err := Wait(ctx, LocalEndpoint(v), 10*time.Second) elapsed := time.Since(start) if !errors.Is(err, context.Canceled) { t.Fatalf("err = %v, want context.Canceled", err) @@ -374,7 +374,7 @@ func TestWaitAlreadyCancelledReturnsImmediately(t *testing.T) { cancel() start := time.Now() - err := Wait(ctx, v, 10*time.Second) + err := Wait(ctx, LocalEndpoint(v), 10*time.Second) elapsed := time.Since(start) if !errors.Is(err, context.Canceled) { t.Fatalf("err = %v, want context.Canceled", err) diff --git a/internal/tui/cloudinit.go b/internal/tui/cloudinit.go index deccd96..6478b47 100644 --- a/internal/tui/cloudinit.go +++ b/internal/tui/cloudinit.go @@ -104,7 +104,7 @@ func decodeCloudInitStatus(out []byte) string { func checkCloudInit(v core.VM) tea.Cmd { name := v.Name return func() tea.Msg { - out, err := exec.Command("ssh", sshx.Args(cfgVM(v), "cloud-init", "status", "--format", "json")...).Output() + out, err := exec.Command("ssh", sshx.Args(sshx.LocalEndpoint(cfgVM(v)), "cloud-init", "status", "--format", "json")...).Output() if exitErr, ok := err.(*exec.ExitError); err != nil && (!ok || exitErr.ExitCode() == 255) { // Not reachable yet is the normal case for the first ~30 seconds // of a boot, so it is a state, not an error. From 765426936fbd33f88ea9d00c36884d2d5716af7c Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 19:59:57 +0300 Subject: [PATCH 03/17] refactor(hostops): separate the data root from the hypervisor Reading and writing VM records needs no hypervisor. A host that cannot start a local VM can still own a data root. Signed-off-by: NovusEdge --- internal/cli/cli.go | 2 +- internal/config/config.go | 6 +++--- internal/hostcheck/checks_other.go | 2 +- internal/hostops/support_linux.go | 11 ++++++++--- internal/hostops/support_other.go | 14 +++++++++----- internal/hostops/support_test.go | 26 ++++++++++++++------------ internal/qemu/run.go | 4 ++-- 7 files changed, 38 insertions(+), 27 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index c509db1..890d1b4 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -499,7 +499,7 @@ func Main(args []string, version string, stdin io.Reader, stdout, stderr io.Writ // secrets, reading project scope, creating the data root, or initializing // logs. The independent capabilities command is dispatched before this // boundary by its owner and remains metadata-only. - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireDataRoot(); err != nil { return a.fail(stdout, stderr, err) } if len(a.Params) > 0 { diff --git a/internal/config/config.go b/internal/config/config.go index 6fd792a..547cece 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -202,7 +202,7 @@ func Root() string { // EnsureRoot creates the data root and its fixed subdirectories. func EnsureRoot() error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireDataRoot(); err != nil { return err } for _, d := range []string{"isos", "recipes"} { @@ -276,7 +276,7 @@ func (v *VM) ISOPath() string { // Save writes vm.toml, creating the VM directory if needed. func (v *VM) Save() error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireDataRoot(); err != nil { return err } if v.Dir == "" { @@ -381,7 +381,7 @@ var sshPortLine = regexp.MustCompile(`(?m)^\s*sshport\s*=\s*(\d+)\s*$`) // Delete removes the VM directory. It never touches isos/. func (v *VM) Delete() error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireDataRoot(); err != nil { return err } if v.Dir == "" || filepath.Dir(v.Dir) != Root() { diff --git a/internal/hostcheck/checks_other.go b/internal/hostcheck/checks_other.go index 72dd597..584f412 100644 --- a/internal/hostcheck/checks_other.go +++ b/internal/hostcheck/checks_other.go @@ -7,7 +7,7 @@ import "github.com/novusedge/stoat/internal/hostops" // RunChecks reports the native qualification boundary without probing Linux // binaries or /dev/kvm. Those requirements do not describe this host. func RunChecks(_ Distro) []Check { - err := hostops.RequireVM() + err := hostops.RequireLocalHypervisor() return []Check{{ Name: "native VM operations", Detail: err.Error(), diff --git a/internal/hostops/support_linux.go b/internal/hostops/support_linux.go index 3a341cd..dbba330 100644 --- a/internal/hostops/support_linux.go +++ b/internal/hostops/support_linux.go @@ -2,6 +2,11 @@ package hostops -// RequireVM permits native VM operations on Linux, the currently qualified -// host platform. -func RequireVM() error { return nil } +// RequireLocalHypervisor permits starting a VM on this host. Linux is the +// qualified platform. +func RequireLocalHypervisor() error { return nil } + +// RequireDataRoot permits owning a data root. Reading, writing and listing +// VM records needs no hypervisor, so a host that cannot start a local VM can +// still manage one that runs elsewhere. +func RequireDataRoot() error { return nil } diff --git a/internal/hostops/support_other.go b/internal/hostops/support_other.go index f4b6f9b..6e6e3af 100644 --- a/internal/hostops/support_other.go +++ b/internal/hostops/support_other.go @@ -4,14 +4,18 @@ package hostops import "runtime" -// RequireVM refuses native VM operations until this host platform has a -// complete runtime qualification. The message names what is missing, what -// still works, and where the work is tracked, so the refusal is actionable -// without a doc lookup. -func RequireVM() error { +// RequireLocalHypervisor refuses native VM operations until this host +// platform has a complete runtime qualification. The message names what is +// missing, what still works, and where the work is tracked, so the refusal +// is actionable without a doc lookup. +func RequireLocalHypervisor() error { return unsupportedError{Message(runtime.GOOS, runtime.GOARCH)} } +// RequireDataRoot permits owning a data root on any host. See the Linux +// implementation for why this is not gated. +func RequireDataRoot() error { return nil } + // unsupportedError carries Message's text as Error() while still satisfying // errors.Is(err, ErrUnsupported): fmt.Errorf("%w: %s", ...) would duplicate // Message's own "not qualified on " opening line. diff --git a/internal/hostops/support_test.go b/internal/hostops/support_test.go index 62df09c..36f7fe6 100644 --- a/internal/hostops/support_test.go +++ b/internal/hostops/support_test.go @@ -1,24 +1,26 @@ -//go:build !linux - package hostops import ( "errors" "runtime" - "strings" "testing" ) -func TestRequireVMUnsupportedHost(t *testing.T) { - err := RequireVM() - if err == nil { - t.Fatal("RequireVM() = nil on an unqualified native host") +func TestRequireDataRootAlwaysAllows(t *testing.T) { + if err := RequireDataRoot(); err != nil { + t.Errorf("RequireDataRoot() = %v, want nil on every platform", err) } - if !errors.Is(err, ErrUnsupported) { - t.Fatalf("RequireVM() = %v, want errors.Is(..., ErrUnsupported)", err) +} + +func TestRequireLocalHypervisorFollowsPlatform(t *testing.T) { + err := RequireLocalHypervisor() + if runtime.GOOS == "linux" { + if err != nil { + t.Errorf("RequireLocalHypervisor() = %v, want nil on linux", err) + } + return } - wantHost := runtime.GOOS + "/" + runtime.GOARCH - if !strings.Contains(err.Error(), wantHost) { - t.Errorf("RequireVM() = %q, want it to identify %s", err, wantHost) + if !errors.Is(err, ErrUnsupported) { + t.Errorf("RequireLocalHypervisor() = %v, want ErrUnsupported", err) } } diff --git a/internal/qemu/run.go b/internal/qemu/run.go index 52576f4..7827022 100644 --- a/internal/qemu/run.go +++ b/internal/qemu/run.go @@ -46,7 +46,7 @@ func diskWritten(v *config.VM) bool { // Start launches QEMU. -daemonize means it detaches itself; stoat supervises // nothing and tracks the process by pidfile. func Start(v *config.VM) error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } if Running(v) { @@ -147,7 +147,7 @@ func consoleCredential(v *config.VM, user string) string { // back to SIGTERM. The fallback is a power cut: fine for live VMs, lossy for // disk ones, which is why it is not the first move. func Stop(v *config.VM) error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } if !Running(v) { From fe16df981b19d79e83d4e7f3c6ed01fd3d412243 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 20:03:19 +0300 Subject: [PATCH 04/17] test(hostops): assert refusal message names the host platform Restores the strings.Contains check on GOOS/GOARCH that the rewrite dropped from TestRequireLocalHypervisorFollowsPlatform. Signed-off-by: NovusEdge --- internal/hostops/support_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/hostops/support_test.go b/internal/hostops/support_test.go index 36f7fe6..2579bef 100644 --- a/internal/hostops/support_test.go +++ b/internal/hostops/support_test.go @@ -3,6 +3,7 @@ package hostops import ( "errors" "runtime" + "strings" "testing" ) @@ -23,4 +24,7 @@ func TestRequireLocalHypervisorFollowsPlatform(t *testing.T) { if !errors.Is(err, ErrUnsupported) { t.Errorf("RequireLocalHypervisor() = %v, want ErrUnsupported", err) } + if !strings.Contains(err.Error(), runtime.GOOS+"/"+runtime.GOARCH) { + t.Errorf("RequireLocalHypervisor() = %v, want message to name %s/%s", err, runtime.GOOS, runtime.GOARCH) + } } From f3c56d43176ad81e00aca9fac189bff541fa306a Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 20:07:14 +0300 Subject: [PATCH 05/17] docs(troubleshooting): match the reworded ssh timeout sshx.Wait names the host and port together since it took an explicit endpoint. The doc heading and the TUI fixture still carried the old port-only wording. Signed-off-by: NovusEdge --- docs/troubleshooting.md | 4 ++-- internal/tui/provstep_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 67d3225..10e6157 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -111,10 +111,10 @@ every command and for the TUI: `STOAT_GRAPHICAL=0` is also the answer to the OpenGL error above, where a session exists but QEMU cannot draw on it. -## `ssh not reachable on port N after 1m30s` +## `ssh not reachable at HOST:PORT after 1m30s` ``` -: ssh not reachable on port after 1m30s +: ssh not reachable at : after 1m30s ``` **If this happens while applying recipes to a disk VM:** the VM is still diff --git a/internal/tui/provstep_test.go b/internal/tui/provstep_test.go index 97c30a6..7336c93 100644 --- a/internal/tui/provstep_test.go +++ b/internal/tui/provstep_test.go @@ -57,8 +57,8 @@ func TestReadProvStep(t *testing.T) { }, { "failed", - "waiting for ssh on port 2200…\nFAILED: work: ssh not reachable on port 2200 after 1m30s\n", - "waiting for ssh", "FAILED: work: ssh not reachable on port 2200 after 1m30s", + "waiting for ssh on port 2200…\nFAILED: work: ssh not reachable at 127.0.0.1:2200 after 1m30s\n", + "waiting for ssh", "FAILED: work: ssh not reachable at 127.0.0.1:2200 after 1m30s", }, } for _, c := range cases { From 4b0fa579396d7da951a5defe79e48a449d7f14bb Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 20:10:28 +0300 Subject: [PATCH 06/17] feat(provider): define the execution surface interface Signed-off-by: NovusEdge --- internal/config/config.go | 4 ++ internal/provider/provider.go | 61 ++++++++++++++++++++++++++++++ internal/provider/provider_test.go | 43 +++++++++++++++++++++ 3 files changed, 108 insertions(+) create mode 100644 internal/provider/provider.go create mode 100644 internal/provider/provider_test.go diff --git a/internal/config/config.go b/internal/config/config.go index 547cece..51d2463 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -94,6 +94,10 @@ type VM struct { // Written by the form at creation time; dispatch elsewhere in stoat // keys off Mode, not this field. Backend string `toml:"backend"` + // Provider is the execution surface: "qemu" for a local hypervisor VM, + // or a cloud provider's name. Empty means "qemu", which is what every + // vm.toml written before this field existed says. + Provider string `toml:"provider,omitempty"` // Base is the absolute path to the shared base image an overlay is // created from. Cloud mode only. Base string `toml:"base"` diff --git a/internal/provider/provider.go b/internal/provider/provider.go new file mode 100644 index 0000000..e40177f --- /dev/null +++ b/internal/provider/provider.go @@ -0,0 +1,61 @@ +// Package provider owns the execution surface a VM runs on. internal/qemu is +// one surface; a cloud API is another. It sits below internal/core and must +// never import it: core maps a Status onto core.State, and the reverse +// dependency would close a cycle. +package provider + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/novusedge/stoat/internal/capabilities" + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/sshx" +) + +// ErrUnknownProvider is For's answer for a vm.toml naming a provider this +// binary does not implement. It is never a fallback to qemu: a VM created by +// a newer stoat must not be operated as a local one. +var ErrUnknownProvider = errors.New("unknown provider") + +// Status is what a provider knows about a machine. Raw carries the +// provider's own status word so a frontend can report what the API said; +// it is empty for a provider whose states already match Running. +type Status struct { + Running bool + StartedAt time.Time + Raw string +} + +// Provider is one execution surface. Endpoint is the boundary: everything +// above it reaches the guest over ssh and needs no provider knowledge. +type Provider interface { + Name() string + Capabilities(v *config.VM) []capabilities.Capability + Start(ctx context.Context, v *config.VM) error + Stop(ctx context.Context, v *config.VM) error + Status(ctx context.Context, v *config.VM) (Status, error) + Endpoint(ctx context.Context, v *config.VM) (sshx.Endpoint, error) +} + +var registry = map[string]Provider{} + +// Register adds p under name. Implementations register from their own +// package's init, so importing an implementation is what makes it available. +func Register(name string, p Provider) { registry[name] = p } + +// For resolves v's provider. An empty field means "qemu": every vm.toml +// written before the field existed describes a local QEMU VM. +func For(v *config.VM) (Provider, error) { + name := v.Provider + if name == "" { + name = "qemu" + } + p, ok := registry[name] + if !ok { + return nil, fmt.Errorf("%w: %q", ErrUnknownProvider, name) + } + return p, nil +} diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go new file mode 100644 index 0000000..333ea0a --- /dev/null +++ b/internal/provider/provider_test.go @@ -0,0 +1,43 @@ +package provider + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/novusedge/stoat/internal/capabilities" + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/sshx" +) + +type stub struct{ name string } + +func (s stub) Name() string { return s.name } +func (stub) Capabilities(*config.VM) []capabilities.Capability { return nil } +func (stub) Start(context.Context, *config.VM) error { return nil } +func (stub) Stop(context.Context, *config.VM) error { return nil } +func (stub) Status(context.Context, *config.VM) (Status, error) { + return Status{Running: true, StartedAt: time.Unix(1, 0)}, nil +} +func (stub) Endpoint(context.Context, *config.VM) (sshx.Endpoint, error) { + return sshx.Endpoint{}, nil +} + +func TestForDefaultsToQemuWhenUnset(t *testing.T) { + Register("qemu", stub{name: "qemu"}) + p, err := For(&config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("For() error = %v", err) + } + if p.Name() != "qemu" { + t.Errorf("Name() = %q, want qemu for a VM with no provider field", p.Name()) + } +} + +func TestForRejectsUnknownProvider(t *testing.T) { + _, err := For(&config.VM{Name: "dev", Provider: "nope"}) + if !errors.Is(err, ErrUnknownProvider) { + t.Errorf("For() error = %v, want ErrUnknownProvider", err) + } +} From c13296c1e016217e13a6ed82b9205669c366bdc7 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 20:14:00 +0300 Subject: [PATCH 07/17] feat(provider): implement the qemu provider Signed-off-by: NovusEdge --- internal/provider/qemu/qemu.go | 35 ++++++++++++++++++++++ internal/provider/qemu/qemu_test.go | 46 +++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 internal/provider/qemu/qemu.go create mode 100644 internal/provider/qemu/qemu_test.go diff --git a/internal/provider/qemu/qemu.go b/internal/provider/qemu/qemu.go new file mode 100644 index 0000000..0aff6d5 --- /dev/null +++ b/internal/provider/qemu/qemu.go @@ -0,0 +1,35 @@ +// Package qemu implements provider.Provider over internal/qemu, the local +// hypervisor. It holds no logic of its own: every call forwards. +package qemu + +import ( + "context" + + "github.com/novusedge/stoat/internal/capabilities" + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" + "github.com/novusedge/stoat/internal/qemu" + "github.com/novusedge/stoat/internal/sshx" +) + +func init() { provider.Register("qemu", Provider{}) } + +type Provider struct{} + +func (Provider) Name() string { return "qemu" } + +// Capabilities returns nothing in C1. The capability set moves here in C2, +// alongside the code that consumes it. +func (Provider) Capabilities(*config.VM) []capabilities.Capability { return nil } + +func (Provider) Start(_ context.Context, v *config.VM) error { return qemu.Start(v) } + +func (Provider) Stop(_ context.Context, v *config.VM) error { return qemu.Stop(v) } + +func (Provider) Status(_ context.Context, v *config.VM) (provider.Status, error) { + return provider.Status{Running: qemu.Running(v), StartedAt: qemu.StartedAt(v)}, nil +} + +func (Provider) Endpoint(_ context.Context, v *config.VM) (sshx.Endpoint, error) { + return sshx.LocalEndpoint(v), nil +} diff --git a/internal/provider/qemu/qemu_test.go b/internal/provider/qemu/qemu_test.go new file mode 100644 index 0000000..8424b74 --- /dev/null +++ b/internal/provider/qemu/qemu_test.go @@ -0,0 +1,46 @@ +package qemu + +import ( + "context" + "testing" + + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" +) + +func TestRegisteredAsQemu(t *testing.T) { + p, err := provider.For(&config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("For() error = %v", err) + } + if p.Name() != "qemu" { + t.Errorf("Name() = %q, want qemu", p.Name()) + } +} + +func TestEndpointIsLoopback(t *testing.T) { + p := Provider{} + e, err := p.Endpoint(context.Background(), &config.VM{Name: "dev", SSHPort: 2222}) + if err != nil { + t.Fatalf("Endpoint() error = %v", err) + } + if e.Host != "127.0.0.1" || e.Port != 2222 { + t.Errorf("Endpoint() = %s:%d, want 127.0.0.1:2222", e.Host, e.Port) + } + if e.KnownHosts != "" { + t.Errorf("KnownHosts = %q, want empty: a loopback forward is not pinned", e.KnownHosts) + } +} + +func TestStatusReportsStoppedForAnUnstartedVM(t *testing.T) { + s, err := Provider{}.Status(context.Background(), &config.VM{Name: "dev", Dir: t.TempDir()}) + if err != nil { + t.Fatalf("Status() error = %v", err) + } + if s.Running { + t.Error("Running = true, want false for a VM with no pidfile") + } + if s.Raw != "" { + t.Errorf("Raw = %q, want empty: qemu has no status word of its own", s.Raw) + } +} From ac15a604fb0055db1856aa34ce35f42f25468959 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 20:28:52 +0300 Subject: [PATCH 08/17] refactor(core): reach the machine through its provider Lifecycle and endpoint calls go through provider.For. Screenshot, snapshots and display still call internal/qemu directly; they get capability checks in C2. internal/capabilities imported internal/core for HostCheck and three sentinel errors. Once core imports provider, and provider imports capabilities, that closed a cycle. HostCheck's fields already matched internal/hostcheck.Check, so capabilities now takes that instead, with a converter at the two callers that build capabilities.Input. The sentinel errors move to a new internal/coreerr leaf package; core re-exports the same values under its existing names so every core.ErrXxx caller compiles unchanged and errors.Is still matches by identity. Signed-off-by: NovusEdge --- internal/capabilities/build.go | 6 +- internal/capabilities/build_test.go | 19 ++--- internal/capabilities/load.go | 10 +-- internal/capabilities/model.go | 4 +- internal/cli/run_capabilities.go | 15 +++- internal/core/access.go | 11 ++- internal/core/apply.go | 30 +++++-- internal/core/autorestart.go | 12 +-- internal/core/copy.go | 17 +++- internal/core/core.go | 8 +- internal/core/exec.go | 7 +- internal/core/forward.go | 8 +- internal/core/prune.go | 14 ++-- internal/core/update.go | 8 +- internal/core/vm.go | 120 ++++++++++++++++++++-------- internal/core/vm_test.go | 22 ++++- internal/core/wait.go | 22 +++-- internal/coreerr/coreerr.go | 15 ++++ internal/mcpsrv/tools_read.go | 15 +++- 19 files changed, 271 insertions(+), 92 deletions(-) create mode 100644 internal/coreerr/coreerr.go diff --git a/internal/capabilities/build.go b/internal/capabilities/build.go index 8893182..db5f393 100644 --- a/internal/capabilities/build.go +++ b/internal/capabilities/build.go @@ -3,7 +3,7 @@ package capabilities import ( "runtime" - "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/hostcheck" ) // Build evaluates the supplied host and metadata observations without I/O. @@ -131,14 +131,14 @@ func currentEntry(name, status, scope string, requirements []Requirement, limits return Capability{Name: name, Status: status, Scope: scope, Requirements: requirements, Limits: limits, Reason: reason, Evidence: []Evidence{implementationEvidence(name)}} } -func qemuProfile(checks []core.HostCheck) Profile { +func qemuProfile(checks []hostcheck.Check) Profile { requirements := []Requirement{ requirement("host_tool", "qemu-system-x86_64", ""), requirement("host_tool", "qemu-img", ""), requirement("host_device", "/dev/kvm", ""), } p := Profile{Name: "qemu-x86_64", Status: StatusUnknown, Scope: ScopeHost, Requirements: requirements, Limits: []Limit{}, Evidence: []Evidence{}} - byName := make(map[string]core.HostCheck, len(checks)) + byName := make(map[string]hostcheck.Check, len(checks)) for _, c := range checks { if _, exists := byName[c.Name]; !exists { byName[c.Name] = c diff --git a/internal/capabilities/build_test.go b/internal/capabilities/build_test.go index d079766..cc0c55f 100644 --- a/internal/capabilities/build_test.go +++ b/internal/capabilities/build_test.go @@ -8,11 +8,12 @@ import ( "strconv" "testing" - "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/coreerr" + "github.com/novusedge/stoat/internal/hostcheck" ) -func passingChecks() []core.HostCheck { - return []core.HostCheck{ +func passingChecks() []hostcheck.Check { + return []hostcheck.Check{ {Name: "qemu-system-x86_64", OK: true}, {Name: "qemu-img", OK: true}, {Name: "/dev/kvm", OK: true}, @@ -174,7 +175,7 @@ func TestCapabilitiesBoundaries(t *testing.T) { } t.Run("partial required host observations are unknown", func(t *testing.T) { - report := Build(Input{HostChecks: []core.HostCheck{{Name: "qemu-system-x86_64", OK: true}}}) + report := Build(Input{HostChecks: []hostcheck.Check{{Name: "qemu-system-x86_64", OK: true}}}) profile := profileEntry(t, report.Profiles, "qemu-x86_64") if profile.Status != "unknown" || profile.Reason == nil || profile.Reason.Code != "host_probe_unavailable" { t.Errorf("qemu-x86_64 = %+v, want unknown/host_probe_unavailable", profile) @@ -182,7 +183,7 @@ func TestCapabilitiesBoundaries(t *testing.T) { }) t.Run("failed fully observed host requirement is limited", func(t *testing.T) { - report := Build(Input{HostChecks: []core.HostCheck{ + report := Build(Input{HostChecks: []hostcheck.Check{ {Name: "qemu-system-x86_64", OK: true}, {Name: "qemu-img", OK: false}, {Name: "/dev/kvm", OK: true}, @@ -197,7 +198,7 @@ func TestCapabilitiesBoundaries(t *testing.T) { }) t.Run("empty host observations are unknown", func(t *testing.T) { - report := Build(Input{HostChecks: []core.HostCheck{}}) + report := Build(Input{HostChecks: []hostcheck.Check{}}) profile := profileEntry(t, report.Profiles, "qemu-x86_64") if profile.Status != "unknown" || profile.Reason == nil || profile.Reason.Code != "host_probe_unavailable" { t.Errorf("qemu-x86_64 = %+v, want unknown/host_probe_unavailable", profile) @@ -248,11 +249,11 @@ func TestCapabilitiesBoundaries(t *testing.T) { } for _, name := range []string{"", "../escape", "bad/name", "-bad", "_bad", "bad name"} { - if _, err := LoadTarget(name); !errors.Is(err, core.ErrInvalidSpec) { + if _, err := LoadTarget(name); !errors.Is(err, coreerr.ErrInvalidSpec) { t.Errorf("LoadTarget(%q) error = %v, want ErrInvalidSpec", name, err) } } - if _, err := LoadTarget("missing"); !errors.Is(err, core.ErrNotFound) { + if _, err := LoadTarget("missing"); !errors.Is(err, coreerr.ErrNotFound) { t.Errorf("LoadTarget(missing) error = %v, want ErrNotFound", err) } brokenDir := filepath.Join(root, "broken") @@ -262,7 +263,7 @@ func TestCapabilitiesBoundaries(t *testing.T) { if err := os.WriteFile(filepath.Join(brokenDir, "vm.toml"), []byte("name = \"broken\"\nmode = \"cloud\n"), 0o644); err != nil { t.Fatal(err) } - if _, err := LoadTarget("broken"); !errors.Is(err, core.ErrBroken) { + if _, err := LoadTarget("broken"); !errors.Is(err, coreerr.ErrBroken) { t.Errorf("LoadTarget(broken) error = %v, want ErrBroken", err) } }) diff --git a/internal/capabilities/load.go b/internal/capabilities/load.go index 8b46fe4..f0e33c6 100644 --- a/internal/capabilities/load.go +++ b/internal/capabilities/load.go @@ -7,7 +7,7 @@ import ( "regexp" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/coreerr" ) var targetNameRE = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`) @@ -15,18 +15,18 @@ var targetNameRE = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`) // LoadTarget reads one VM's stored metadata without inspecting runtime state. func LoadTarget(name string) (Target, error) { if !targetNameRE.MatchString(name) { - return Target{}, fmt.Errorf("%w: VM name %q", core.ErrInvalidSpec, name) + return Target{}, fmt.Errorf("%w: VM name %q", coreerr.ErrInvalidSpec, name) } path := filepath.Join(config.Root(), name, "vm.toml") if _, err := os.Stat(path); err != nil { if os.IsNotExist(err) { - return Target{}, fmt.Errorf("%w: %s", core.ErrNotFound, name) + return Target{}, fmt.Errorf("%w: %s", coreerr.ErrNotFound, name) } - return Target{}, fmt.Errorf("%w: %s: %v", core.ErrBroken, name, err) + return Target{}, fmt.Errorf("%w: %s: %v", coreerr.ErrBroken, name, err) } v, err := config.Load(name) if err != nil { - return Target{}, fmt.Errorf("%w: %s: %v", core.ErrBroken, name, err) + return Target{}, fmt.Errorf("%w: %s: %v", coreerr.ErrBroken, name, err) } return Target{Name: name, Mode: v.Mode, AgentAccess: v.AgentAccess}, nil } diff --git a/internal/capabilities/model.go b/internal/capabilities/model.go index 8724128..a849462 100644 --- a/internal/capabilities/model.go +++ b/internal/capabilities/model.go @@ -1,6 +1,6 @@ package capabilities -import "github.com/novusedge/stoat/internal/core" +import "github.com/novusedge/stoat/internal/hostcheck" const ( StatusSupported = "supported" @@ -103,6 +103,6 @@ type Evidence struct { type Input struct { Version string ProjectState string - HostChecks []core.HostCheck + HostChecks []hostcheck.Check Target *Target } diff --git a/internal/cli/run_capabilities.go b/internal/cli/run_capabilities.go index 64724d1..7253917 100644 --- a/internal/cli/run_capabilities.go +++ b/internal/cli/run_capabilities.go @@ -7,8 +7,21 @@ import ( "github.com/novusedge/stoat/internal/capabilities" "github.com/novusedge/stoat/internal/cli/wire" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/hostcheck" ) +// toHostChecks adapts core.Doctor's result to capabilities.Input.HostChecks. +// The two types have identical fields but capabilities cannot import core: +// core now imports provider, which imports capabilities, so the reverse +// import would close a cycle. +func toHostChecks(cs []core.HostCheck) []hostcheck.Check { + out := make([]hostcheck.Check, len(cs)) + for i, c := range cs { + out[i] = hostcheck.Check{Name: c.Name, OK: c.OK, Detail: c.Detail, Fix: c.Fix, Optional: c.Optional} + } + return out +} + func runCapabilities(a *Args, version string, stdout, stderr io.Writer) int { var target *capabilities.Target if a.VM != "" { @@ -25,7 +38,7 @@ func runCapabilities(a *Args, version string, stdout, stderr io.Writer) int { report := capabilities.Build(capabilities.Input{ Version: version, ProjectState: projectState, - HostChecks: core.Doctor(), + HostChecks: toHostChecks(core.Doctor()), Target: target, }) if a.JSON { diff --git a/internal/core/access.go b/internal/core/access.go index 15fee1f..8ebafff 100644 --- a/internal/core/access.go +++ b/internal/core/access.go @@ -2,6 +2,7 @@ package core import ( "bytes" + "context" "errors" "fmt" "io" @@ -30,7 +31,15 @@ func SSHCommand(name string) ([]string, error) { if err != nil { return nil, err } - return append([]string{"ssh"}, sshx.Args(sshx.LocalEndpoint(v))...), nil + p, err := providerFor(v) + if err != nil { + return nil, err + } + ep, err := p.Endpoint(context.Background(), v) + if err != nil { + return nil, err + } + return append([]string{"ssh"}, sshx.Args(ep)...), nil } // Which selects one of a VM's two log files. diff --git a/internal/core/apply.go b/internal/core/apply.go index bf21401..e59f62c 100644 --- a/internal/core/apply.go +++ b/internal/core/apply.go @@ -13,7 +13,6 @@ import ( "github.com/novusedge/stoat/internal/backend" "github.com/novusedge/stoat/internal/cloudinit" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/recipes" "github.com/novusedge/stoat/internal/sshx" ) @@ -142,7 +141,11 @@ func resolveTargets(v *config.VM, only []string) ([]string, error) { // applyLocked is Apply's body, run while Apply holds name's provision lock. func applyLocked(ctx context.Context, v *config.VM, opts ApplyOpts) error { - if !qemu.Running(v) { + state, err := StateOf(ctx, v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s", ErrNotRunning, v.Name) } @@ -283,10 +286,19 @@ func applyLocked(ctx context.Context, v *config.VM, opts ApplyOpts) error { func rebootAndWait(ctx context.Context, v *config.VM, recipe string) error { appendProvisionLog(v, fmt.Sprintf("rebooting %s to finish %s...\n", v.Name, recipe)) + p, err := providerFor(v) + if err != nil { + return err + } + ep, err := p.Endpoint(ctx, v) + if err != nil { + return err + } + // `reboot` tears down the ssh session before the process can report an // exit status back to this host, so cmd.Run() returning an error here is // expected and not a failure signal; only the wait below is. - cmd := exec.CommandContext(ctx, "ssh", sshx.Args(sshx.LocalEndpoint(v), "reboot")...) + cmd := exec.CommandContext(ctx, "ssh", sshx.Args(ep, "reboot")...) _ = cmd.Run() // The pre-reboot sshd can keep answering for a moment after the reboot @@ -328,7 +340,7 @@ func appendProvisionWarnings(v *config.VM, warnings []string) { // discoverCloudInitApplied rebuilds v.Applied for a cloudinit VM from the // marker files cloud-init left after first boot. It runs over ssh, so the VM -// must be reachable; applyLocked calls it only after the qemu.Running check. +// must be reachable; applyLocked calls it only after the running check. // // It no-ops unless the backend is cloudinit and v.Applied is still empty: once // a post-boot Apply has recorded state, that state is authoritative and this @@ -344,7 +356,15 @@ func discoverCloudInitApplied(ctx context.Context, v *config.VM) ([]string, erro return nil, nil } script := fmt.Sprintf("for marker in %s/*; do case \"$marker\" in *.out) continue;; esac; [ -f \"$marker\" ] || continue; name=$(basename \"$marker\"); printf '===%%s\\n' \"$name\"; cat \"$marker.out\" 2>/dev/null; done", cloudinit.MarkerDir) - out, err := exec.CommandContext(ctx, "ssh", sshx.Args(sshx.LocalEndpoint(v), script)...).Output() + p, err := providerFor(v) + if err != nil { + return nil, err + } + ep, err := p.Endpoint(ctx, v) + if err != nil { + return nil, err + } + out, err := exec.CommandContext(ctx, "ssh", sshx.Args(ep, script)...).Output() if err != nil { return nil, nil // marker dir missing or a transient ssh error; discover nothing } diff --git a/internal/core/autorestart.go b/internal/core/autorestart.go index 0658563..379bb71 100644 --- a/internal/core/autorestart.go +++ b/internal/core/autorestart.go @@ -3,8 +3,6 @@ package core import ( "context" "errors" - - "github.com/novusedge/stoat/internal/qemu" ) // AutoRestartAfterInstall waits for an uninstalled disk VM's unattended @@ -19,14 +17,18 @@ import ( // apkovlBackend.Args adds -no-reboot for this exact boot, so a successful // install's own "poweroff" (internal/apkovl's installScript) exits QEMU // instead of re-entering the installer. A failed install leaves the -// installer's shell running, so qemu.Running never turns false and this -// call rides out ctx's deadline instead of restarting. +// installer's shell running, so the VM never stops and this call rides out +// ctx's deadline instead of restarting. func AutoRestartAfterInstall(ctx context.Context, name string) (bool, error) { v, err := load(name) if err != nil { return false, err } - if v.Mode != "disk" || v.Installed || !qemu.Running(v) { + if v.Mode != "disk" || v.Installed { + return false, nil + } + state, err := StateOf(ctx, v) + if err != nil || state != StateRunning { return false, nil } diff --git a/internal/core/copy.go b/internal/core/copy.go index 9e15adc..c999d48 100644 --- a/internal/core/copy.go +++ b/internal/core/copy.go @@ -8,7 +8,6 @@ import ( "os/exec" "strings" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/sshx" ) @@ -52,12 +51,24 @@ func doCopy(ctx context.Context, name, localPath, remotePath string, toRemote bo // A stopped VM is the common cause of an scp failure. Report it as // ErrNotRunning rather than let scp's "connection refused" surface as a // bare non-zero exit. Exec checks the same way. - if !qemu.Running(v) { + state, err := StateOf(ctx, v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s", ErrNotRunning, name) } + p, err := providerFor(v) + if err != nil { + return err + } + ep, err := p.Endpoint(ctx, v) + if err != nil { + return err + } var stderr bytes.Buffer - c := exec.CommandContext(ctx, "scp", sshx.CopyArgs(sshx.LocalEndpoint(v), localPath, remotePath, toRemote)...) + c := exec.CommandContext(ctx, "scp", sshx.CopyArgs(ep, localPath, remotePath, toRemote)...) c.Stderr = &stderr err = c.Run() diff --git a/internal/core/core.go b/internal/core/core.go index cd48bbe..7f42ef8 100644 --- a/internal/core/core.go +++ b/internal/core/core.go @@ -19,16 +19,20 @@ import ( "strings" "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/coreerr" "github.com/novusedge/stoat/internal/recipes" ) // Typed errors, because every caller branches on them and string matching is // how that goes wrong. Each wraps with the specific subject. +// +// ErrNotFound and ErrInvalidSpec are coreerr's values, not new ones: capabilities.LoadTarget +// wraps the same sentinels, and errors.Is matches by identity. var ( - ErrNotFound = errors.New("not found") + ErrNotFound = coreerr.ErrNotFound ErrNameTaken = errors.New("name already taken") ErrImageNotDownloaded = errors.New("image not downloaded") - ErrInvalidSpec = errors.New("invalid spec") + ErrInvalidSpec = coreerr.ErrInvalidSpec ErrInUse = errors.New("in use") // ErrRecipeNotApplicable: a recipe was named that this VM's OS and // backend cannot run. Typed because a caller retrying with a corrected diff --git a/internal/core/exec.go b/internal/core/exec.go index 7516770..234a157 100644 --- a/internal/core/exec.go +++ b/internal/core/exec.go @@ -6,7 +6,6 @@ import ( "fmt" "strings" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/sshx" ) @@ -56,7 +55,11 @@ func Exec(ctx context.Context, name string, cmd []string) (ExecResult, error) { // is the most common reason Exec cannot connect. ErrNotRunning is // faster and clearer than a bare 255 with "connection refused" buried // in stderr. - if !qemu.Running(v) { + state, err := StateOf(ctx, v) + if err != nil { + return ExecResult{}, err + } + if state != StateRunning { return ExecResult{}, fmt.Errorf("%w: %s", ErrNotRunning, name) } diff --git a/internal/core/forward.go b/internal/core/forward.go index afca1f0..886e6ae 100644 --- a/internal/core/forward.go +++ b/internal/core/forward.go @@ -1,13 +1,13 @@ package core import ( + "context" "fmt" "path/filepath" "strconv" "strings" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" ) // PortForward is an alias, not a copy, of config.PortForward. This package @@ -71,7 +71,11 @@ func Forward(name string, fwds []PortForward) (active bool, err error) { if err := v.Save(); err != nil { return false, err } - return !qemu.Running(v), nil + state, err := StateOf(context.Background(), v) + if err != nil { + return false, err + } + return state != StateRunning, nil } // validateForwards checks a proposed forward list against everything that diff --git a/internal/core/prune.go b/internal/core/prune.go index 030b35d..ac98e46 100644 --- a/internal/core/prune.go +++ b/internal/core/prune.go @@ -1,6 +1,7 @@ package core import ( + "context" "os" "path/filepath" "regexp" @@ -9,7 +10,6 @@ import ( "time" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" ) // partStaleAfter is how long a *.part file under isos/ sits with no mtime @@ -158,12 +158,12 @@ func pruneBroken(dryRun bool) ([]PruneItem, error) { dir := filepath.Join(config.Root(), b.Name) bv := &config.VM{Name: b.Name, Dir: dir} - // qemu.Running only needs v.Dir and v.PidPath(), both derivable - // without a parsed vm.toml, so it works on a broken VM too. The VM - // may have started before the edit that broke vm.toml. Destroy - // refuses to touch a running VM; Prune must refuse the same way, - // even acting in bulk. - if qemu.Running(bv) { + // StateOf only needs v.Dir and v.PidPath(), both derivable without a + // parsed vm.toml, so it works on a broken VM too. The VM may have + // started before the edit that broke vm.toml. Destroy refuses to + // touch a running VM; Prune must refuse the same way, even acting + // in bulk. + if state, err := StateOf(context.Background(), bv); err == nil && state == StateRunning { continue } diff --git a/internal/core/update.go b/internal/core/update.go index 6e38ebc..c394b14 100644 --- a/internal/core/update.go +++ b/internal/core/update.go @@ -1,6 +1,7 @@ package core import ( + "context" "errors" "fmt" "os" @@ -9,7 +10,6 @@ import ( "strings" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/recipes" ) @@ -559,7 +559,11 @@ func validateDiskGrow(v *config.VM, size string) (string, error) { // because qemu-img resize runs against the live file below. RAM, CPUs and // SSHPort defer to next start instead. ErrAlreadyRunning matches how Destroy // signals the same "needs stopped, isn't" refusal. - if qemu.Running(v) { + state, err := StateOf(context.Background(), v) + if err != nil { + return "", err + } + if state == StateRunning { return "", fmt.Errorf("%w: disk: stop %s before resizing its disk", ErrAlreadyRunning, v.Name) } diff --git a/internal/core/vm.go b/internal/core/vm.go index 4116912..fbe133b 100644 --- a/internal/core/vm.go +++ b/internal/core/vm.go @@ -1,6 +1,7 @@ package core import ( + "context" "errors" "fmt" "os" @@ -9,9 +10,11 @@ import ( "time" "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/coreerr" "github.com/novusedge/stoat/internal/guest" "github.com/novusedge/stoat/internal/iso" - "github.com/novusedge/stoat/internal/qemu" + "github.com/novusedge/stoat/internal/provider" + _ "github.com/novusedge/stoat/internal/provider/qemu" "github.com/novusedge/stoat/internal/recipes" ) @@ -19,9 +22,9 @@ import ( // fresh from the process table and the filesystem each time. // // The state model has six states. Only three are knowable today. StateStopped -// and StateRunning come from qemu.Running, which checks pid liveness and -// matches /proc//cmdline, so a reused pid never reads as running. -// StateBroken comes from a vm.toml that exists but fails to parse +// and StateRunning come from the VM's provider (StateOf); qemu's own check +// matches pid liveness against /proc//cmdline, so a reused pid never +// reads as running. StateBroken comes from a vm.toml that exists but fails to parse // (config.ListBroken's concept). // // StateStarting, StateApplying and StateFailed are not declared here. No @@ -57,7 +60,10 @@ var ErrAlreadyRunning = errors.New("already running") // destroyed) rather than looking deleted. Start/Stop/Destroy have no VM view // to attach a state to, so a typed error is the only way to say "broken" to // those callers. -var ErrBroken = errors.New("broken vm.toml") +// +// It is coreerr's value: capabilities.LoadTarget wraps the same sentinel, +// and errors.Is matches by identity. +var ErrBroken = coreerr.ErrBroken // Paths are the on-disk locations for one VM, resolved once here so a caller // (an MCP server describing a VM, a TUI detail screen) does not have to @@ -109,7 +115,7 @@ type RecipeState struct { // VM answers "what is this VM doing right now". It is not the on-disk // record. config.VM is vm.toml: what was asked for, valid the instant it was // last saved. It says nothing about "is it running" on its own; that needs -// qemu.Running too. core.VM combines both, computed together, so a caller +// StateOf too. core.VM combines both, computed together, so a caller // never has one without the other. // // The design doc's VM (§1) also lists Progress and Created. Progress is @@ -261,8 +267,12 @@ func checkGuest(v *config.VM) error { // State and Paths are the two things config.VM cannot answer for itself. func fromConfigUnchecked(v *config.VM) VM { state := StateStopped - if qemu.Running(v) { - state = StateRunning + var startedAt time.Time + if p, err := providerFor(v); err == nil { + if s, err := p.Status(context.Background(), v); err == nil && s.Running { + state = StateRunning + startedAt = s.StartedAt + } } osName, backend := inferMissing(v) return VM{ @@ -274,7 +284,7 @@ func fromConfigUnchecked(v *config.VM) VM { Mode: v.Mode, Backend: backend, State: state, - StartedAt: qemu.StartedAt(v), + StartedAt: startedAt, RAM: v.RAM, CPUs: v.CPUs, CPUModel: v.CPUModel, @@ -511,48 +521,92 @@ func Get(name string) (VM, error) { return fromConfigChecked(v) } -// Start launches VM name. It wraps qemu.Start; the actual work (pidfile, -// backend Prepare, marking a disk VM installed) lives there. +// providerFor resolves v's execution surface. Every core call site goes +// through here instead of provider.For directly, so C2's capability checks +// have one place to land. +func providerFor(v *config.VM) (provider.Provider, error) { return provider.For(v) } + +// StateOf asks v's provider what the machine is doing. A provider this +// binary does not implement is an error, never a fallback to qemu: a VM +// created by a newer stoat must not be operated as a local one. +func StateOf(ctx context.Context, v *config.VM) (State, error) { + p, err := providerFor(v) + if err != nil { + return StateBroken, err + } + s, err := p.Status(ctx, v) + if err != nil { + return StateBroken, err + } + if s.Running { + return StateRunning, nil + } + return StateStopped, nil +} + +// Start launches VM name through its provider. The actual work (pidfile, +// backend Prepare, marking a disk VM installed) lives in the qemu provider +// for a local VM. // -// qemu.Start already refuses to run twice, but with an untyped error -// ("%s is already running") that a caller can only detect by string -// matching. Start checks qemu.Running first and returns typed -// ErrAlreadyRunning instead; qemu.Start's own check never fires, because +// The provider's own Start already refuses to run twice, but with an +// untyped error ("%s is already running") that a caller can only detect by +// string matching. Start checks StateOf first and returns typed +// ErrAlreadyRunning instead; the provider's own check never fires, because // this function has already returned. func Start(name string) error { v, err := load(name) if err != nil { return err } - if qemu.Running(v) { + state, err := StateOf(context.Background(), v) + if err != nil { + return err + } + if state == StateRunning { return fmt.Errorf("%w: %s", ErrAlreadyRunning, name) } - return qemu.Start(v) + p, err := providerFor(v) + if err != nil { + return err + } + return p.Start(context.Background(), v) } // EnsureRunning refuses with ErrNotRunning when v is not running. It exists // so a caller above core, such as mcpsrv, answers the same error Stop and -// Exec give without importing qemu.Running itself. +// Exec give without calling StateOf itself. func EnsureRunning(v *config.VM) error { - if !qemu.Running(v) { + state, err := StateOf(context.Background(), v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s", ErrNotRunning, v.Name) } return nil } -// Stop powers down VM name. qemu.Stop treats "already stopped" as a -// successful no-op; Stop does not. The CLI's `down` (internal/cli/cli.go's -// runDown) already refuses a stopped VM as a failure, and Stop preserves -// that behavior. +// Stop powers down VM name through its provider. The provider's own Stop +// treats "already stopped" as a successful no-op; Stop does not. The CLI's +// `down` (internal/cli/cli.go's runDown) already refuses a stopped VM as a +// failure, and Stop preserves that behavior. func Stop(name string) error { v, err := load(name) if err != nil { return err } - if !qemu.Running(v) { + state, err := StateOf(context.Background(), v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s", ErrNotRunning, name) } - return qemu.Stop(v) + p, err := providerFor(v) + if err != nil { + return err + } + return p.Stop(context.Background(), v) } // Destroy removes VM name's directory and vm.toml. @@ -585,14 +639,14 @@ func Destroy(name string) error { // why Get/List surface broken VMs instead of hiding them: they can // be cleared instead of sitting forever unparseable. // - // The running check below still applies. qemu.Running only needs - // Dir, which is reconstructed here; it does not need a parsed - // config.VM. Skipping this check let a vm.toml corrupted after its - // VM was started bypass the running-VM refusal, deleting the - // directory, pidfile, monitor socket and disk out from under a live - // qemu process. + // The running check below still applies. StateOf only needs Dir, + // which is reconstructed here; it does not need a parsed config.VM. + // Skipping this check let a vm.toml corrupted after its VM was + // started bypass the running-VM refusal, deleting the directory, + // pidfile, monitor socket and disk out from under a live qemu + // process. bv := &config.VM{Name: name, Dir: filepath.Join(config.Root(), name)} - if qemu.Running(bv) { + if state, err := StateOf(context.Background(), bv); err == nil && state == StateRunning { return fmt.Errorf("%w: %s: stop it first", ErrAlreadyRunning, name) } return bv.Delete() @@ -600,7 +654,7 @@ func Destroy(name string) error { if err != nil { return err } - if qemu.Running(v) { + if state, err := StateOf(context.Background(), v); err == nil && state == StateRunning { return fmt.Errorf("%w: %s: stop it first", ErrAlreadyRunning, name) } return v.Delete() diff --git a/internal/core/vm_test.go b/internal/core/vm_test.go index 734fb01..f1f513b 100644 --- a/internal/core/vm_test.go +++ b/internal/core/vm_test.go @@ -1,6 +1,7 @@ package core import ( + "context" "errors" "os" "path/filepath" @@ -9,6 +10,7 @@ import ( "time" "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" "github.com/novusedge/stoat/internal/testutil" ) @@ -550,7 +552,7 @@ func TestAppliedNilWhenNoRecipesApplied(t *testing.T) { } // TestDestroyRefusesARunningBrokenVM pins a real bug: Destroy's broken-VM -// branch once skipped the running check. qemu.Running needs only Dir, which +// branch once skipped the running check. StateOf needs only Dir, which // that branch reconstructs. Skipping the check let a vm.toml corrupted // after its VM was started bypass the refusal that applies to every // healthy VM, deleting the pidfile, monitor socket and disk from under a @@ -569,3 +571,21 @@ func TestDestroyRefusesARunningBrokenVM(t *testing.T) { t.Fatalf("the directory was deleted from under a running qemu: %v", err) } } + +func TestStateUsesTheProvider(t *testing.T) { + v := &config.VM{Name: "dev", Dir: t.TempDir()} + got, err := StateOf(context.Background(), v) + if err != nil { + t.Fatalf("StateOf() error = %v", err) + } + if got != StateStopped { + t.Errorf("StateOf() = %q, want %q for a VM with no running process", got, StateStopped) + } +} + +func TestStateRejectsAnUnknownProvider(t *testing.T) { + v := &config.VM{Name: "dev", Dir: t.TempDir(), Provider: "nope"} + if _, err := StateOf(context.Background(), v); !errors.Is(err, provider.ErrUnknownProvider) { + t.Errorf("StateOf() error = %v, want ErrUnknownProvider", err) + } +} diff --git a/internal/core/wait.go b/internal/core/wait.go index 0dabacb..07afab2 100644 --- a/internal/core/wait.go +++ b/internal/core/wait.go @@ -14,7 +14,6 @@ import ( "github.com/novusedge/stoat/internal/backend" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" ) // Until is a state Wait can block for. It is a small, closed set distinct @@ -32,7 +31,7 @@ const ( // UntilApplied is the most recent recipe run having finished // successfully. See waitApplied. UntilApplied Until = "applied" - // UntilStopped is qemu.Running turning false. + // UntilStopped is the provider's state turning to not-running. UntilStopped Until = "stopped" // UntilHealthy is every applied recipe's health check passing. UntilHealthy Until = "healthy" @@ -171,7 +170,11 @@ func healthFailure(verdict RecipeHealth) error { // polled. Wait never starts a VM itself; that is Start's job. A VM that is // already not running can never bring sshd up on its own. func waitReachable(ctx context.Context, v *config.VM) error { - if !qemu.Running(v) { + state, err := StateOf(ctx, v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s: not running", ErrCannotReach, v.Name) } return pollUntil(ctx, func() bool { return sshBannerUp(ctx, v) }) @@ -276,12 +279,15 @@ func lastProvisionLineIs(v *config.VM, want string) bool { return false } -// waitStopped blocks until qemu.Running(v) turns false. Unlike Reachable and -// Applied, there is no impossible-by-construction case to refuse up front: -// a running VM can always, in principle, be stopped by something else before -// ctx gives up. +// waitStopped blocks until v's provider reports it not running. Unlike +// Reachable and Applied, there is no impossible-by-construction case to +// refuse up front: a running VM can always, in principle, be stopped by +// something else before ctx gives up. func waitStopped(ctx context.Context, v *config.VM) error { - return pollUntil(ctx, func() bool { return !qemu.Running(v) }) + return pollUntil(ctx, func() bool { + state, err := StateOf(ctx, v) + return err == nil && state != StateRunning + }) } // pollUntil calls check immediately, so an already-satisfied condition diff --git a/internal/coreerr/coreerr.go b/internal/coreerr/coreerr.go new file mode 100644 index 0000000..4befddf --- /dev/null +++ b/internal/coreerr/coreerr.go @@ -0,0 +1,15 @@ +// Package coreerr holds the sentinel errors internal/core and +// internal/capabilities both need to identify by identity (errors.Is). +// capabilities cannot import core directly: core imports provider, provider +// imports capabilities, and a capabilities->core edge would close that +// cycle. core re-exports these under its own names so every existing +// core.ErrXxx caller compiles unchanged. +package coreerr + +import "errors" + +var ( + ErrNotFound = errors.New("not found") + ErrInvalidSpec = errors.New("invalid spec") + ErrBroken = errors.New("broken vm.toml") +) diff --git a/internal/mcpsrv/tools_read.go b/internal/mcpsrv/tools_read.go index 4ee0d2b..643b9af 100644 --- a/internal/mcpsrv/tools_read.go +++ b/internal/mcpsrv/tools_read.go @@ -9,8 +9,21 @@ import ( "github.com/novusedge/stoat/internal/capabilities" "github.com/novusedge/stoat/internal/cli/wire" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/hostcheck" ) +// toHostChecks adapts core.Doctor's result to capabilities.Input.HostChecks. +// The two types have identical fields but capabilities cannot import core: +// core now imports provider, which imports capabilities, so the reverse +// import would close a cycle. +func toHostChecks(cs []core.HostCheck) []hostcheck.Check { + out := make([]hostcheck.Check, len(cs)) + for i, c := range cs { + out[i] = hostcheck.Check{Name: c.Name, OK: c.OK, Detail: c.Detail, Fix: c.Fix, Optional: c.Optional} + } + return out +} + type emptyIn struct{} type vmIn struct { @@ -159,7 +172,7 @@ func (s *srv) registerRead(server *mcp.Server) { } return wire.Capabilities(capabilities.Build(capabilities.Input{ Version: s.opts.Version, ProjectState: projectState, - HostChecks: core.Doctor(), Target: target, + HostChecks: toHostChecks(core.Doctor()), Target: target, })), nil }) From 2fde90ecbf42a38c3f0636e50c9e53a58406853c Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 22:09:20 +0300 Subject: [PATCH 09/17] test(provider): add the fake provider Core tests run with no QEMU process. Replaces fakeRunning, except clone_test.go's running-source check and autorestart_test.go's ISO-missing case: both need the real qemu provider, since one reads qemu.Running directly and the other's assertion depends on a real Start attempt actually failing rather than the fake's Start, which always succeeds. Signed-off-by: NovusEdge --- internal/core/autorestart_test.go | 2 +- internal/core/clone_test.go | 6 +-- internal/core/vm_test.go | 40 ++++++++--------- internal/provider/fake/fake.go | 69 +++++++++++++++++++++++++++++ internal/provider/fake/fake_test.go | 26 +++++++++++ 5 files changed, 119 insertions(+), 24 deletions(-) create mode 100644 internal/provider/fake/fake.go create mode 100644 internal/provider/fake/fake_test.go diff --git a/internal/core/autorestart_test.go b/internal/core/autorestart_test.go index cdeaf49..ba1843c 100644 --- a/internal/core/autorestart_test.go +++ b/internal/core/autorestart_test.go @@ -119,7 +119,7 @@ func TestAutoRestartAfterInstallAttemptsStartOnceInstallerStops(t *testing.T) { t.Fatal(err) } v.Dir = dir + "/work" - stop := fakeRunning(t, v) + stop := realQemuProcess(t, v) go func() { time.Sleep(2 * pollInterval) diff --git a/internal/core/clone_test.go b/internal/core/clone_test.go index 404492d..8ca6efc 100644 --- a/internal/core/clone_test.go +++ b/internal/core/clone_test.go @@ -52,8 +52,8 @@ func TestCloneRefusesInvalidName(t *testing.T) { // A running source must be refused outright: an overlay's backing file must // never change after the overlay is made, and a running qemu process is -// writing to its disk continuously. fakeRunning (vm_test.go) fakes liveness -// without a real qemu process. +// writing to its disk continuously. Clone checks qemu.Running directly, not +// the provider, so this needs a real process; see realQemuProcess. func TestCloneRefusesRunningSource(t *testing.T) { dir := root(t) v := &config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200} @@ -61,7 +61,7 @@ func TestCloneRefusesRunningSource(t *testing.T) { t.Fatal(err) } v.Dir = filepath.Join(dir, "work") - stop := fakeRunning(t, v) + stop := realQemuProcess(t, v) defer stop() if _, err := Clone("work", "clone1"); !errors.Is(err, ErrAlreadyRunning) { diff --git a/internal/core/vm_test.go b/internal/core/vm_test.go index f1f513b..f0712b5 100644 --- a/internal/core/vm_test.go +++ b/internal/core/vm_test.go @@ -11,6 +11,7 @@ import ( "github.com/novusedge/stoat/internal/config" "github.com/novusedge/stoat/internal/provider" + "github.com/novusedge/stoat/internal/provider/fake" "github.com/novusedge/stoat/internal/testutil" ) @@ -29,16 +30,22 @@ func writeRawVMToml(t *testing.T, name, content string) { } } -// fakeRunning marks v as running without a real qemu process. It spawns -// `sleep` with v.Dir in its argv (qemu.Running's cmdlineMatches only checks -// that /proc//cmdline contains dir+"/", not which binary it is), and -// points the VM's pidfile at it. The returned func kills the process; the -// caller must defer it. -// -// This makes Start/Stop/Destroy's "is it running" branches testable without -// qemu-system-x86_64 installed, the same CI constraint existing tests work -// around for qemu-img. +// fakeRunning marks v as running in the fake provider installed for this +// test. The returned func marks it stopped again; the caller must call or +// defer it. func fakeRunning(t *testing.T, v *config.VM) func() { + f := fake.Install(t) + f.RunningVMs[v.Name] = true + return func() { delete(f.RunningVMs, v.Name) } +} + +// realQemuProcess spawns a real process and points v's pidfile at it, +// leaving the real "qemu" provider registered. Use this instead of +// fakeRunning for a test that exercises code reading qemu.Running directly +// (clone.go) or a provider.Start call that must actually run and fail +// (autorestart_test.go's ISO-missing case): fake.Install swaps in a Start +// that always succeeds, which would pass those tests for the wrong reason. +func realQemuProcess(t *testing.T, v *config.VM) func() { return testutil.FakeRunning(t, v.Dir) } @@ -237,8 +244,7 @@ func TestStartAlreadyRunning(t *testing.T) { t.Fatal(err) } v.Dir = filepath.Join(dir, "work") - stop := fakeRunning(t, v) - defer stop() + defer fakeRunning(t, v)() if err := Start("work"); !errors.Is(err, ErrAlreadyRunning) { t.Fatalf("err = %v, want ErrAlreadyRunning", err) @@ -277,15 +283,12 @@ func TestDestroyRefusesWhileRunning(t *testing.T) { stop := fakeRunning(t, v) if err := Destroy("work"); !errors.Is(err, ErrAlreadyRunning) { - stop() t.Fatalf("err = %v, want ErrAlreadyRunning", err) } if _, err := os.Stat(v.Dir); err != nil { - stop() t.Fatalf("VM directory should still exist after a refused destroy: %v", err) } stop() - _ = os.Remove(v.PidPath()) if err := Destroy("work"); err != nil { t.Fatalf("Destroy after stopping: %v", err) @@ -475,7 +478,7 @@ func TestGetDoesNotModifyVMTomlOnDisk(t *testing.T) { // from the pidfile qemu.Running just read, and a stopped one gets the zero // time, not some stale value left over from a previous run. func TestStartedAtRunningVsStopped(t *testing.T) { - dir := root(t) + root(t) if err := (&config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200}).Save(); err != nil { t.Fatal(err) } @@ -488,9 +491,7 @@ func TestStartedAtRunningVsStopped(t *testing.T) { t.Errorf("stopped VM: StartedAt = %v, want the zero time", v.StartedAt) } - cv := &config.VM{Name: "work", Dir: filepath.Join(dir, "work")} - stop := fakeRunning(t, cv) - defer stop() + defer fakeRunning(t, &config.VM{Name: "work"})() v, err = Get("work") if err != nil { @@ -561,8 +562,7 @@ func TestDestroyRefusesARunningBrokenVM(t *testing.T) { root(t) // A directory that is running but whose vm.toml no longer parses. writeRawVMToml(t, "hosed", "name = \"hosed\"\nmode = \"disk\n") - stop := fakeRunning(t, &config.VM{Name: "hosed", Dir: filepath.Join(config.Root(), "hosed")}) - defer stop() + defer fakeRunning(t, &config.VM{Name: "hosed"})() if err := Destroy("hosed"); !errors.Is(err, ErrAlreadyRunning) { t.Fatalf("Destroy on a running broken VM = %v, want ErrAlreadyRunning", err) diff --git a/internal/provider/fake/fake.go b/internal/provider/fake/fake.go new file mode 100644 index 0000000..a937dab --- /dev/null +++ b/internal/provider/fake/fake.go @@ -0,0 +1,69 @@ +// Package fake is a provider.Provider for tests. It lets a core test run with +// no QEMU process and no cloud API. +package fake + +import ( + "context" + "testing" + "time" + + "github.com/novusedge/stoat/internal/capabilities" + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" + _ "github.com/novusedge/stoat/internal/provider/qemu" + "github.com/novusedge/stoat/internal/sshx" +) + +type Provider struct { + RunningVMs map[string]bool + StartErr error + StopErr error + Ep sshx.Endpoint +} + +func (Provider) Name() string { return "qemu" } +func (Provider) Capabilities(*config.VM) []capabilities.Capability { return nil } + +func (p *Provider) Start(_ context.Context, v *config.VM) error { + if p.StartErr != nil { + return p.StartErr + } + p.RunningVMs[v.Name] = true + return nil +} + +func (p *Provider) Stop(_ context.Context, v *config.VM) error { + if p.StopErr != nil { + return p.StopErr + } + delete(p.RunningVMs, v.Name) + return nil +} + +func (p *Provider) Status(_ context.Context, v *config.VM) (provider.Status, error) { + if !p.RunningVMs[v.Name] { + return provider.Status{}, nil + } + return provider.Status{Running: true, StartedAt: time.Unix(1, 0)}, nil +} + +func (p *Provider) Endpoint(_ context.Context, v *config.VM) (sshx.Endpoint, error) { + if p.Ep.Host != "" { + return p.Ep, nil + } + return sshx.LocalEndpoint(v), nil +} + +// Install registers a fresh fake under "qemu" and restores the previous +// provider when the test ends. +func Install(t *testing.T) *Provider { + t.Helper() + prev, err := provider.For(&config.VM{}) + if err != nil { + t.Fatalf("no provider registered as qemu: %v", err) + } + f := &Provider{RunningVMs: map[string]bool{}} + provider.Register("qemu", f) + t.Cleanup(func() { provider.Register("qemu", prev) }) + return f +} diff --git a/internal/provider/fake/fake_test.go b/internal/provider/fake/fake_test.go new file mode 100644 index 0000000..32eca5f --- /dev/null +++ b/internal/provider/fake/fake_test.go @@ -0,0 +1,26 @@ +package fake + +import ( + "context" + "testing" + + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" +) + +func TestInstallReplacesQemuForOneTest(t *testing.T) { + f := Install(t) + f.RunningVMs["dev"] = true + + p, err := provider.For(&config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("For() error = %v", err) + } + s, err := p.Status(context.Background(), &config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("Status() error = %v", err) + } + if !s.Running { + t.Error("Running = false, want true: the fake was told this VM runs") + } +} From 2c363a0cb9a37ef6bc822615990556f43a3efb57 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 22:17:03 +0300 Subject: [PATCH 10/17] test(core): exercise real StartedAt in TestStartedAtRunningVsStopped Signed-off-by: NovusEdge --- internal/core/vm_test.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/internal/core/vm_test.go b/internal/core/vm_test.go index f0712b5..495371f 100644 --- a/internal/core/vm_test.go +++ b/internal/core/vm_test.go @@ -476,28 +476,33 @@ func TestGetDoesNotModifyVMTomlOnDisk(t *testing.T) { // TestStartedAtRunningVsStopped pins that a running VM's StartedAt comes // from the pidfile qemu.Running just read, and a stopped one gets the zero -// time, not some stale value left over from a previous run. +// time, not some stale value left over from a previous run. It uses +// realQemuProcess, not fakeRunning, so this still exercises qemu.StartedAt +// reading the pidfile's mtime rather than a hardcoded fake value. func TestStartedAtRunningVsStopped(t *testing.T) { - root(t) - if err := (&config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200}).Save(); err != nil { + dir := root(t) + v := &config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200} + if err := v.Save(); err != nil { t.Fatal(err) } + v.Dir = filepath.Join(dir, "work") - v, err := Get("work") + got, err := Get("work") if err != nil { t.Fatal(err) } - if !v.StartedAt.IsZero() { - t.Errorf("stopped VM: StartedAt = %v, want the zero time", v.StartedAt) + if !got.StartedAt.IsZero() { + t.Errorf("stopped VM: StartedAt = %v, want the zero time", got.StartedAt) } - defer fakeRunning(t, &config.VM{Name: "work"})() + stop := realQemuProcess(t, v) + defer stop() - v, err = Get("work") + got, err = Get("work") if err != nil { t.Fatal(err) } - if v.StartedAt.IsZero() { + if got.StartedAt.IsZero() { t.Error("running VM: StartedAt is the zero time, want the pidfile's mtime") } } From bcbee2d3a8e85efa60bd6ca8a5bd89f549aba9d0 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 22:17:05 +0300 Subject: [PATCH 11/17] test(core): fix stale comment on TestPruneNeverRemovesARunningBrokenVM Signed-off-by: NovusEdge --- internal/core/prune_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/internal/core/prune_test.go b/internal/core/prune_test.go index 2e42552..09b4d8c 100644 --- a/internal/core/prune_test.go +++ b/internal/core/prune_test.go @@ -340,10 +340,9 @@ func TestPruneRemovesABrokenVMWithAStalePidfile(t *testing.T) { // disk. A vm.toml can be corrupted after its VM started; that is the state // this test reaches. // -// It uses fakeRunning, which spawns a real process with the VM's directory -// in its argv: that is what qemu.Running matches on. An earlier version -// used an impossible pid instead, so the guard never ran; the mutation -// `if qemu.Running(bv)` -> `if false && ...` still passed every test. +// It uses fakeRunning, which marks the VM running in the fake provider +// Prune's StateOf call resolves to. An earlier version used an impossible +// pid instead, so the guard never ran. func TestPruneNeverRemovesARunningBrokenVM(t *testing.T) { dir := root(t) writeBroken(t, dir, "busted") From e2d4c63cdea940269e7f85fb41b114a4913ca6e7 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 22:17:07 +0300 Subject: [PATCH 12/17] fix(core): mark a VM broken when its provider can't be reached Signed-off-by: NovusEdge --- internal/core/vm.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/core/vm.go b/internal/core/vm.go index fbe133b..e3733e8 100644 --- a/internal/core/vm.go +++ b/internal/core/vm.go @@ -268,11 +268,14 @@ func checkGuest(v *config.VM) error { func fromConfigUnchecked(v *config.VM) VM { state := StateStopped var startedAt time.Time - if p, err := providerFor(v); err == nil { - if s, err := p.Status(context.Background(), v); err == nil && s.Running { - state = StateRunning - startedAt = s.StartedAt - } + var stateErr string + p, err := providerFor(v) + if err != nil { + state, stateErr = StateBroken, err.Error() + } else if s, err := p.Status(context.Background(), v); err != nil { + state, stateErr = StateBroken, err.Error() + } else if s.Running { + state, startedAt = StateRunning, s.StartedAt } osName, backend := inferMissing(v) return VM{ @@ -284,6 +287,7 @@ func fromConfigUnchecked(v *config.VM) VM { Mode: v.Mode, Backend: backend, State: state, + Error: stateErr, StartedAt: startedAt, RAM: v.RAM, CPUs: v.CPUs, From 721c935a8c92f64e3dbf508ad4ab498f0f3032a0 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 22:25:37 +0300 Subject: [PATCH 13/17] fix(core): propagate StateOf's error in Destroy Destroy discarded StateOf's error the same way fromConfigUnchecked did before it was fixed, letting a vm.toml naming an unimplemented provider fall through to Delete with no running check performed. Also update VM.Error's doc: fromConfigUnchecked now writes providerFor's and Status's error strings into it too, not only config.Load's parse error. Signed-off-by: NovusEdge --- internal/core/vm.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/internal/core/vm.go b/internal/core/vm.go index e3733e8..9fed9cb 100644 --- a/internal/core/vm.go +++ b/internal/core/vm.go @@ -198,8 +198,9 @@ type VM struct { Paths Paths // Error is populated only when State is StateBroken, and holds - // config.Load's parse error so a caller can show the user why, not just - // that it's broken. + // config.Load's parse error, or providerFor's or Status's error when the + // config parsed but its provider could not be resolved, so a caller can + // show the user why, not just that it's broken. Error string // Project is the absolute directory of the stoat.toml that declared this @@ -658,7 +659,11 @@ func Destroy(name string) error { if err != nil { return err } - if state, err := StateOf(context.Background(), v); err == nil && state == StateRunning { + state, err := StateOf(context.Background(), v) + if err != nil { + return err + } + if state == StateRunning { return fmt.Errorf("%w: %s: stop it first", ErrAlreadyRunning, name) } return v.Delete() From bd5940c20486ddd322780f2c4fdadcfed834a622 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 22:36:01 +0300 Subject: [PATCH 14/17] test(provider): add the fake provider core.waitStopped polls Status from its own goroutine while a test flips a VM, so the fake's liveness map takes a mutex and callers go through SetRunning and SetStopped. Replaces fakeRunning. AutoRestartAfterInstall returns StateOf's error instead of reporting nothing to restart for a VM whose provider will not resolve. Signed-off-by: NovusEdge --- internal/core/autorestart.go | 5 ++- internal/core/vm.go | 5 +-- internal/core/vm_test.go | 4 +-- internal/provider/fake/fake.go | 48 +++++++++++++++++++++++------ internal/provider/fake/fake_test.go | 2 +- 5 files changed, 48 insertions(+), 16 deletions(-) diff --git a/internal/core/autorestart.go b/internal/core/autorestart.go index 379bb71..82fad69 100644 --- a/internal/core/autorestart.go +++ b/internal/core/autorestart.go @@ -28,7 +28,10 @@ func AutoRestartAfterInstall(ctx context.Context, name string) (bool, error) { return false, nil } state, err := StateOf(ctx, v) - if err != nil || state != StateRunning { + if err != nil { + return false, err + } + if state != StateRunning { return false, nil } diff --git a/internal/core/vm.go b/internal/core/vm.go index 9fed9cb..c1e93eb 100644 --- a/internal/core/vm.go +++ b/internal/core/vm.go @@ -24,8 +24,9 @@ import ( // The state model has six states. Only three are knowable today. StateStopped // and StateRunning come from the VM's provider (StateOf); qemu's own check // matches pid liveness against /proc//cmdline, so a reused pid never -// reads as running. StateBroken comes from a vm.toml that exists but fails to parse -// (config.ListBroken's concept). +// reads as running. StateBroken has two sources: a vm.toml that exists but +// fails to parse (config.ListBroken's concept), and a vm.toml that parsed +// whose provider cannot be resolved or cannot answer (fromConfigUnchecked). // // StateStarting, StateApplying and StateFailed are not declared here. No // code path yet distinguishes "qemu process is up" from "guest is diff --git a/internal/core/vm_test.go b/internal/core/vm_test.go index 495371f..6583e03 100644 --- a/internal/core/vm_test.go +++ b/internal/core/vm_test.go @@ -35,8 +35,8 @@ func writeRawVMToml(t *testing.T, name, content string) { // defer it. func fakeRunning(t *testing.T, v *config.VM) func() { f := fake.Install(t) - f.RunningVMs[v.Name] = true - return func() { delete(f.RunningVMs, v.Name) } + f.SetRunning(v.Name) + return func() { f.SetStopped(v.Name) } } // realQemuProcess spawns a real process and points v's pidfile at it, diff --git a/internal/provider/fake/fake.go b/internal/provider/fake/fake.go index a937dab..457f38b 100644 --- a/internal/provider/fake/fake.go +++ b/internal/provider/fake/fake.go @@ -4,6 +4,7 @@ package fake import ( "context" + "sync" "testing" "time" @@ -14,21 +15,48 @@ import ( "github.com/novusedge/stoat/internal/sshx" ) +// Provider answers from a liveness map the test controls. core.waitStopped +// polls Status from its own goroutine while the test flips a VM, so every +// read and write of that map takes mu. type Provider struct { - RunningVMs map[string]bool - StartErr error - StopErr error - Ep sshx.Endpoint + StartErr error + StopErr error + Ep sshx.Endpoint + + mu sync.Mutex + running map[string]bool +} + +func (*Provider) Name() string { return "qemu" } +func (*Provider) Capabilities(*config.VM) []capabilities.Capability { return nil } + +// SetRunning marks name live. Tests that need a VM to look started without +// calling Start use this. +func (p *Provider) SetRunning(name string) { + p.mu.Lock() + defer p.mu.Unlock() + p.running[name] = true } -func (Provider) Name() string { return "qemu" } -func (Provider) Capabilities(*config.VM) []capabilities.Capability { return nil } +// SetStopped marks name dead. +func (p *Provider) SetStopped(name string) { + p.mu.Lock() + defer p.mu.Unlock() + delete(p.running, name) +} + +// IsRunning reports the recorded liveness of name. +func (p *Provider) IsRunning(name string) bool { + p.mu.Lock() + defer p.mu.Unlock() + return p.running[name] +} func (p *Provider) Start(_ context.Context, v *config.VM) error { if p.StartErr != nil { return p.StartErr } - p.RunningVMs[v.Name] = true + p.SetRunning(v.Name) return nil } @@ -36,12 +64,12 @@ func (p *Provider) Stop(_ context.Context, v *config.VM) error { if p.StopErr != nil { return p.StopErr } - delete(p.RunningVMs, v.Name) + p.SetStopped(v.Name) return nil } func (p *Provider) Status(_ context.Context, v *config.VM) (provider.Status, error) { - if !p.RunningVMs[v.Name] { + if !p.IsRunning(v.Name) { return provider.Status{}, nil } return provider.Status{Running: true, StartedAt: time.Unix(1, 0)}, nil @@ -62,7 +90,7 @@ func Install(t *testing.T) *Provider { if err != nil { t.Fatalf("no provider registered as qemu: %v", err) } - f := &Provider{RunningVMs: map[string]bool{}} + f := &Provider{running: map[string]bool{}} provider.Register("qemu", f) t.Cleanup(func() { provider.Register("qemu", prev) }) return f diff --git a/internal/provider/fake/fake_test.go b/internal/provider/fake/fake_test.go index 32eca5f..88f7923 100644 --- a/internal/provider/fake/fake_test.go +++ b/internal/provider/fake/fake_test.go @@ -10,7 +10,7 @@ import ( func TestInstallReplacesQemuForOneTest(t *testing.T) { f := Install(t) - f.RunningVMs["dev"] = true + f.SetRunning("dev") p, err := provider.For(&config.VM{Name: "dev"}) if err != nil { From 4cf033f519e2b92eb0eea9befe3fc36ee5eb2056 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 22:55:59 +0300 Subject: [PATCH 15/17] fix(core): route Clone's running check through StateOf Clone called qemu.Running(src) directly, bypassing the provider seam every other lifecycle check goes through; a provider="gce" VM would be cloned after consulting a local pidfile that can never be live. Restores Dir and Name as load-bearing in three tests that had stopped exercising the reconstructed-config path they claim to guard, rewrites two tests that asserted only zero-value or mock behaviour, drops a tautological test that cannot fail on either build tag, and corrects comments pointing at code paths that no longer exist. Signed-off-by: NovusEdge --- docs/getting-started/installation.md | 7 +++--- docs/reference/samples/vm.toml | 1 + docs/troubleshooting.md | 11 +++++----- internal/cli/cli.go | 7 +++--- internal/core/autorestart_test.go | 6 ++--- internal/core/clone.go | 8 +++++-- internal/core/clone_test.go | 9 +++----- internal/core/prune_test.go | 8 +++---- internal/core/vm.go | 4 ++-- internal/core/vm_test.go | 31 +++++++++++++++++--------- internal/hostops/message_test.go | 4 ++-- internal/hostops/support.go | 7 +++--- internal/hostops/support_test.go | 6 ----- internal/provider/fake/fake_test.go | 33 +++++++++++++++++++++------- internal/provider/qemu/qemu.go | 4 ++-- internal/provider/qemu/qemu_test.go | 21 +++++++++++++----- internal/tui/vmlist.go | 8 +++---- 17 files changed, 104 insertions(+), 71 deletions(-) diff --git a/docs/getting-started/installation.md b/docs/getting-started/installation.md index d61ccc1..1b160d5 100644 --- a/docs/getting-started/installation.md +++ b/docs/getting-started/installation.md @@ -187,9 +187,10 @@ install -Dm755 stoat ~/.local/bin/stoat ``` A release also carries `darwin_amd64` and `darwin_arm64` tarballs, and -`windows_amd64` and `windows_arm64` zip archives. On those hosts Stoat runs -`doctor` and `capabilities` only. Every other command reports the host as -unqualified, because macOS needs the QEMU HVF accelerator +`windows_amd64` and `windows_arm64` zip archives. On those hosts Stoat cannot +start or stop a local VM; it reports the host as unqualified. Commands that +only read or edit VM records still run. This is because macOS needs the +QEMU HVF accelerator ([#82](https://github.com/NovusEdge/stoat/issues/82)) and Windows needs WHPX ([#83](https://github.com/NovusEdge/stoat/issues/83)). Linux with KVM is the supported host today. diff --git a/docs/reference/samples/vm.toml b/docs/reference/samples/vm.toml index 39bd56a..7c91fed 100644 --- a/docs/reference/samples/vm.toml +++ b/docs/reference/samples/vm.toml @@ -16,6 +16,7 @@ sshport = 2200 # int; default an allocated free port; stoat recipes = ["docker"] # string[]; default []; user/TUI and stoat create/update write the selection. display = "auto" # string; default "auto"; user/TUI writes: auto, window, or vnc. backend = "apkovl" # string; default inferred from image; stoat writes: apkovl, cloudinit, or ssh. +provider = "qemu" # string; default qemu when empty; stoat writes the execution surface. base = "" # string path; default empty; stoat writes the absolute shared base-image path. sshuser = "root" # string; default guest-defined user (empty means root); stoat writes it. console_password = "" # string; default "stoat" for cloud VMs, empty otherwise; stoat writes, never ssh. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 10e6157..b8ab295 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -7,20 +7,19 @@ Find the reported symptom or error text below, then follow its recovery steps. ``` stoat: up: native VM operations are not qualified on darwin/arm64. darwin/arm64 needs a qualified runtime with the QEMU HVF accelerator (tracked in stoat#82). -doctor and capabilities still work here; every other command needs a qualified host. +starting and stopping a local VM needs a qualified host; commands that only read or edit VM records still work here. Linux with KVM is the supported configuration today. ``` -Stoat's VM lifecycle (create, start, stop, ssh, recipes, and every other -command except `doctor` and `capabilities`) is qualified on Linux with KVM -only. macOS arm64 and Windows amd64 builds exist and compile, but their -native runtimes are not qualified yet; see +Starting and stopping a local VM is qualified on Linux with KVM only. macOS +arm64 and Windows amd64 builds exist and compile, but their native runtimes +are not qualified yet; see [stoat#82](https://github.com/novusedge/stoat/issues/82) and [stoat#83](https://github.com/novusedge/stoat/issues/83). `--json` and MCP report this as the `host_unsupported` error code. **Fix:** run Stoat on Linux with KVM, or track the issue above for your -platform. `stoat doctor` and `stoat capabilities` still run on any host. +platform. Commands that only read or edit VM records still run on any host. ## No QEMU window appears diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 890d1b4..cbf7931 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -495,10 +495,9 @@ func Main(args []string, version string, stdin io.Reader, stdout, stderr io.Writ return runDoctor(a, stdout, stderr) } - // Every mutating or process-facing command must reject before resolving - // secrets, reading project scope, creating the data root, or initializing - // logs. The independent capabilities command is dispatched before this - // boundary by its owner and remains metadata-only. + // RequireDataRoot never refuses today. It stays on the path a mutating + // command takes before it resolves secrets, reads project scope or writes + // logs, so a future host gate has one place to land. if err := hostops.RequireDataRoot(); err != nil { return a.fail(stdout, stderr, err) } diff --git a/internal/core/autorestart_test.go b/internal/core/autorestart_test.go index ba1843c..c7b2241 100644 --- a/internal/core/autorestart_test.go +++ b/internal/core/autorestart_test.go @@ -107,11 +107,11 @@ func TestAutoRestartAfterInstallGivesUpSilentlyOnTimeout(t *testing.T) { // TestAutoRestartAfterInstallAttemptsStartOnceInstallerStops proves the // installer-stop signal actually drives a restart attempt, not just the -// guard. qemu.Start has no fake seam, so the restart attempt fails here for +// guard. The fake provider's Start always succeeds, so this test keeps the +// real qemu provider (realQemuProcess) and lets the restart attempt fail for // lack of a real install ISO (v.ISOPath() names nothing on disk); the test // asserts on that failure's shape rather than a real boot, distinguishing a -// restart ATTEMPT (the failure names v's ISO path) from the guard's silent -// no-op (nil error, near-instant return). +// restart ATTEMPT from the guard's silent no-op. func TestAutoRestartAfterInstallAttemptsStartOnceInstallerStops(t *testing.T) { dir := root(t) v := &config.VM{Name: "work", Mode: "disk", Installed: false, OS: "alpine", Backend: "apkovl", RAM: 1024, CPUs: 1, SSHPort: 2404} diff --git a/internal/core/clone.go b/internal/core/clone.go index 5c99e3f..168f722 100644 --- a/internal/core/clone.go +++ b/internal/core/clone.go @@ -1,6 +1,7 @@ package core import ( + "context" "fmt" "os" "os/exec" @@ -11,7 +12,6 @@ import ( "github.com/novusedge/stoat/internal/config" "github.com/novusedge/stoat/internal/guest" "github.com/novusedge/stoat/internal/keys" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/recipes" ) @@ -45,7 +45,11 @@ func Clone(name, newName string) (VM, error) { if err != nil { return VM{}, err } - if qemu.Running(src) { + state, err := StateOf(context.Background(), src) + if err != nil { + return VM{}, err + } + if state == StateRunning { return VM{}, fmt.Errorf("%w: %s: stop it before cloning", ErrAlreadyRunning, name) } diff --git a/internal/core/clone_test.go b/internal/core/clone_test.go index 8ca6efc..5422d3d 100644 --- a/internal/core/clone_test.go +++ b/internal/core/clone_test.go @@ -52,17 +52,14 @@ func TestCloneRefusesInvalidName(t *testing.T) { // A running source must be refused outright: an overlay's backing file must // never change after the overlay is made, and a running qemu process is -// writing to its disk continuously. Clone checks qemu.Running directly, not -// the provider, so this needs a real process; see realQemuProcess. +// writing to its disk continuously. func TestCloneRefusesRunningSource(t *testing.T) { - dir := root(t) + root(t) v := &config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200} if err := v.Save(); err != nil { t.Fatal(err) } - v.Dir = filepath.Join(dir, "work") - stop := realQemuProcess(t, v) - defer stop() + defer fakeRunning(t, v)() if _, err := Clone("work", "clone1"); !errors.Is(err, ErrAlreadyRunning) { t.Fatalf("err = %v, want ErrAlreadyRunning", err) diff --git a/internal/core/prune_test.go b/internal/core/prune_test.go index 09b4d8c..fcfe25c 100644 --- a/internal/core/prune_test.go +++ b/internal/core/prune_test.go @@ -340,15 +340,15 @@ func TestPruneRemovesABrokenVMWithAStalePidfile(t *testing.T) { // disk. A vm.toml can be corrupted after its VM started; that is the state // this test reaches. // -// It uses fakeRunning, which marks the VM running in the fake provider -// Prune's StateOf call resolves to. An earlier version used an impossible -// pid instead, so the guard never ran. +// It spawns a real process, so the guard exercises qemu.Running on the +// reconstructed Dir. An earlier version used an impossible pid instead, so +// the guard never ran. func TestPruneNeverRemovesARunningBrokenVM(t *testing.T) { dir := root(t) writeBroken(t, dir, "busted") vdir := filepath.Join(dir, "busted") - stop := fakeRunning(t, &config.VM{Name: "busted", Dir: vdir}) + stop := realQemuProcess(t, &config.VM{Name: "busted", Dir: vdir}) defer stop() removed, err := Prune(PruneOpts{Broken: true}) diff --git a/internal/core/vm.go b/internal/core/vm.go index c1e93eb..7254e35 100644 --- a/internal/core/vm.go +++ b/internal/core/vm.go @@ -528,8 +528,8 @@ func Get(name string) (VM, error) { } // providerFor resolves v's execution surface. Every core call site goes -// through here instead of provider.For directly, so C2's capability checks -// have one place to land. +// through here rather than provider.For, so a per-provider check added +// later has one place to land. func providerFor(v *config.VM) (provider.Provider, error) { return provider.For(v) } // StateOf asks v's provider what the machine is doing. A provider this diff --git a/internal/core/vm_test.go b/internal/core/vm_test.go index 6583e03..d4612ca 100644 --- a/internal/core/vm_test.go +++ b/internal/core/vm_test.go @@ -42,9 +42,12 @@ func fakeRunning(t *testing.T, v *config.VM) func() { // realQemuProcess spawns a real process and points v's pidfile at it, // leaving the real "qemu" provider registered. Use this instead of // fakeRunning for a test that exercises code reading qemu.Running directly -// (clone.go) or a provider.Start call that must actually run and fail -// (autorestart_test.go's ISO-missing case): fake.Install swaps in a Start -// that always succeeds, which would pass those tests for the wrong reason. +// (clone.go), a provider.Start call that must actually run and fail +// (autorestart_test.go's ISO-missing case: fake.Install swaps in a Start +// that always succeeds, which would pass that test for the wrong reason), +// or a broken-VM path whose running check is fed a hand-reconstructed +// config.VM (TestDestroyRefusesARunningBrokenVM below), where fakeRunning's +// name-only match can't tell whether Dir was reconstructed. func realQemuProcess(t *testing.T, v *config.VM) func() { return testutil.FakeRunning(t, v.Dir) } @@ -567,7 +570,7 @@ func TestDestroyRefusesARunningBrokenVM(t *testing.T) { root(t) // A directory that is running but whose vm.toml no longer parses. writeRawVMToml(t, "hosed", "name = \"hosed\"\nmode = \"disk\n") - defer fakeRunning(t, &config.VM{Name: "hosed"})() + defer realQemuProcess(t, &config.VM{Name: "hosed", Dir: filepath.Join(config.Root(), "hosed")})() if err := Destroy("hosed"); !errors.Is(err, ErrAlreadyRunning) { t.Fatalf("Destroy on a running broken VM = %v, want ErrAlreadyRunning", err) @@ -577,14 +580,22 @@ func TestDestroyRefusesARunningBrokenVM(t *testing.T) { } } -func TestStateUsesTheProvider(t *testing.T) { +// The fake is keyed by Name and the real provider by Dir, so this fails if +// StateOf stops consulting the registry. +func TestStateOfFollowsTheProvider(t *testing.T) { + f := fake.Install(t) v := &config.VM{Name: "dev", Dir: t.TempDir()} - got, err := StateOf(context.Background(), v) - if err != nil { - t.Fatalf("StateOf() error = %v", err) + + if got, err := StateOf(context.Background(), v); err != nil || got != StateStopped { + t.Fatalf("StateOf() = %q, %v, want %q, nil", got, err, StateStopped) + } + f.SetRunning("dev") + if got, err := StateOf(context.Background(), v); err != nil || got != StateRunning { + t.Fatalf("StateOf() = %q, %v, want %q, nil", got, err, StateRunning) } - if got != StateStopped { - t.Errorf("StateOf() = %q, want %q for a VM with no running process", got, StateStopped) + f.SetStopped("dev") + if got, err := StateOf(context.Background(), v); err != nil || got != StateStopped { + t.Fatalf("StateOf() = %q, %v, want %q, nil", got, err, StateStopped) } } diff --git a/internal/hostops/message_test.go b/internal/hostops/message_test.go index a3aafc8..bd9835f 100644 --- a/internal/hostops/message_test.go +++ b/internal/hostops/message_test.go @@ -23,7 +23,7 @@ func TestMessage(t *testing.T) { "darwin/arm64", "QEMU HVF accelerator", "stoat#82", - "doctor and capabilities still work", + "starting and stopping a local VM needs a qualified host", "Linux with KVM is the supported configuration today", }, }, @@ -35,7 +35,7 @@ func TestMessage(t *testing.T) { "windows/amd64", "QEMU WHPX accelerator", "stoat#83", - "doctor and capabilities still work", + "starting and stopping a local VM needs a qualified host", "Linux with KVM is the supported configuration today", }, }, diff --git a/internal/hostops/support.go b/internal/hostops/support.go index 24ca990..4fa3d65 100644 --- a/internal/hostops/support.go +++ b/internal/hostops/support.go @@ -7,8 +7,9 @@ import ( ) // ErrUnsupported reports a native host whose VM operations have not been -// qualified yet. Only doctor and capabilities run there. The gate in -// internal/cli sits ahead of every other command, so ls and get refuse too. +// qualified yet. Only RequireLocalHypervisor returns it, so a command that +// starts or stops a local VM refuses while one that reads or edits a VM +// record does not. var ErrUnsupported = errors.New("native VM operations are not qualified") // requirement names the accelerator a host needs before stoat qualifies it, @@ -44,7 +45,7 @@ func Message(goos, goarch string) string { lines = append(lines, fmt.Sprintf("%s has no qualified runtime yet.", host)) } lines = append(lines, - "doctor and capabilities still work here; every other command needs a qualified host.", + "starting and stopping a local VM needs a qualified host; commands that only read or edit VM records still work here.", "Linux with KVM is the supported configuration today.", ) out := lines[0] diff --git a/internal/hostops/support_test.go b/internal/hostops/support_test.go index 2579bef..47621a3 100644 --- a/internal/hostops/support_test.go +++ b/internal/hostops/support_test.go @@ -7,12 +7,6 @@ import ( "testing" ) -func TestRequireDataRootAlwaysAllows(t *testing.T) { - if err := RequireDataRoot(); err != nil { - t.Errorf("RequireDataRoot() = %v, want nil on every platform", err) - } -} - func TestRequireLocalHypervisorFollowsPlatform(t *testing.T) { err := RequireLocalHypervisor() if runtime.GOOS == "linux" { diff --git a/internal/provider/fake/fake_test.go b/internal/provider/fake/fake_test.go index 88f7923..aa6083f 100644 --- a/internal/provider/fake/fake_test.go +++ b/internal/provider/fake/fake_test.go @@ -9,18 +9,35 @@ import ( ) func TestInstallReplacesQemuForOneTest(t *testing.T) { - f := Install(t) - f.SetRunning("dev") - - p, err := provider.For(&config.VM{Name: "dev"}) + before, err := provider.For(&config.VM{}) if err != nil { t.Fatalf("For() error = %v", err) } - s, err := p.Status(context.Background(), &config.VM{Name: "dev"}) + + t.Run("installed", func(t *testing.T) { + f := Install(t) + f.SetRunning("dev") + p, err := provider.For(&config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("For() error = %v", err) + } + if p != provider.Provider(f) { + t.Fatalf("For() = %T, want the installed fake", p) + } + s, err := p.Status(context.Background(), &config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("Status() error = %v", err) + } + if !s.Running { + t.Error("Running = false, want true: the fake was told this VM runs") + } + }) + + after, err := provider.For(&config.VM{}) if err != nil { - t.Fatalf("Status() error = %v", err) + t.Fatalf("For() error = %v", err) } - if !s.Running { - t.Error("Running = false, want true: the fake was told this VM runs") + if after != before { + t.Errorf("provider after the subtest = %T, want the qemu provider restored", after) } } diff --git a/internal/provider/qemu/qemu.go b/internal/provider/qemu/qemu.go index 0aff6d5..cb1e278 100644 --- a/internal/provider/qemu/qemu.go +++ b/internal/provider/qemu/qemu.go @@ -18,8 +18,8 @@ type Provider struct{} func (Provider) Name() string { return "qemu" } -// Capabilities returns nothing in C1. The capability set moves here in C2, -// alongside the code that consumes it. +// Capabilities returns nothing. The capability set still lives in +// internal/capabilities and moves here with the code that reads it. func (Provider) Capabilities(*config.VM) []capabilities.Capability { return nil } func (Provider) Start(_ context.Context, v *config.VM) error { return qemu.Start(v) } diff --git a/internal/provider/qemu/qemu_test.go b/internal/provider/qemu/qemu_test.go index 8424b74..327df99 100644 --- a/internal/provider/qemu/qemu_test.go +++ b/internal/provider/qemu/qemu_test.go @@ -6,6 +6,7 @@ import ( "github.com/novusedge/stoat/internal/config" "github.com/novusedge/stoat/internal/provider" + "github.com/novusedge/stoat/internal/testutil" ) func TestRegisteredAsQemu(t *testing.T) { @@ -32,15 +33,23 @@ func TestEndpointIsLoopback(t *testing.T) { } } -func TestStatusReportsStoppedForAnUnstartedVM(t *testing.T) { - s, err := Provider{}.Status(context.Background(), &config.VM{Name: "dev", Dir: t.TempDir()}) +func TestStatusReflectsTheQemuProcess(t *testing.T) { + v := &config.VM{Name: "dev", Dir: t.TempDir()} + + s, err := Provider{}.Status(context.Background(), v) + if err != nil || s.Running || !s.StartedAt.IsZero() { + t.Fatalf("Status() = %+v, %v, want a stopped zero status", s, err) + } + + defer testutil.FakeRunning(t, v.Dir)() + s, err = Provider{}.Status(context.Background(), v) if err != nil { t.Fatalf("Status() error = %v", err) } - if s.Running { - t.Error("Running = true, want false for a VM with no pidfile") + if !s.Running { + t.Error("Running = false, want true while the pidfile names a live process") } - if s.Raw != "" { - t.Errorf("Raw = %q, want empty: qemu has no status word of its own", s.Raw) + if s.StartedAt.IsZero() { + t.Error("StartedAt is zero, want the pidfile's mtime") } } diff --git a/internal/tui/vmlist.go b/internal/tui/vmlist.go index 6926995..bd18fa5 100644 --- a/internal/tui/vmlist.go +++ b/internal/tui/vmlist.go @@ -107,10 +107,10 @@ func (d vmDelegate) Render(w io.Writer, m list.Model, index int, item list.Item) // next reload. if v.State == core.StateRunning { dot, dotStyle = glyphRunning, upStyle - // State and StartedAt come from separate qemu.Running checks in - // fromConfig; a pidfile vanishing between them leaves a running row - // with a zero StartedAt, which time.Since renders as a nonsense - // six-figure uptime. Drop the duration in that window. + // fromConfigUnchecked takes State and StartedAt from one provider.Status + // call, but the qemu provider reads pid liveness and pidfile mtime + // separately: a pidfile vanishing between them leaves a running row with a + // zero StartedAt, which time.Since renders as a nonsense six-figure uptime. up := "up ?" if !v.StartedAt.IsZero() { up = "up " + time.Since(v.StartedAt).Truncate(time.Second).String() From 04547993f17a6b3994bc420b763f5ed7a0698623 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 23:02:38 +0300 Subject: [PATCH 16/17] fix(core): probe the provider's endpoint when waiting for ssh sshBannerUp dialled 127.0.0.1 while waitReachable resolved state through the provider one line above, so a VM answering anywhere else would never be seen as reachable. Adds the two tests that pin the endpoint seam and the provider's Start/Stop errors; fake.Ep, StartErr and StopErr had no caller until now. Signed-off-by: NovusEdge --- internal/core/vm_test.go | 52 ++++++++++++++++++++++++++++++++++++++++ internal/core/wait.go | 15 ++++++++++-- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/internal/core/vm_test.go b/internal/core/vm_test.go index d4612ca..4899d82 100644 --- a/internal/core/vm_test.go +++ b/internal/core/vm_test.go @@ -12,6 +12,7 @@ import ( "github.com/novusedge/stoat/internal/config" "github.com/novusedge/stoat/internal/provider" "github.com/novusedge/stoat/internal/provider/fake" + "github.com/novusedge/stoat/internal/sshx" "github.com/novusedge/stoat/internal/testutil" ) @@ -605,3 +606,54 @@ func TestStateRejectsAnUnknownProvider(t *testing.T) { t.Errorf("StateOf() error = %v, want ErrUnknownProvider", err) } } + +// The point of the endpoint seam is that a VM answering somewhere other than +// a loopback forward is reached there. A fake endpoint is the only way to +// prove SSHCommand consults the provider instead of rebuilding +// sshx.LocalEndpoint for itself. +func TestSSHCommandUsesTheProvidersEndpoint(t *testing.T) { + root(t) + f := fake.Install(t) + f.Ep = sshx.Endpoint{Name: "work", Host: "203.0.113.7", Port: 2022, User: "stoat"} + if err := (&config.VM{Name: "work", Mode: "cloud", RAM: 1024, CPUs: 1, SSHPort: 2200}).Save(); err != nil { + t.Fatal(err) + } + + argv, err := SSHCommand("work") + if err != nil { + t.Fatal(err) + } + got := strings.Join(argv, " ") + if !strings.Contains(got, "stoat@203.0.113.7") { + t.Errorf("SSHCommand = %q, want the provider's host", got) + } + if !strings.Contains(got, "-p 2022") { + t.Errorf("SSHCommand = %q, want the provider's port", got) + } + if strings.Contains(got, "127.0.0.1") { + t.Errorf("SSHCommand = %q, must not fall back to loopback", got) + } +} + +func TestStartAndStopPropagateTheProvidersError(t *testing.T) { + root(t) + f := fake.Install(t) + boom := errors.New("provider refused") + f.StartErr = boom + if err := (&config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200}).Save(); err != nil { + t.Fatal(err) + } + + if err := Start("work"); !errors.Is(err, boom) { + t.Errorf("Start() = %v, want the provider's error", err) + } + + f.StartErr = nil + if err := Start("work"); err != nil { + t.Fatal(err) + } + f.StopErr = boom + if err := Stop("work"); !errors.Is(err, boom) { + t.Errorf("Stop() = %v, want the provider's error", err) + } +} diff --git a/internal/core/wait.go b/internal/core/wait.go index 07afab2..cc16c9e 100644 --- a/internal/core/wait.go +++ b/internal/core/wait.go @@ -180,16 +180,27 @@ func waitReachable(ctx context.Context, v *config.VM) error { return pollUntil(ctx, func() bool { return sshBannerUp(ctx, v) }) } -// sshBannerUp reports whether v's forwarded port answers as a real sshd, not +// sshBannerUp reports whether v's ssh endpoint answers as a real sshd, not // merely accepting TCP. QEMU/libslirp's user-mode networking accepts the // host-side socket before the guest is dialled, so a bare accept() proves // nothing. sshBannerUp requires the "SSH-" identification banner, the same // check sshx.Wait's bannerReady makes. It is a ctx-aware reimplementation, // not a call to bannerReady, because sshx.Wait takes a fixed timeout and // cannot give up early when ctx is cancelled mid-dial. +// +// The address comes from the provider, so a VM that answers somewhere other +// than a loopback forward is probed where it actually lives. func sshBannerUp(ctx context.Context, v *config.VM) bool { + p, err := providerFor(v) + if err != nil { + return false + } + e, err := p.Endpoint(ctx, v) + if err != nil { + return false + } d := net.Dialer{Timeout: time.Second} - c, err := d.DialContext(ctx, "tcp", fmt.Sprintf("127.0.0.1:%d", v.SSHPort)) + c, err := d.DialContext(ctx, "tcp", fmt.Sprintf("%s:%d", e.Host, e.Port)) if err != nil { return false } From 1b511ca0fec9ee1c2b21354c4935c3499d27db99 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Tue, 8 Sep 2026 23:11:44 +0300 Subject: [PATCH 17/17] fix(cli): keep the host gate closed until a provider needs it open Splitting RequireVM moved config's three gates and the CLI boundary onto RequireDataRoot, so on macOS and Windows up, apply, ssh, exec, cp, snapshot and rm proceeded past a refusal and created STOAT_HOME. Nothing sits behind the opened gate yet: every provider is still QEMU. The two hostops functions stay; C3 moves the call sites when a provider runs a VM off this host. Signed-off-by: NovusEdge --- internal/cli/cli.go | 13 +++++++++---- internal/config/config.go | 6 +++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index cbf7931..d187c99 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -495,10 +495,15 @@ func Main(args []string, version string, stdin io.Reader, stdout, stderr io.Writ return runDoctor(a, stdout, stderr) } - // RequireDataRoot never refuses today. It stays on the path a mutating - // command takes before it resolves secrets, reads project scope or writes - // logs, so a future host gate has one place to land. - if err := hostops.RequireDataRoot(); err != nil { + // Every command past this point either drives a machine or writes to the + // data root, so an unqualified host refuses here, before secrets resolve, + // project scope is read, or a log is written. + // + // hostops.RequireDataRoot exists for the day a provider runs a VM + // somewhere other than this host. Moving these call sites onto it needs + // that provider first; opening the gate earlier only lets a macOS user + // reach a QEMU path that cannot work. + if err := hostops.RequireLocalHypervisor(); err != nil { return a.fail(stdout, stderr, err) } if len(a.Params) > 0 { diff --git a/internal/config/config.go b/internal/config/config.go index 51d2463..6ed6b08 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -206,7 +206,7 @@ func Root() string { // EnsureRoot creates the data root and its fixed subdirectories. func EnsureRoot() error { - if err := hostops.RequireDataRoot(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } for _, d := range []string{"isos", "recipes"} { @@ -280,7 +280,7 @@ func (v *VM) ISOPath() string { // Save writes vm.toml, creating the VM directory if needed. func (v *VM) Save() error { - if err := hostops.RequireDataRoot(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } if v.Dir == "" { @@ -385,7 +385,7 @@ var sshPortLine = regexp.MustCompile(`(?m)^\s*sshport\s*=\s*(\d+)\s*$`) // Delete removes the VM directory. It never touches isos/. func (v *VM) Delete() error { - if err := hostops.RequireDataRoot(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } if v.Dir == "" || filepath.Dir(v.Dir) != Root() {