From f0333c7786b7c970f0c664cef16916f6d3168e24 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 02:42:30 +0300 Subject: [PATCH 01/20] test(wire): pin Code type and Codes() coverage Signed-off-by: NovusEdge --- internal/cli/wire/errors.go | 8 ++++++ internal/cli/wire/errors_test.go | 42 ++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index 992ee98c..cb05d0c0 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -33,6 +33,14 @@ const ( CodeInternal = "internal" ) +// Code is a stable, machine-readable error code. Codes are only ever ADDED: +// never renamed, never repurposed, never removed (§2's compatibility +// promise). A consumer MUST treat an unrecognized code as a generic failure. +type Code string + +// Codes returns every declared code, sorted. +func Codes() []Code { return []Code{"STUB"} } + // ErrConfirmationRequired is confirmation_required's sentinel (§2: "new, // CLI-only"). core has no equivalent; --json never prompts (§1), so `rm` // without -y wraps this instead of reading stdin. A future cli.go caller diff --git a/internal/cli/wire/errors_test.go b/internal/cli/wire/errors_test.go index 4d1b49a7..3c532f62 100644 --- a/internal/cli/wire/errors_test.go +++ b/internal/cli/wire/errors_test.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "regexp" + "slices" "testing" "github.com/novusedge/stoat/internal/core" @@ -98,3 +100,43 @@ func TestErrorInfoWithSubject(t *testing.T) { t.Errorf("got %+v", got) } } + +// Every code in codeTable is declared by Codes(). A code that reaches a +// consumer but is absent from the published list is a code no consumer can +// generate a switch for. +func TestCodesCoversTheTable(t *testing.T) { + declared := make(map[Code]bool, len(Codes())) + for _, c := range Codes() { + declared[c] = true + } + for _, row := range codeTable { + if !declared[Code(row.code)] { + t.Errorf("codeTable has %q, Codes() does not", row.code) + } + } + for _, c := range []Code{CodeUsage, CodeInternal} { + if !declared[c] { + t.Errorf("Codes() is missing the CLI-owned code %q", c) + } + } +} + +// Codes are the machine-readable half of the contract: snake_case, unique, +// and sorted, so a generated switch is stable between builds. +func TestCodesAreSnakeCaseUniqueAndSorted(t *testing.T) { + valid := regexp.MustCompile(`^[a-z][a-z0-9_]*$`) + seen := make(map[Code]bool) + codes := Codes() + for _, c := range codes { + if !valid.MatchString(string(c)) { + t.Errorf("code %q is not snake_case", c) + } + if seen[c] { + t.Errorf("code %q is declared twice", c) + } + seen[c] = true + } + if !slices.IsSorted(codes) { + t.Errorf("Codes() = %v, want sorted", codes) + } +} From e4c0282eaa1244d5c27aca309f77f6404eb9b72f Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 02:43:10 +0300 Subject: [PATCH 02/20] test(qemu): pin sentinels for the eight failures Signed-off-by: NovusEdge --- internal/cli/wire/errors.go | 9 ++++++ internal/cli/wire/errors_test.go | 23 ++++++++++++++ internal/qemu/errors.go | 20 ++++++++++++ internal/qemu/errors_test.go | 52 ++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+) create mode 100644 internal/qemu/errors.go create mode 100644 internal/qemu/errors_test.go diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index cb05d0c0..1e32d754 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -31,6 +31,15 @@ const ( CodeUsage = "usage" CodeConfirmationRequired = "confirmation_required" CodeInternal = "internal" + + CodeQemuMissing = "qemu_missing" + CodeKVMUnusable = "kvm_unusable" + CodeQemuStartFailed = "qemu_start_failed" + CodeMonitorUnreachable = "monitor_unreachable" + CodeMonitorRejected = "monitor_rejected" + CodeNoConsolePassword = "no_console_password" + CodeShareInvalid = "share_invalid" + CodeNoXattr = "no_xattr" ) // Code is a stable, machine-readable error code. Codes are only ever ADDED: diff --git a/internal/cli/wire/errors_test.go b/internal/cli/wire/errors_test.go index 3c532f62..d16e8f99 100644 --- a/internal/cli/wire/errors_test.go +++ b/internal/cli/wire/errors_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/qemu" ) func TestMapErrorEveryCoreSentinel(t *testing.T) { @@ -140,3 +141,25 @@ func TestCodesAreSnakeCaseUniqueAndSorted(t *testing.T) { t.Errorf("Codes() = %v, want sorted", codes) } } + +// Each qemu sentinel has exactly one row in codeTable. A sentinel with no row +// reaches a consumer as "internal" with prose, which is the shape this whole +// table exists to remove. +func TestEveryQemuSentinelHasOneRow(t *testing.T) { + want := map[error]string{ + qemu.ErrBinaryMissing: CodeQemuMissing, + qemu.ErrKVMUnusable: CodeKVMUnusable, + qemu.ErrStartFailed: CodeQemuStartFailed, + qemu.ErrMonitorUnreachable: CodeMonitorUnreachable, + qemu.ErrMonitorRejected: CodeMonitorRejected, + qemu.ErrNoConsolePassword: CodeNoConsolePassword, + qemu.ErrShareInvalid: CodeShareInvalid, + qemu.ErrNoXattr: CodeNoXattr, + } + for sentinel, code := range want { + got := MapError(fmt.Errorf("%w: subject", sentinel)) + if got.Code != code { + t.Errorf("MapError(%v).Code = %q, want %q", sentinel, got.Code, code) + } + } +} diff --git a/internal/qemu/errors.go b/internal/qemu/errors.go new file mode 100644 index 00000000..caa7ef1c --- /dev/null +++ b/internal/qemu/errors.go @@ -0,0 +1,20 @@ +package qemu + +import "errors" + +// Sentinels for the failures a caller branches on. Each is wrapped in front +// of the message the call site already printed, so the text is unchanged and +// errors.Is reaches the sentinel through core's own wrapping. +// +// ErrNoXattr lives in xattr.go, beside the probe that raises it. +var ( + ErrBinaryMissing = errors.New("qemu binary not found") + ErrKVMUnusable = errors.New("/dev/kvm not usable") + ErrStartFailed = errors.New("qemu failed to start") + ErrMonitorUnreachable = errors.New("qemu monitor unreachable") + // ErrMonitorRejected is qemu answering a command with an error object. + // The command reached qemu; qemu refused it. + ErrMonitorRejected = errors.New("qemu monitor rejected the command") + ErrNoConsolePassword = errors.New("no console password set") + ErrShareInvalid = errors.New("share is not a directory") +) diff --git a/internal/qemu/errors_test.go b/internal/qemu/errors_test.go new file mode 100644 index 00000000..679e92a5 --- /dev/null +++ b/internal/qemu/errors_test.go @@ -0,0 +1,52 @@ +package qemu + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/novusedge/stoat/internal/config" +) + +// A qemu binary that is not on PATH is the first thing Preflight checks, so +// an empty PATH reaches ErrBinaryMissing before the /dev/kvm open. +func TestPreflightReportsAMissingBinary(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + if err := Preflight(); !errors.Is(err, ErrBinaryMissing) { + t.Errorf("Preflight() = %v, want ErrBinaryMissing", err) + } +} + +// A share that names a file, not a directory, is a vm.toml mistake a caller +// corrects by editing the field, so it is told apart from a missing path. +func TestPrepareSharesReportsAShareThatIsAFile(t *testing.T) { + dir := t.TempDir() + t.Setenv("STOAT_HOME", dir) + file := filepath.Join(dir, "notadir") + if err := os.WriteFile(file, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + v := &config.VM{Name: "work", Dir: filepath.Join(dir, "work"), Share: file} + if err := prepareShares(v); !errors.Is(err, ErrShareInvalid) { + t.Errorf("prepareShares() = %v, want ErrShareInvalid", err) + } +} + +// A stopped VM has no QMP socket to dial, and a caller that gets +// ErrMonitorUnreachable knows to start the VM rather than to retry. +func TestDialQMPReportsAnUnreachableMonitor(t *testing.T) { + v := &config.VM{Name: "work", Dir: t.TempDir()} + if _, err := dialQMP(v); !errors.Is(err, ErrMonitorUnreachable) { + t.Errorf("dialQMP() = %v, want ErrMonitorUnreachable", err) + } +} + +// TypeConsolePassword refuses a VM with no password rather than typing +// nothing at the login prompt and reporting success. +func TestTypeConsolePasswordReportsNoPassword(t *testing.T) { + v := &config.VM{Name: "work", Dir: t.TempDir()} + if err := TypeConsolePassword(v); !errors.Is(err, ErrNoConsolePassword) { + t.Errorf("TypeConsolePassword() = %v, want ErrNoConsolePassword", err) + } +} From c7edffe5ae6d1a71524f47a0700e45e1662541a7 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 02:43:45 +0300 Subject: [PATCH 03/20] test(iso): pin sentinels for download failures Signed-off-by: NovusEdge --- internal/cli/wire/errors.go | 5 +++++ internal/cli/wire/errors_test.go | 17 ++++++++++++++++ internal/iso/errors.go | 13 ++++++++++++ internal/iso/iso_test.go | 35 ++++++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+) create mode 100644 internal/iso/errors.go diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index 1e32d754..39f554f5 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -40,6 +40,11 @@ const ( CodeNoConsolePassword = "no_console_password" CodeShareInvalid = "share_invalid" CodeNoXattr = "no_xattr" + + CodeDownloadFailed = "download_failed" + CodeDownloadStalled = "download_stalled" + CodeChecksumMismatch = "checksum_mismatch" + CodeNoSuchImage = "no_such_image" ) // Code is a stable, machine-readable error code. Codes are only ever ADDED: diff --git a/internal/cli/wire/errors_test.go b/internal/cli/wire/errors_test.go index d16e8f99..b48d2295 100644 --- a/internal/cli/wire/errors_test.go +++ b/internal/cli/wire/errors_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/iso" "github.com/novusedge/stoat/internal/qemu" ) @@ -163,3 +164,19 @@ func TestEveryQemuSentinelHasOneRow(t *testing.T) { } } } + +// Each iso sentinel has exactly one row in codeTable. +func TestEveryISOSentinelHasOneRow(t *testing.T) { + want := map[error]string{ + iso.ErrDownloadFailed: CodeDownloadFailed, + iso.ErrDownloadStalled: CodeDownloadStalled, + iso.ErrChecksumMismatch: CodeChecksumMismatch, + iso.ErrNoSuchImage: CodeNoSuchImage, + } + for sentinel, code := range want { + got := MapError(fmt.Errorf("%w: subject", sentinel)) + if got.Code != code { + t.Errorf("MapError(%v).Code = %q, want %q", sentinel, got.Code, code) + } + } +} diff --git a/internal/iso/errors.go b/internal/iso/errors.go new file mode 100644 index 00000000..331dcb92 --- /dev/null +++ b/internal/iso/errors.go @@ -0,0 +1,13 @@ +package iso + +import "errors" + +// Sentinels for the failures a caller branches on. A stalled download and a +// checksum mismatch are both "the image is not here", but the first is worth +// retrying on the same mirror and the second is not. +var ( + ErrDownloadFailed = errors.New("download failed") + ErrDownloadStalled = errors.New("download stalled") + ErrChecksumMismatch = errors.New("checksum mismatch") + ErrNoSuchImage = errors.New("no such image") +) diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index bfbb46d3..57a80d1a 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -6,6 +6,7 @@ import ( "crypto/sha256" "crypto/sha512" "encoding/hex" + "errors" "fmt" "net/http" "net/http/httptest" @@ -728,3 +729,37 @@ func TestDownloadCancelStopsTheTransfer(t *testing.T) { t.Error("a cancelled download was renamed into place") } } + +// A mirror that answers with a status other than 200 is a download failure a +// caller retries; it is not a broken stoat. +func TestDownloadReportsAFailedFetch(t *testing.T) { + t.Setenv("STOAT_HOME", t.TempDir()) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + r := &Release{File: "broken.iso", URL: srv.URL + "/broken.iso"} + if _, err := Download(context.Background(), r, nil); !errors.Is(err, ErrDownloadFailed) { + t.Errorf("Download() = %v, want ErrDownloadFailed", err) + } +} + +// Bytes that do not match the published digest are a mismatch, not a +// transport failure: the fetch succeeded and the content is wrong. +func TestDownloadReportsAChecksumMismatch(t *testing.T) { + t.Setenv("STOAT_HOME", t.TempDir()) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + _, _ = w.Write([]byte("not the expected bytes")) + })) + defer srv.Close() + + r := &Release{ + File: "mismatch.iso", + URL: srv.URL + "/mismatch.iso", + SHA256: strings.Repeat("0", 64), + } + if _, err := Download(context.Background(), r, nil); !errors.Is(err, ErrChecksumMismatch) { + t.Errorf("Download() = %v, want ErrChecksumMismatch", err) + } +} From 28ce4ff90fc75bc492a2eb65be122225cb4860b2 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 02:44:03 +0300 Subject: [PATCH 04/20] test(core): pin member lists for status enums Signed-off-by: NovusEdge --- internal/core/access.go | 6 ++++ internal/core/enums_test.go | 59 +++++++++++++++++++++++++++++++++++++ internal/core/health.go | 14 +++++++++ internal/core/vm.go | 3 ++ internal/core/wait.go | 6 ++++ 5 files changed, 88 insertions(+) create mode 100644 internal/core/enums_test.go create mode 100644 internal/core/health.go diff --git a/internal/core/access.go b/internal/core/access.go index fedd4ae4..b672e384 100644 --- a/internal/core/access.go +++ b/internal/core/access.go @@ -48,6 +48,12 @@ const ( // reading the wrong file. var ErrUnknownWhich = fmt.Errorf("unknown log selector") +// Whichs returns every log a caller can ask for. +func Whichs() []Which { return nil } + +// Valid reports whether w is one of Whichs(). +func (w Which) Valid() bool { return false } + // Logs opens the requested log file for VM name. // // A missing file is normal, not an error. A VM that never started has no diff --git a/internal/core/enums_test.go b/internal/core/enums_test.go new file mode 100644 index 00000000..22a6cf21 --- /dev/null +++ b/internal/core/enums_test.go @@ -0,0 +1,59 @@ +package core + +import ( + "context" + "errors" + "testing" +) + +// Wait refuses an Until it does not know, rather than falling into a default +// branch that silently means one of the three real ones. +func TestWaitRefusesAnUnknownUntil(t *testing.T) { + root(t) + err := Wait(context.Background(), "work", Until("nope")) + if !errors.Is(err, ErrInvalidSpec) { + t.Errorf("Wait(nope) = %v, want ErrInvalidSpec", err) + } +} + +// Logs refuses a Which it does not know. Before Valid(), the default branch +// read the apply log for any unrecognised selector. +func TestLogsRefusesAnUnknownWhich(t *testing.T) { + root(t) + if _, err := Logs("work", Which("nope")); !errors.Is(err, ErrUnknownWhich) { + t.Errorf("Logs(nope) = %v, want ErrUnknownWhich", err) + } +} + +// The member lists are what a consumer generates a switch from, so each one +// holds exactly the declared values and calls them valid. +func TestMemberListsMatchTheDeclaredValues(t *testing.T) { + if got, want := States(), []State{StateStopped, StateRunning, StateBroken}; !equalStates(got, want) { + t.Errorf("States() = %v, want %v", got, want) + } + for _, u := range Untils() { + if !u.Valid() { + t.Errorf("Untils() lists %q, which Valid() rejects", u) + } + } + for _, w := range Whichs() { + if !w.Valid() { + t.Errorf("Whichs() lists %q, which Valid() rejects", w) + } + } + if got, want := Healths(), []Health{HealthOK, HealthFailed, HealthUnknown}; len(got) != len(want) { + t.Errorf("Healths() = %v, want %v", got, want) + } +} + +func equalStates(a, b []State) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/internal/core/health.go b/internal/core/health.go new file mode 100644 index 00000000..e5763828 --- /dev/null +++ b/internal/core/health.go @@ -0,0 +1,14 @@ +package core + +// Health is a recipe's health-check result. The recipe contract writes the +// checks that report it; this declares the values they may report. +type Health string + +const ( + HealthOK Health = "ok" + HealthFailed Health = "failed" + HealthUnknown Health = "unknown" +) + +// Healths returns every declared health value. +func Healths() []Health { return nil } diff --git a/internal/core/vm.go b/internal/core/vm.go index a747f050..61f5f185 100644 --- a/internal/core/vm.go +++ b/internal/core/vm.go @@ -36,6 +36,9 @@ const ( StateBroken State = "broken" ) +// States returns every State this package produces. +func States() []State { return nil } + // ErrNotRunning is returned by Stop (and by Destroy, transitively) for a VM // that is not running. qemu.Stop itself treats "already stopped" as a // no-op; Stop checks first so the caller gets a typed error instead. diff --git a/internal/core/wait.go b/internal/core/wait.go index b16b2205..abdb32c2 100644 --- a/internal/core/wait.go +++ b/internal/core/wait.go @@ -33,6 +33,12 @@ const ( UntilStopped Until = "stopped" ) +// Untils returns every state Wait can block for. +func Untils() []Until { return nil } + +// Valid reports whether u is one of Untils(). +func (u Until) Valid() bool { return false } + // ErrCannotReach is returned by Wait for a VM that cannot, by construction, // ever reach the requested state: a stopped VM asked to become Reachable, or // a VM with no recipes asked to become Applied. Wait checks for these cases From 5a4aa715da3c44d65ef1984f958cae11a49f4381 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 02:48:51 +0300 Subject: [PATCH 05/20] refactor(wire): implement Codes() Signed-off-by: NovusEdge --- internal/cli/wire/errors.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index 39f554f5..8c136573 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -3,6 +3,7 @@ package wire import ( "context" "errors" + "slices" "github.com/novusedge/stoat/internal/core" ) @@ -52,8 +53,25 @@ const ( // promise). A consumer MUST treat an unrecognized code as a generic failure. type Code string -// Codes returns every declared code, sorted. -func Codes() []Code { return []Code{"STUB"} } +// Codes returns every declared code, sorted. Built from the same string +// constants codeTable and ErrorInfo.Code use, so a code added to one and +// forgotten here fails TestCodesCoversTheTable. +func Codes() []Code { + out := []Code{ + CodeNotFound, CodeBroken, CodeNameTaken, CodeInvalidSpec, + CodeImageNotDownloaded, CodeRecipeNotApplicable, CodeNotRunning, + CodeAlreadyRunning, CodeNoDisk, CodeImmutableField, CodeDiskShrink, + CodeCannotReach, CodeUnknownLog, CodeTimeout, CodeCanceled, + CodeUsage, CodeConfirmationRequired, CodeInternal, + CodeQemuMissing, CodeKVMUnusable, CodeQemuStartFailed, + CodeMonitorUnreachable, CodeMonitorRejected, CodeNoConsolePassword, + CodeShareInvalid, CodeNoXattr, + CodeDownloadFailed, CodeDownloadStalled, CodeChecksumMismatch, + CodeNoSuchImage, + } + slices.Sort(out) + return out +} // ErrConfirmationRequired is confirmation_required's sentinel (§2: "new, // CLI-only"). core has no equivalent; --json never prompts (§1), so `rm` From 75f9509f2fe828a7b9afe432f7cb0d8da1d5a8e8 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 02:51:17 +0300 Subject: [PATCH 06/20] feat(qemu): wrap failures with typed sentinels Signed-off-by: NovusEdge --- internal/cli/wire/errors.go | 9 +++++++++ internal/qemu/qmp.go | 8 ++++---- internal/qemu/run.go | 6 +++--- internal/qemu/sendkey.go | 6 +++--- internal/qemu/share.go | 2 +- 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index 8c136573..2ad995b6 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -6,6 +6,7 @@ import ( "slices" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/qemu" ) // Stable snake_case error codes (§2), one per typed error in internal/core @@ -108,6 +109,14 @@ var codeTable = []struct { {CodeTimeout, context.DeadlineExceeded}, {CodeCanceled, context.Canceled}, {CodeConfirmationRequired, ErrConfirmationRequired}, + {CodeQemuMissing, qemu.ErrBinaryMissing}, + {CodeKVMUnusable, qemu.ErrKVMUnusable}, + {CodeQemuStartFailed, qemu.ErrStartFailed}, + {CodeMonitorUnreachable, qemu.ErrMonitorUnreachable}, + {CodeMonitorRejected, qemu.ErrMonitorRejected}, + {CodeNoConsolePassword, qemu.ErrNoConsolePassword}, + {CodeShareInvalid, qemu.ErrShareInvalid}, + {CodeNoXattr, qemu.ErrNoXattr}, } // MapError converts a core (or context) error into an ErrorInfo, walking diff --git a/internal/qemu/qmp.go b/internal/qemu/qmp.go index fff54f73..a6de11d6 100644 --- a/internal/qemu/qmp.go +++ b/internal/qemu/qmp.go @@ -35,7 +35,7 @@ type qmp struct { func dialQMP(v *config.VM) (*qmp, error) { c, err := net.Dial("unix", v.QMPPath()) if err != nil { - return nil, fmt.Errorf("qmp: %w", err) + return nil, fmt.Errorf("%w: qmp: %w", ErrMonitorUnreachable, err) } if err := c.SetDeadline(time.Now().Add(qmpTimeout)); err != nil { _ = c.Close() @@ -50,11 +50,11 @@ func dialQMP(v *config.VM) (*qmp, error) { } if err := q.dec.Decode(&greeting); err != nil { _ = c.Close() - return nil, fmt.Errorf("qmp greeting: %w", err) + return nil, fmt.Errorf("%w: qmp greeting: %w", ErrMonitorUnreachable, err) } if greeting.QMP == nil { _ = c.Close() - return nil, fmt.Errorf("qmp: not a QMP socket (no greeting)") + return nil, fmt.Errorf("%w: qmp: not a QMP socket (no greeting)", ErrMonitorUnreachable) } if _, err := q.command("qmp_capabilities", nil); err != nil { _ = c.Close() @@ -97,7 +97,7 @@ func (q *qmp) command(name string, args map[string]any) (json.RawMessage, error) continue // asynchronous event, not our reply } if resp.Error != nil { - return nil, fmt.Errorf("qmp %s: %s", name, resp.Error.Desc) + return nil, fmt.Errorf("%w: qmp %s: %s", ErrMonitorRejected, name, resp.Error.Desc) } return resp.Return, nil } diff --git a/internal/qemu/run.go b/internal/qemu/run.go index 7424d353..d27961dd 100644 --- a/internal/qemu/run.go +++ b/internal/qemu/run.go @@ -18,11 +18,11 @@ import ( // Preflight reports why VMs cannot start, or nil if they can. func Preflight() error { if _, err := exec.LookPath(Binary); err != nil { - return fmt.Errorf("%s not found in PATH", Binary) + return fmt.Errorf("%w: %s not found in PATH", ErrBinaryMissing, Binary) } f, err := os.OpenFile("/dev/kvm", os.O_RDWR, 0) if err != nil { - return fmt.Errorf("/dev/kvm not usable: %w (are you in the kvm group?)", err) + return fmt.Errorf("%w: /dev/kvm not usable: %w (are you in the kvm group?)", ErrKVMUnusable, err) } _ = f.Close() return nil @@ -141,7 +141,7 @@ func Start(v *config.VM) error { msg = err.Error() } logx.L().Error("start failed", "vm", v.Name, "err", msg) - return fmt.Errorf("qemu failed to start: %s%s", msg, explainDisplayFailure(msg)) + return fmt.Errorf("%w: qemu failed to start: %s%s", ErrStartFailed, msg, explainDisplayFailure(msg)) } // Log how to get in, not just that it started. This is the line someone // greps for when a VM is up but unreachable. diff --git a/internal/qemu/sendkey.go b/internal/qemu/sendkey.go index 876a9a2c..7548e3ae 100644 --- a/internal/qemu/sendkey.go +++ b/internal/qemu/sendkey.go @@ -54,7 +54,7 @@ func passwordKeys(password string) ([]string, error) { // password to send. func TypeConsolePassword(v *config.VM) error { if v.ConsolePassword == "" { - return fmt.Errorf("%s has no console password set", v.Name) + return fmt.Errorf("%w: %s has no console password set", ErrNoConsolePassword, v.Name) } keys, err := passwordKeys(v.ConsolePassword) if err != nil { @@ -62,12 +62,12 @@ func TypeConsolePassword(v *config.VM) error { } c, err := dialMonitor(v) if err != nil { - return fmt.Errorf("qemu monitor: %w", err) + return fmt.Errorf("%w: qemu monitor: %w", ErrMonitorUnreachable, err) } defer func() { _ = c.Close() }() for _, k := range keys { if _, err := fmt.Fprintln(c, "sendkey "+k); err != nil { - return fmt.Errorf("qemu monitor: %w", err) + return fmt.Errorf("%w: qemu monitor: %w", ErrMonitorUnreachable, err) } time.Sleep(consoleKeyDelay) } diff --git a/internal/qemu/share.go b/internal/qemu/share.go index 34ef1098..aaa81430 100644 --- a/internal/qemu/share.go +++ b/internal/qemu/share.go @@ -34,7 +34,7 @@ func prepareShares(v *config.VM) error { return fmt.Errorf("share %s for %s: %w", v.Share, v.Name, err) } if !st.IsDir() { - return fmt.Errorf("share %s for %s is not a directory", v.Share, v.Name) + return fmt.Errorf("%w: share %s for %s is not a directory", ErrShareInvalid, v.Share, v.Name) } return nil } From 3426a39963eac5e824ed74b7b8a15e9da2da726f Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 02:54:03 +0300 Subject: [PATCH 07/20] feat(iso): wrap download failures with typed sentinels Signed-off-by: NovusEdge --- internal/cli/wire/errors.go | 5 +++++ internal/iso/iso.go | 14 +++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index 2ad995b6..f9f3acfb 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -6,6 +6,7 @@ import ( "slices" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/iso" "github.com/novusedge/stoat/internal/qemu" ) @@ -117,6 +118,10 @@ var codeTable = []struct { {CodeNoConsolePassword, qemu.ErrNoConsolePassword}, {CodeShareInvalid, qemu.ErrShareInvalid}, {CodeNoXattr, qemu.ErrNoXattr}, + {CodeDownloadFailed, iso.ErrDownloadFailed}, + {CodeDownloadStalled, iso.ErrDownloadStalled}, + {CodeChecksumMismatch, iso.ErrChecksumMismatch}, + {CodeNoSuchImage, iso.ErrNoSuchImage}, } // MapError converts a core (or context) error into an ErrorInfo, walking diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 70216915..434ceab1 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -348,7 +348,7 @@ func Latest(flavor string) (*Release, error) { } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("index: %s", resp.Status) + return nil, fmt.Errorf("%w: index: %s", ErrDownloadFailed, resp.Status) } var releases []Release if err := yaml.NewDecoder(resp.Body).Decode(&releases); err != nil { @@ -359,7 +359,7 @@ func Latest(flavor string) (*Release, error) { return &releases[i], nil } } - return nil, fmt.Errorf("no %s iso in index", flavor) + return nil, fmt.Errorf("%w: no %s iso in index", ErrNoSuchImage, flavor) } // Resolve turns a catalog Entry into a concrete Release ready for Download. @@ -412,7 +412,7 @@ func fetchChecksum(checksumURL, filename string) (string, error) { } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("checksum: %s", resp.Status) + return "", fmt.Errorf("%w: checksum: %s", ErrDownloadFailed, resp.Status) } body, err := io.ReadAll(resp.Body) if err != nil { @@ -450,7 +450,7 @@ func parseChecksum(body []byte, filename string) (string, error) { return strings.ToLower(fields[0]), nil } } - return "", fmt.Errorf("no checksum for %s", filename) + return "", fmt.Errorf("%w: no checksum for %s", ErrDownloadFailed, filename) } // newDigest picks the hash algorithm by the expected digest's hex length: @@ -530,7 +530,7 @@ func Download(ctx context.Context, r *Release, progress func(done, total int64)) } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("download: %s", resp.Status) + return "", fmt.Errorf("%w: download: %s", ErrDownloadFailed, resp.Status) } // Cancelling the request unblocks a Read that would otherwise never @@ -592,7 +592,7 @@ func Download(ctx context.Context, r *Release, progress func(done, total int64)) _ = f.Close() _ = os.Remove(part) if stalled.Load() { - return "", fmt.Errorf("download stalled: no data for %s", stallTimeout) + return "", fmt.Errorf("%w: download stalled: no data for %s", ErrDownloadStalled, stallTimeout) } return "", rerr } @@ -609,7 +609,7 @@ func Download(ctx context.Context, r *Release, progress func(done, total int64)) if r.SHA256 != "" { if got := hex.EncodeToString(h.Sum(nil)); got != r.SHA256 { _ = os.Remove(part) - return "", fmt.Errorf("checksum mismatch: got %s, want %s", got, r.SHA256) + return "", fmt.Errorf("%w: checksum mismatch: got %s, want %s", ErrChecksumMismatch, got, r.SHA256) } r.Verified = true } From 159cc70c1e681216e3ca46042f30f8706b61cef1 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 02:56:56 +0300 Subject: [PATCH 08/20] feat(core): member lists and Valid for status enums Signed-off-by: NovusEdge --- internal/cli/wire/status.go | 14 ++++++++++++++ internal/core/access.go | 17 ++++++++--------- internal/core/health.go | 2 +- internal/core/vm.go | 5 +++-- internal/core/wait.go | 15 +++++++++------ 5 files changed, 35 insertions(+), 18 deletions(-) create mode 100644 internal/cli/wire/status.go diff --git a/internal/cli/wire/status.go b/internal/cli/wire/status.go new file mode 100644 index 00000000..138d293d --- /dev/null +++ b/internal/cli/wire/status.go @@ -0,0 +1,14 @@ +package wire + +// The status values a DTO carries, so no constructor writes a raw literal. +// They are plain strings, not a named type: wire.VM.State is a string field +// and a consumer reads the JSON, not the Go type. +const ( + StateStopped = "stopped" + StateRunning = "running" + StateBroken = "broken" + + HealthOK = "ok" + HealthFailed = "failed" + HealthUnknown = "unknown" +) diff --git a/internal/core/access.go b/internal/core/access.go index b672e384..8e6ff76b 100644 --- a/internal/core/access.go +++ b/internal/core/access.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "slices" "github.com/novusedge/stoat/internal/config" "github.com/novusedge/stoat/internal/sshx" @@ -49,10 +50,10 @@ const ( var ErrUnknownWhich = fmt.Errorf("unknown log selector") // Whichs returns every log a caller can ask for. -func Whichs() []Which { return nil } +func Whichs() []Which { return []Which{WhichConsole, WhichApply} } // Valid reports whether w is one of Whichs(). -func (w Which) Valid() bool { return false } +func (w Which) Valid() bool { return slices.Contains(Whichs(), w) } // Logs opens the requested log file for VM name. // @@ -66,6 +67,9 @@ func (w Which) Valid() bool { return false } // field, and a broken VM is exactly the one a user wants console output from, // so Logs builds a directory-only VM, like Destroy does. func Logs(name string, which Which) (io.ReadCloser, error) { + if !which.Valid() { + return nil, fmt.Errorf("%w: %q", ErrUnknownWhich, which) + } v, err := load(name) switch { case errors.Is(err, ErrBroken): @@ -74,14 +78,9 @@ func Logs(name string, which Which) (io.ReadCloser, error) { return nil, err } - var path string - switch which { - case WhichConsole: + path := v.ProvisionLogPath() + if which == WhichConsole { path = v.ConsoleLogPath() - case WhichApply: - path = v.ProvisionLogPath() - default: - return nil, fmt.Errorf("%w: %q", ErrUnknownWhich, which) } f, err := os.Open(path) diff --git a/internal/core/health.go b/internal/core/health.go index e5763828..5913e8f5 100644 --- a/internal/core/health.go +++ b/internal/core/health.go @@ -11,4 +11,4 @@ const ( ) // Healths returns every declared health value. -func Healths() []Health { return nil } +func Healths() []Health { return []Health{HealthOK, HealthFailed, HealthUnknown} } diff --git a/internal/core/vm.go b/internal/core/vm.go index 61f5f185..bc43f57c 100644 --- a/internal/core/vm.go +++ b/internal/core/vm.go @@ -36,8 +36,9 @@ const ( StateBroken State = "broken" ) -// States returns every State this package produces. -func States() []State { return nil } +// States returns every State this package produces. Three of the design +// doc's six; see State's own comment for why the other three are absent. +func States() []State { return []State{StateStopped, StateRunning, StateBroken} } // ErrNotRunning is returned by Stop (and by Destroy, transitively) for a VM // that is not running. qemu.Stop itself treats "already stopped" as a diff --git a/internal/core/wait.go b/internal/core/wait.go index abdb32c2..79d2a04e 100644 --- a/internal/core/wait.go +++ b/internal/core/wait.go @@ -7,6 +7,7 @@ import ( "io" "net" "os" + "slices" "strings" "time" @@ -34,10 +35,11 @@ const ( ) // Untils returns every state Wait can block for. -func Untils() []Until { return nil } +func Untils() []Until { return []Until{UntilReachable, UntilApplied, UntilStopped} } -// Valid reports whether u is one of Untils(). -func (u Until) Valid() bool { return false } +// Valid reports whether u is one of Untils(). Wait calls it before it loads +// the VM, so a typo fails with the reason rather than with "not found". +func (u Until) Valid() bool { return slices.Contains(Untils(), u) } // ErrCannotReach is returned by Wait for a VM that cannot, by construction, // ever reach the requested state: a stopped VM asked to become Reachable, or @@ -71,19 +73,20 @@ const InstallTimeout = 15 * time.Minute // already mid-boot by List. Wait lets that caller block on the outcome // instead of re-polling Get in a loop. func Wait(ctx context.Context, name string, until Until) error { + if !until.Valid() { + return fmt.Errorf("%w: unknown Until %q", ErrInvalidSpec, until) + } v, err := load(name) if err != nil { return err } switch until { - case UntilReachable: - return waitReachable(ctx, v) case UntilApplied: return waitApplied(ctx, v) case UntilStopped: return waitStopped(ctx, v) default: - return fmt.Errorf("%w: unknown Until %q", ErrInvalidSpec, until) + return waitReachable(ctx, v) } } From 5cfa8e3057f5234ccab7bb2c41187795918de60e Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:10:06 +0300 Subject: [PATCH 09/20] fix(wire): type Code and wrap qemu already-running Retype every CodeX constant, codeTable's field and ErrorInfo.Code to Code, and add qemu.ErrAlreadyRunning with its codeTable row. Both were missing from the prior commits despite being in each task's Produces list. Signed-off-by: NovusEdge --- internal/cli/json_test.go | 4 +- internal/cli/subcommands_test.go | 4 +- internal/cli/wait_test.go | 8 ++-- internal/cli/wire/envelope.go | 2 +- internal/cli/wire/envelope_test.go | 2 +- internal/cli/wire/errors.go | 73 +++++++++++++++--------------- internal/cli/wire/errors_test.go | 11 +++-- internal/qemu/errors.go | 5 ++ internal/qemu/run.go | 2 +- 9 files changed, 60 insertions(+), 51 deletions(-) diff --git a/internal/cli/json_test.go b/internal/cli/json_test.go index 93e93e35..594e32b6 100644 --- a/internal/cli/json_test.go +++ b/internal/cli/json_test.go @@ -63,7 +63,7 @@ func TestJSONEnvelopeEveryCommand(t *testing.T) { name string argv []string ok bool - code string // error.code when ok is false + code wire.Code // error.code when ok is false exit int }{ {name: "ls", argv: []string{"ls"}, ok: true, exit: ExitOK}, @@ -139,7 +139,7 @@ func TestJSONEnvelopeEveryCommand(t *testing.T) { if errObj == nil { t.Fatalf("failed result has no error object: %v", res) } - if errObj["code"] != tt.code { + if errObj["code"] != string(tt.code) { t.Errorf("error.code = %v, want %q", errObj["code"], tt.code) } if msg, _ := errObj["message"].(string); msg == "" { diff --git a/internal/cli/subcommands_test.go b/internal/cli/subcommands_test.go index ab75835e..569968d7 100644 --- a/internal/cli/subcommands_test.go +++ b/internal/cli/subcommands_test.go @@ -70,7 +70,7 @@ func TestGetMissingVMIsNotFound(t *testing.T) { t.Fatalf("exit = %d, want %d", code, ExitFail) } errObj, _ := result(t, objs)["error"].(map[string]any) - if errObj["code"] != wire.CodeNotFound { + if errObj["code"] != string(wire.CodeNotFound) { t.Errorf("code = %v, want %q", errObj["code"], wire.CodeNotFound) } } @@ -277,7 +277,7 @@ func TestApplyMissingVMIsNotFound(t *testing.T) { t.Fatalf("exit = %d, want %d", code, ExitFail) } errObj, _ := result(t, objs)["error"].(map[string]any) - if errObj["code"] != wire.CodeNotFound { + if errObj["code"] != string(wire.CodeNotFound) { t.Errorf("code = %v, want %q", errObj["code"], wire.CodeNotFound) } } diff --git a/internal/cli/wait_test.go b/internal/cli/wait_test.go index 97fa87f4..274052d0 100644 --- a/internal/cli/wait_test.go +++ b/internal/cli/wait_test.go @@ -17,7 +17,7 @@ func TestWaitMissingVM(t *testing.T) { } res := result(t, objs) errObj, _ := res["error"].(map[string]any) - if errObj["code"] != wire.CodeNotFound { + if errObj["code"] != string(wire.CodeNotFound) { t.Errorf("error.code = %v, want %q", errObj["code"], wire.CodeNotFound) } } @@ -37,7 +37,7 @@ func TestWaitCannotReach(t *testing.T) { } res := result(t, objs) errObj, _ := res["error"].(map[string]any) - if errObj["code"] != wire.CodeCannotReach { + if errObj["code"] != string(wire.CodeCannotReach) { t.Errorf("error.code = %v, want %q", errObj["code"], wire.CodeCannotReach) } } @@ -63,7 +63,7 @@ func TestWaitTimeout(t *testing.T) { } res := result(t, objs) errObj, _ := res["error"].(map[string]any) - if errObj["code"] != wire.CodeTimeout { + if errObj["code"] != string(wire.CodeTimeout) { t.Errorf("error.code = %v, want %q", errObj["code"], wire.CodeTimeout) } } @@ -94,7 +94,7 @@ func TestWaitUsageErrorsUnderJSON(t *testing.T) { } res := result(t, objs) errObj, _ := res["error"].(map[string]any) - if errObj["code"] != wire.CodeUsage { + if errObj["code"] != string(wire.CodeUsage) { t.Errorf("Main(%v) error.code = %v, want %q", args, errObj["code"], wire.CodeUsage) } } diff --git a/internal/cli/wire/envelope.go b/internal/cli/wire/envelope.go index 3f108fb6..a4a95d49 100644 --- a/internal/cli/wire/envelope.go +++ b/internal/cli/wire/envelope.go @@ -42,7 +42,7 @@ const ( // site supplies them explicitly rather than this package trying to parse // them back out. type ErrorInfo struct { - Code string `json:"code"` + Code Code `json:"code"` Message string `json:"message"` // Subject names what the error is about ("work", "os"); absent when // there is nothing more specific than Message to give. diff --git a/internal/cli/wire/envelope_test.go b/internal/cli/wire/envelope_test.go index db154a9f..3afc2e16 100644 --- a/internal/cli/wire/envelope_test.go +++ b/internal/cli/wire/envelope_test.go @@ -63,7 +63,7 @@ func TestEmitterErrorResultOmitsData(t *testing.T) { if !ok { t.Fatalf("expected an error object, got %v", got["error"]) } - if errObj["code"] != CodeConfirmationRequired { + if errObj["code"] != string(CodeConfirmationRequired) { t.Errorf("code = %v, want %s", errObj["code"], CodeConfirmationRequired) } } diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index f9f3acfb..538626f6 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -10,51 +10,51 @@ import ( "github.com/novusedge/stoat/internal/qemu" ) +// Code is a stable, machine-readable error code. Codes are only ever ADDED: +// never renamed, never repurposed, never removed (§2's compatibility +// promise). A consumer MUST treat an unrecognized code as a generic failure. +type Code string + // Stable snake_case error codes (§2), one per typed error in internal/core // plus the three the CLI layer owns (usage, confirmation_required, // internal). Codes are only ever ADDED: never renamed, never repurposed, // never removed (§2's compatibility promise). A consumer MUST treat an // unrecognized code as a generic failure, never crash on it. const ( - CodeNotFound = "not_found" - CodeBroken = "broken" - CodeNameTaken = "name_taken" - CodeInvalidSpec = "invalid_spec" - CodeImageNotDownloaded = "image_not_downloaded" - CodeRecipeNotApplicable = "recipe_not_applicable" - CodeNotRunning = "not_running" - CodeAlreadyRunning = "already_running" - CodeNoDisk = "no_disk" - CodeImmutableField = "immutable_field" - CodeDiskShrink = "disk_shrink" - CodeCannotReach = "cannot_reach" - CodeUnknownLog = "unknown_log" - CodeTimeout = "timeout" - CodeCanceled = "canceled" - CodeUsage = "usage" - CodeConfirmationRequired = "confirmation_required" - CodeInternal = "internal" + CodeNotFound Code = "not_found" + CodeBroken Code = "broken" + CodeNameTaken Code = "name_taken" + CodeInvalidSpec Code = "invalid_spec" + CodeImageNotDownloaded Code = "image_not_downloaded" + CodeRecipeNotApplicable Code = "recipe_not_applicable" + CodeNotRunning Code = "not_running" + CodeAlreadyRunning Code = "already_running" + CodeNoDisk Code = "no_disk" + CodeImmutableField Code = "immutable_field" + CodeDiskShrink Code = "disk_shrink" + CodeCannotReach Code = "cannot_reach" + CodeUnknownLog Code = "unknown_log" + CodeTimeout Code = "timeout" + CodeCanceled Code = "canceled" + CodeUsage Code = "usage" + CodeConfirmationRequired Code = "confirmation_required" + CodeInternal Code = "internal" - CodeQemuMissing = "qemu_missing" - CodeKVMUnusable = "kvm_unusable" - CodeQemuStartFailed = "qemu_start_failed" - CodeMonitorUnreachable = "monitor_unreachable" - CodeMonitorRejected = "monitor_rejected" - CodeNoConsolePassword = "no_console_password" - CodeShareInvalid = "share_invalid" - CodeNoXattr = "no_xattr" + CodeQemuMissing Code = "qemu_missing" + CodeKVMUnusable Code = "kvm_unusable" + CodeQemuStartFailed Code = "qemu_start_failed" + CodeMonitorUnreachable Code = "monitor_unreachable" + CodeMonitorRejected Code = "monitor_rejected" + CodeNoConsolePassword Code = "no_console_password" + CodeShareInvalid Code = "share_invalid" + CodeNoXattr Code = "no_xattr" - CodeDownloadFailed = "download_failed" - CodeDownloadStalled = "download_stalled" - CodeChecksumMismatch = "checksum_mismatch" - CodeNoSuchImage = "no_such_image" + CodeDownloadFailed Code = "download_failed" + CodeDownloadStalled Code = "download_stalled" + CodeChecksumMismatch Code = "checksum_mismatch" + CodeNoSuchImage Code = "no_such_image" ) -// Code is a stable, machine-readable error code. Codes are only ever ADDED: -// never renamed, never repurposed, never removed (§2's compatibility -// promise). A consumer MUST treat an unrecognized code as a generic failure. -type Code string - // Codes returns every declared code, sorted. Built from the same string // constants codeTable and ErrorInfo.Code use, so a code added to one and // forgotten here fails TestCodesCoversTheTable. @@ -91,7 +91,7 @@ var ErrConfirmationRequired = errors.New("confirmation required; pass -y under - // ErrAlreadyRunning directly, never ErrDiskShrink, so there is nothing to // arbitrate there. Rows keep the design doc's order in case that changes. var codeTable = []struct { - code string + code Code err error }{ {CodeNotFound, core.ErrNotFound}, @@ -118,6 +118,7 @@ var codeTable = []struct { {CodeNoConsolePassword, qemu.ErrNoConsolePassword}, {CodeShareInvalid, qemu.ErrShareInvalid}, {CodeNoXattr, qemu.ErrNoXattr}, + {CodeAlreadyRunning, qemu.ErrAlreadyRunning}, {CodeDownloadFailed, iso.ErrDownloadFailed}, {CodeDownloadStalled, iso.ErrDownloadStalled}, {CodeChecksumMismatch, iso.ErrChecksumMismatch}, diff --git a/internal/cli/wire/errors_test.go b/internal/cli/wire/errors_test.go index b48d2295..77367327 100644 --- a/internal/cli/wire/errors_test.go +++ b/internal/cli/wire/errors_test.go @@ -16,7 +16,7 @@ import ( func TestMapErrorEveryCoreSentinel(t *testing.T) { tests := []struct { err error - code string + code Code }{ {fmt.Errorf("%w: work", core.ErrNotFound), CodeNotFound}, {fmt.Errorf("%w: work: parse fail", core.ErrBroken), CodeBroken}, @@ -36,7 +36,7 @@ func TestMapErrorEveryCoreSentinel(t *testing.T) { {ErrConfirmationRequired, CodeConfirmationRequired}, } for _, tt := range tests { - t.Run(tt.code, func(t *testing.T) { + t.Run(string(tt.code), func(t *testing.T) { got := MapError(tt.err) if got == nil { t.Fatal("MapError returned nil for a non-nil error") @@ -147,7 +147,7 @@ func TestCodesAreSnakeCaseUniqueAndSorted(t *testing.T) { // reaches a consumer as "internal" with prose, which is the shape this whole // table exists to remove. func TestEveryQemuSentinelHasOneRow(t *testing.T) { - want := map[error]string{ + want := map[error]Code{ qemu.ErrBinaryMissing: CodeQemuMissing, qemu.ErrKVMUnusable: CodeKVMUnusable, qemu.ErrStartFailed: CodeQemuStartFailed, @@ -156,6 +156,9 @@ func TestEveryQemuSentinelHasOneRow(t *testing.T) { qemu.ErrNoConsolePassword: CodeNoConsolePassword, qemu.ErrShareInvalid: CodeShareInvalid, qemu.ErrNoXattr: CodeNoXattr, + // No new code: core.ErrAlreadyRunning already names this condition, + // and qemu cannot import core to reuse the sentinel itself. + qemu.ErrAlreadyRunning: CodeAlreadyRunning, } for sentinel, code := range want { got := MapError(fmt.Errorf("%w: subject", sentinel)) @@ -167,7 +170,7 @@ func TestEveryQemuSentinelHasOneRow(t *testing.T) { // Each iso sentinel has exactly one row in codeTable. func TestEveryISOSentinelHasOneRow(t *testing.T) { - want := map[error]string{ + want := map[error]Code{ iso.ErrDownloadFailed: CodeDownloadFailed, iso.ErrDownloadStalled: CodeDownloadStalled, iso.ErrChecksumMismatch: CodeChecksumMismatch, diff --git a/internal/qemu/errors.go b/internal/qemu/errors.go index caa7ef1c..296d52b0 100644 --- a/internal/qemu/errors.go +++ b/internal/qemu/errors.go @@ -7,6 +7,10 @@ import "errors" // errors.Is reaches the sentinel through core's own wrapping. // // ErrNoXattr lives in xattr.go, beside the probe that raises it. +// +// ErrAlreadyRunning duplicates core.ErrAlreadyRunning by necessity: core +// imports qemu, so qemu cannot name core's sentinel. wire maps both to +// already_running. var ( ErrBinaryMissing = errors.New("qemu binary not found") ErrKVMUnusable = errors.New("/dev/kvm not usable") @@ -17,4 +21,5 @@ var ( ErrMonitorRejected = errors.New("qemu monitor rejected the command") ErrNoConsolePassword = errors.New("no console password set") ErrShareInvalid = errors.New("share is not a directory") + ErrAlreadyRunning = errors.New("already running") ) diff --git a/internal/qemu/run.go b/internal/qemu/run.go index d27961dd..9802d095 100644 --- a/internal/qemu/run.go +++ b/internal/qemu/run.go @@ -104,7 +104,7 @@ func diskWritten(v *config.VM) bool { // nothing and tracks the process by pidfile. func Start(v *config.VM) error { if Running(v) { - return fmt.Errorf("%s is already running", v.Name) + return fmt.Errorf("%w: %s is already running", ErrAlreadyRunning, v.Name) } _ = os.Remove(v.MonitorPath()) // The interactive install happens inside the guest, where stoat can't From 6b58fdc88a1cc0908b0e7346795feb0572815f45 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:20:59 +0300 Subject: [PATCH 10/20] test(qemu,core,cli): pin screenshot contracts Signed-off-by: NovusEdge --- internal/cli/cli.go | 5 +++++ internal/cli/grammar.go | 17 +++++++++++++---- internal/cli/json_test.go | 3 +++ internal/cli/run_screenshot.go | 21 +++++++++++++++++++++ internal/cli/wire/dto.go | 15 +++++++++++++++ internal/core/screenshot.go | 18 ++++++++++++++++++ internal/core/screenshot_test.go | 20 ++++++++++++++++++++ internal/qemu/errors.go | 4 ++++ internal/qemu/screenshot.go | 11 +++++++++++ internal/qemu/screenshot_test.go | 20 ++++++++++++++++++++ 10 files changed, 130 insertions(+), 4 deletions(-) create mode 100644 internal/cli/run_screenshot.go create mode 100644 internal/core/screenshot.go create mode 100644 internal/core/screenshot_test.go create mode 100644 internal/qemu/screenshot.go create mode 100644 internal/qemu/screenshot_test.go diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 7e2a1334..d1d6748d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -49,6 +49,9 @@ type Args struct { Yes bool N int // logs -n + // Out belongs to "screenshot": the -o path, empty for the default. + Out string + // NoApply belongs to "up": it skips the automatic post-boot apply, // leaving `up` returning as soon as the VM starts, as it did before that // behavior existed. @@ -404,6 +407,8 @@ func Main(args []string, version string, stdin io.Reader, stdout, stderr io.Writ return runGuest(a, stdout, stderr) case "logs": return runLogs(a, stdout, stderr) + case "screenshot": + return runScreenshot(a, stdout, stderr) case "get": return runGet(a, stdout, stderr) case "ssh-command": diff --git a/internal/cli/grammar.go b/internal/cli/grammar.go index 8e550384..59a9db5c 100644 --- a/internal/cli/grammar.go +++ b/internal/cli/grammar.go @@ -57,10 +57,11 @@ type grammar struct { Recipe recipeCmd `cmd:"" help:"author recipes"` Guest recipeGuestCmd `cmd:"" help:"list or show guest OS definitions"` - Logs logsCmd `cmd:"" help:"tail a VM's log, or stoat's own"` - Doctor doctorCmd `cmd:"" help:"check host prerequisites"` - Version versionCmd `cmd:"" help:"print the stoat version"` - Help helpCmd `cmd:"" help:"show this message"` + Logs logsCmd `cmd:"" help:"tail a VM's log, or stoat's own"` + Screenshot screenshotCmd `cmd:"" help:"write the VM's screen to a PNG"` + Doctor doctorCmd `cmd:"" help:"check host prerequisites"` + Version versionCmd `cmd:"" help:"print the stoat version"` + Help helpCmd `cmd:"" help:"show this message"` } type helpCmd struct{} @@ -244,6 +245,11 @@ type logsCmd struct { Which string `enum:"console,apply" default:"console" help:"which log, with a vm name"` } +type screenshotCmd struct { + VM string `arg:"" help:"vm name"` + Out string `short:"o" help:"where to write the png; default /screenshots/.png"` +} + // toArgs flattens the selected command into the Args every runX consumes. // Args stays the interface between parsing and running rather than // threading the kong structs through 18 run functions directly. @@ -425,6 +431,9 @@ func (g *grammar) toArgs(path string) (*Args, error) { l := g.Logs a.VM, a.N, a.Which = l.VM, l.N, core.Which(l.Which) + case "screenshot": + a.VM, a.Out = g.Screenshot.VM, g.Screenshot.Out + default: return nil, usageError("unknown subcommand " + path) } diff --git a/internal/cli/json_test.go b/internal/cli/json_test.go index 594e32b6..ef5f7986 100644 --- a/internal/cli/json_test.go +++ b/internal/cli/json_test.go @@ -85,6 +85,9 @@ func TestJSONEnvelopeEveryCommand(t *testing.T) { {name: "doctor", argv: []string{"doctor"}, ok: true, exit: ExitOK}, // The plan is computed host-side, so a stopped fixture VM answers it. {name: "apply dry-run", argv: []string{"apply", "work", "--dry-run"}, ok: true, exit: ExitOK}, + // The fixture VM is stopped, so the honest answer is not_running; the + // point here is that the command answers in the envelope at all. + {name: "screenshot stopped", argv: []string{"screenshot", "work"}, code: wire.CodeNotRunning, exit: ExitFail}, {name: "up unknown", argv: []string{"up", "nope"}, code: wire.CodeNotFound, exit: ExitFail}, {name: "down stopped", argv: []string{"down", "work"}, code: wire.CodeNotRunning, exit: ExitFail}, diff --git a/internal/cli/run_screenshot.go b/internal/cli/run_screenshot.go new file mode 100644 index 00000000..e87de872 --- /dev/null +++ b/internal/cli/run_screenshot.go @@ -0,0 +1,21 @@ +package cli + +import ( + "fmt" + "io" + + "github.com/novusedge/stoat/internal/cli/wire" + "github.com/novusedge/stoat/internal/core" +) + +func runScreenshot(a *Args, stdout, stderr io.Writer) int { + shot, err := core.Screenshot(a.VM, a.Out) + if err != nil { + return a.fail(stdout, stderr, err) + } + if a.JSON { + return a.ok(stdout, wire.FromShot(a.VM, shot)) + } + fmt.Fprintf(stdout, "%s (%dx%d, %d bytes)\n", shot.Path, shot.Width, shot.Height, shot.Bytes) + return ExitOK +} diff --git a/internal/cli/wire/dto.go b/internal/cli/wire/dto.go index 71e85601..b37a57fc 100644 --- a/internal/cli/wire/dto.go +++ b/internal/cli/wire/dto.go @@ -422,6 +422,21 @@ func FromGuests(os []guest.OS) []Guest { return nonNil(out) } +// Screenshot is `screenshot` data. Path is the host path qemu wrote, always +// absolute, so a caller that passed a relative -o can read back what +// actually happened. +type Screenshot struct { + VM string `json:"vm"` + Path string `json:"path"` + Bytes int64 `json:"bytes"` + Width int `json:"width"` + Height int `json:"height"` +} + +func FromShot(vm string, s core.Shot) Screenshot { + return Screenshot{VM: vm, Path: s.Path, Bytes: s.Bytes, Width: s.Width, Height: s.Height} +} + // GuestList is `guest ls` data. type GuestList struct { Guests []Guest `json:"guests"` diff --git a/internal/core/screenshot.go b/internal/core/screenshot.go new file mode 100644 index 00000000..7c157d4e --- /dev/null +++ b/internal/core/screenshot.go @@ -0,0 +1,18 @@ +package core + +// Shot is one screenshot: where it landed and what is in it. +type Shot struct { + Path string + Bytes int64 + Width int + Height int +} + +// Screenshot writes VM name's screen to dest, or to a default path under +// the VM's directory when dest is empty. +// +// Stub: TestScreenshotRefusesAStoppedVM pins the ErrNotRunning contract; +// task 6 fills in the qemu.Screenshot call and the default path. +func Screenshot(name, dest string) (Shot, error) { + return Shot{}, nil +} diff --git a/internal/core/screenshot_test.go b/internal/core/screenshot_test.go new file mode 100644 index 00000000..e77c380e --- /dev/null +++ b/internal/core/screenshot_test.go @@ -0,0 +1,20 @@ +package core + +import ( + "errors" + "testing" + + "github.com/novusedge/stoat/internal/config" +) + +// A stopped VM has no framebuffer, so Screenshot refuses up front with the +// same typed error Stop uses, rather than failing at the monitor socket. +func TestScreenshotRefusesAStoppedVM(t *testing.T) { + root(t) + if err := (&config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200}).Save(); err != nil { + t.Fatal(err) + } + if _, err := Screenshot("work", ""); !errors.Is(err, ErrNotRunning) { + t.Errorf("Screenshot() = %v, want ErrNotRunning", err) + } +} diff --git a/internal/qemu/errors.go b/internal/qemu/errors.go index 296d52b0..fc85c07c 100644 --- a/internal/qemu/errors.go +++ b/internal/qemu/errors.go @@ -22,4 +22,8 @@ var ( ErrNoConsolePassword = errors.New("no console password set") ErrShareInvalid = errors.New("share is not a directory") ErrAlreadyRunning = errors.New("already running") + ErrScreenshotFailed = errors.New("screenshot failed") + // ErrNotRunning duplicates core.ErrNotRunning: core imports qemu, so + // qemu cannot name core's sentinel. wire maps both to not_running. + ErrNotRunning = errors.New("not running") ) diff --git a/internal/qemu/screenshot.go b/internal/qemu/screenshot.go new file mode 100644 index 00000000..193a0c07 --- /dev/null +++ b/internal/qemu/screenshot.go @@ -0,0 +1,11 @@ +package qemu + +import "github.com/novusedge/stoat/internal/config" + +// Screenshot writes v's framebuffer to path as a PNG. +// +// Stub: TestScreenshotRefusesAStoppedVM pins the ErrNotRunning contract; +// task 5 fills in the QMP screendump call. +func Screenshot(v *config.VM, path string) error { + return nil +} diff --git a/internal/qemu/screenshot_test.go b/internal/qemu/screenshot_test.go new file mode 100644 index 00000000..2ab9e969 --- /dev/null +++ b/internal/qemu/screenshot_test.go @@ -0,0 +1,20 @@ +package qemu + +import ( + "errors" + "path/filepath" + "testing" + + "github.com/novusedge/stoat/internal/config" +) + +// A stopped VM has no framebuffer, and its dead monitor socket would +// otherwise report the transport rather than the reason. +func TestScreenshotRefusesAStoppedVM(t *testing.T) { + dir := t.TempDir() + v := &config.VM{Name: "work", Dir: dir} + err := Screenshot(v, filepath.Join(dir, "shot.png")) + if !errors.Is(err, ErrNotRunning) { + t.Errorf("Screenshot() = %v, want ErrNotRunning", err) + } +} From f1a96fcbd575c4453e66745e32fd614844b492c1 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:23:12 +0300 Subject: [PATCH 11/20] feat(qemu): screenshot over QMP screendump Signed-off-by: NovusEdge --- internal/cli/wire/errors.go | 5 +++++ internal/qemu/screenshot.go | 33 +++++++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index 538626f6..d3853ece 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -53,6 +53,8 @@ const ( CodeDownloadStalled Code = "download_stalled" CodeChecksumMismatch Code = "checksum_mismatch" CodeNoSuchImage Code = "no_such_image" + + CodeScreenshotFailed Code = "screenshot_failed" ) // Codes returns every declared code, sorted. Built from the same string @@ -70,6 +72,7 @@ func Codes() []Code { CodeShareInvalid, CodeNoXattr, CodeDownloadFailed, CodeDownloadStalled, CodeChecksumMismatch, CodeNoSuchImage, + CodeScreenshotFailed, } slices.Sort(out) return out @@ -123,6 +126,8 @@ var codeTable = []struct { {CodeDownloadStalled, iso.ErrDownloadStalled}, {CodeChecksumMismatch, iso.ErrChecksumMismatch}, {CodeNoSuchImage, iso.ErrNoSuchImage}, + {CodeScreenshotFailed, qemu.ErrScreenshotFailed}, + {CodeNotRunning, qemu.ErrNotRunning}, } // MapError converts a core (or context) error into an ErrorInfo, walking diff --git a/internal/qemu/screenshot.go b/internal/qemu/screenshot.go index 193a0c07..a902dec5 100644 --- a/internal/qemu/screenshot.go +++ b/internal/qemu/screenshot.go @@ -1,11 +1,36 @@ package qemu -import "github.com/novusedge/stoat/internal/config" +import ( + "fmt" + "path/filepath" -// Screenshot writes v's framebuffer to path as a PNG. + "github.com/novusedge/stoat/internal/config" +) + +// Screenshot writes the guest's framebuffer to path as a PNG. +// +// QMP screendump takes format: "png" from QEMU 7.1 on, verified by hand +// against QEMU 11.1, so nothing here converts a PPM afterwards. The +// framebuffer exists whatever the display backend is, so this answers for a +// GTK window and for a VNC socket alike. // -// Stub: TestScreenshotRefusesAStoppedVM pins the ErrNotRunning contract; -// task 5 fills in the QMP screendump call. +// qemu writes the file itself, as its own user, so path must be absolute: +// a relative one resolves against qemu's working directory, not the +// caller's. func Screenshot(v *config.VM, path string) error { + if !Running(v) { + return fmt.Errorf("%w: %s is not running", ErrNotRunning, v.Name) + } + if !filepath.IsAbs(path) { + return fmt.Errorf("%w: screenshot path %s is not absolute", ErrScreenshotFailed, path) + } + q, err := dialQMP(v) + if err != nil { + return err + } + defer func() { _ = q.Close() }() + if _, err := q.command("screendump", map[string]any{"filename": path, "format": "png"}); err != nil { + return fmt.Errorf("%w: %v", ErrScreenshotFailed, err) + } return nil } From 5c9656269d89c732052f64dd12b65640bf84c838 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:23:57 +0300 Subject: [PATCH 12/20] feat(core): screenshot with a default path Signed-off-by: NovusEdge --- internal/core/screenshot.go | 82 ++++++++++++++++++++++++++++++++++--- 1 file changed, 77 insertions(+), 5 deletions(-) diff --git a/internal/core/screenshot.go b/internal/core/screenshot.go index 7c157d4e..705cfc6f 100644 --- a/internal/core/screenshot.go +++ b/internal/core/screenshot.go @@ -1,5 +1,17 @@ package core +import ( + "fmt" + "image/png" + "os" + "path/filepath" + "strconv" + "strings" + "time" + + "github.com/novusedge/stoat/internal/qemu" +) + // Shot is one screenshot: where it landed and what is in it. type Shot struct { Path string @@ -8,11 +20,71 @@ type Shot struct { Height int } -// Screenshot writes VM name's screen to dest, or to a default path under -// the VM's directory when dest is empty. +// Screenshot writes VM name's screen to dest, or to +// /screenshots/.png when dest is +// empty. See shotPath. // -// Stub: TestScreenshotRefusesAStoppedVM pins the ErrNotRunning contract; -// task 6 fills in the qemu.Screenshot call and the default path. +// The running check is here rather than in qemu.Screenshot so a stopped VM +// answers ErrNotRunning, the same error Stop gives, instead of the +// monitor-socket error a caller cannot act on differently. func Screenshot(name, dest string) (Shot, error) { - return Shot{}, nil + v, err := load(name) + if err != nil { + return Shot{}, err + } + if !qemu.Running(v) { + return Shot{}, fmt.Errorf("%w: %s", ErrNotRunning, name) + } + + if dest == "" { + dir := filepath.Join(v.Dir, "screenshots") + if err := os.MkdirAll(dir, 0o755); err != nil { + return Shot{}, err + } + dest = shotPath(dir, time.Now().UTC()) + } + dest, err = filepath.Abs(dest) + if err != nil { + return Shot{}, err + } + if err := qemu.Screenshot(v, dest); err != nil { + return Shot{}, err + } + + fi, err := os.Stat(dest) + if err != nil { + return Shot{}, err + } + f, err := os.Open(dest) + if err != nil { + return Shot{}, err + } + defer func() { _ = f.Close() }() + // The header alone gives the size, so a 4K frame is never decoded into + // memory to answer two integers. + cfg, err := png.DecodeConfig(f) + if err != nil { + return Shot{}, err + } + return Shot{Path: dest, Bytes: fi.Size(), Width: cfg.Width, Height: cfg.Height}, nil +} + +// shotPath names a screenshot after the second it was taken: RFC3339 with +// the colons stripped, since a colon in a filename breaks scp's host:path +// split and Windows refuses it outright. +// +// A second shot inside the same second gets -2, then -3. A caller polling a +// boot takes frames faster than the name's resolution, and overwriting the +// frame that showed the stall is the failure this avoids. +func shotPath(dir string, at time.Time) string { + stamp := strings.ReplaceAll(at.Format(time.RFC3339), ":", "") + path := filepath.Join(dir, stamp+".png") + for n := 2; ; n++ { + // Any stat failure other than "gone" is answered the same way: the + // name is used, and qemu reports the write failure with the reason. + if _, err := os.Stat(path); err != nil { + return path + } + path = filepath.Join(dir, stamp+"-"+strconv.Itoa(n)+".png") + } } From ee7c1de78a3f703729d57d233fe129851d2befdb Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:26:01 +0300 Subject: [PATCH 13/20] docs(json): document screenshot command Signed-off-by: NovusEdge --- docs/reference/json.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/reference/json.md b/docs/reference/json.md index adc83808..6e68ff29 100644 --- a/docs/reference/json.md +++ b/docs/reference/json.md @@ -324,6 +324,7 @@ so a leak fails the build rather than shipping. | `guest show` | `{"guest":Guest}` | | `recipe list` | `{"dir":"...","recipes":["xfce"]}`, see note below | | `recipe new` | `{"path":"/home/u/.stoat/recipes/foo.alpine.sh"}` | +| `screenshot` | `{"vm":"work","path":"/home/u/.stoat/work/screenshots/2026-09-05T140302Z.png","bytes":48213,"width":1280,"height":800}` | Both `recipe` subcommands report `"cmd":"recipe"`, not `"cmd":"recipe list"`, and both `guest` subcommands report `"cmd":"guest"`. Distinguish them by which From d5ee52d2cbc3436f8cfc3c69da9814ffe31eaab6 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:38:00 +0300 Subject: [PATCH 14/20] test(apkovl): pin the three boot-defect fixes Signed-off-by: NovusEdge --- internal/apkovl/postinstall.go | 9 ++ internal/apkovl/postinstall_test.go | 132 ++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 internal/apkovl/postinstall.go create mode 100644 internal/apkovl/postinstall_test.go diff --git a/internal/apkovl/postinstall.go b/internal/apkovl/postinstall.go new file mode 100644 index 00000000..82bf6eca --- /dev/null +++ b/internal/apkovl/postinstall.go @@ -0,0 +1,9 @@ +package apkovl + +// extlinuxTimeoutFix, workMountFix and issueFix are stubs. Tasks 8, 9 and 10 +// each replace one body with the sh fragment that closes its issue. +const extlinuxTimeoutFix = `` + +const workMountFix = `` + +const issueFix = `` diff --git a/internal/apkovl/postinstall_test.go b/internal/apkovl/postinstall_test.go new file mode 100644 index 00000000..2dc407ab --- /dev/null +++ b/internal/apkovl/postinstall_test.go @@ -0,0 +1,132 @@ +package apkovl + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// runFix runs one post-install fragment against a fake target root, the way +// the installer runs it, and returns nothing: a failing shell fails the test. +func runFix(t *testing.T, target, fragment string) { + t.Helper() + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh not installed") + } + cmd := exec.Command("sh", "-c", "set -e\ntarget="+target+"\n"+fragment) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("fragment failed: %v\n%s", err, out) + } +} + +// syslinux cancels TIMEOUT when a key arrives during the countdown and then +// waits forever. TOTALTIMEOUT fires whatever the user typed. The install runs +// the append on a disk that may already carry the line, so it must not stack. +func TestExtlinuxTimeoutFixIsIdempotent(t *testing.T) { + target := t.TempDir() + if err := os.MkdirAll(filepath.Join(target, "boot"), 0o755); err != nil { + t.Fatal(err) + } + conf := filepath.Join(target, "boot", "extlinux.conf") + if err := os.WriteFile(conf, []byte("DEFAULT menu.c32\nPROMPT 0\nMENU HIDDEN\nTIMEOUT 10\n"), 0o644); err != nil { + t.Fatal(err) + } + + runFix(t, target, extlinuxTimeoutFix) + runFix(t, target, extlinuxTimeoutFix) + + b, err := os.ReadFile(conf) + if err != nil { + t.Fatal(err) + } + if n := strings.Count(string(b), "TOTALTIMEOUT"); n != 1 { + t.Errorf("TOTALTIMEOUT appears %d times, want 1:\n%s", n, b) + } +} + +// The installed fstab carries stoat's work share as "work /work 9p ... 0 2": +// setup-disk writes the target's fstab from the live system's mounts, and +// stoat mounts its shares under /mnt, the directory setup-disk mounts the +// target on. fsck then looks for fsck.9p and /work does not exist, so the +// guest reports a failed mount on every boot. +func TestWorkMountFixRepairsTheInstalledFstab(t *testing.T) { + target := t.TempDir() + if err := os.MkdirAll(filepath.Join(target, "etc"), 0o755); err != nil { + t.Fatal(err) + } + fstab := filepath.Join(target, "etc", "fstab") + body := "/dev/vda3 / ext4 rw,relatime 0 1\n" + + "work /work 9p trans=virtio,version=9p2000.L,rw,nofail 0 2\n" + if err := os.WriteFile(fstab, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + + runFix(t, target, workMountFix) + runFix(t, target, workMountFix) + + b, err := os.ReadFile(fstab) + if err != nil { + t.Fatal(err) + } + got := string(b) + if !strings.Contains(got, "work /mnt/work 9p") { + t.Errorf("fstab does not point work at /mnt/work:\n%s", got) + } + for _, line := range strings.Split(strings.TrimRight(got, "\n"), "\n") { + f := strings.Fields(line) + if len(f) >= 6 && f[2] == "9p" && f[5] != "0" { + t.Errorf("9p line has fsck pass %q, want 0: %s", f[5], line) + } + } + if !strings.Contains(got, "/dev/vda3 / ext4 rw,relatime 0 1") { + t.Errorf("the root line was rewritten:\n%s", got) + } + if _, err := os.Stat(filepath.Join(target, "mnt", "work")); err != nil { + t.Errorf("mountpoint was not created: %v", err) + } +} + +// setup-disk -m sys copies the live /etc onto the target, so the installer +// banner is still what the installed system's login screen shows. +func TestIssueFixRestoresTheStockBanner(t *testing.T) { + target := t.TempDir() + if err := os.MkdirAll(filepath.Join(target, "etc"), 0o755); err != nil { + t.Fatal(err) + } + issue := filepath.Join(target, "etc", "issue") + if err := os.WriteFile(issue, []byte(installIssue), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(target, "etc", "alpine-release"), []byte("3.22.1\n"), 0o644); err != nil { + t.Fatal(err) + } + + runFix(t, target, issueFix) + + b, err := os.ReadFile(issue) + if err != nil { + t.Fatal(err) + } + got := string(b) + if strings.Contains(got, "Installing Alpine") { + t.Errorf("the installer banner survived:\n%s", got) + } + if !strings.Contains(got, "Welcome to Alpine Linux 3.22") { + t.Errorf("issue = %q, want the stock banner", got) + } + + // A second run must leave a banner the user may have edited alone. + if err := os.WriteFile(issue, []byte("my own banner\n"), 0o644); err != nil { + t.Fatal(err) + } + runFix(t, target, issueFix) + b, err = os.ReadFile(issue) + if err != nil { + t.Fatal(err) + } + if string(b) != "my own banner\n" { + t.Errorf("issue = %q, want the user's own banner untouched", b) + } +} From 61e0b40d68d5364f422d23d35a09923afee73c76 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:42:39 +0300 Subject: [PATCH 15/20] fix(apkovl): stop the boot stall at the hidden menu Signed-off-by: NovusEdge --- internal/apkovl/apkovl.go | 2 +- internal/apkovl/postinstall.go | 53 ++++++++++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/internal/apkovl/apkovl.go b/internal/apkovl/apkovl.go index 15ade204..b526a195 100644 --- a/internal/apkovl/apkovl.go +++ b/internal/apkovl/apkovl.go @@ -77,7 +77,7 @@ echo "stoat: installing Alpine unattended, this takes a few minutes..." # here so the child inherits it and repartitions /dev/vda unattended. export ERASE_DISKS=/dev/vda if setup-alpine -e -f /etc/stoat/answerfile; then - echo "$(date -Iseconds)" > /mnt/work/.installed +` + postInstall + ` echo "$(date -Iseconds)" > /mnt/work/.installed echo "stoat: install complete, powering off; run 'stoat up' to boot the disk" poweroff else diff --git a/internal/apkovl/postinstall.go b/internal/apkovl/postinstall.go index 82bf6eca..469fc077 100644 --- a/internal/apkovl/postinstall.go +++ b/internal/apkovl/postinstall.go @@ -1,9 +1,56 @@ package apkovl -// extlinuxTimeoutFix, workMountFix and issueFix are stubs. Tasks 8, 9 and 10 -// each replace one body with the sh fragment that closes its issue. -const extlinuxTimeoutFix = `` +// postInstall runs on the installed disk after setup-alpine succeeds and +// before the installer powers off. It is sh, and it runs in the live +// installer environment, so every path it touches is under $target. +// +// setup-disk leaves nothing mounted for this to reuse, and the partition +// layout is whatever setup-disk chose, so mountTarget finds the root by +// looking for /etc/inittab rather than by assuming a partition number. The +// installer environment has no /etc/mtab worth reading either: the target +// was mounted and unmounted by a child process. +const postInstall = mountTarget + extlinuxTimeoutFix + workMountFix + issueFix + umountTarget +const mountTarget = `target=/mnt/target +mkdir -p "$target" +root= +for p in /dev/vda3 /dev/vda2 /dev/vda1; do + [ -b "$p" ] || continue + mount "$p" "$target" 2>/dev/null || continue + if [ -f "$target/etc/inittab" ]; then root=$p; break; fi + umount "$target" +done +if [ -z "$root" ]; then + echo "stoat: no installed root found on /dev/vda; skipping the post-install fixes" +else + # /boot is inside the root on a single-partition install and its own + # partition otherwise. Either way extlinux.conf must be writable. + [ -f "$target/boot/extlinux.conf" ] || mount /dev/vda1 "$target/boot" 2>/dev/null || true +` + +const umountTarget = ` umount "$target/boot" 2>/dev/null || true + umount "$target" +fi +` + +// extlinuxTimeoutFix appends TOTALTIMEOUT to the installed extlinux.conf +// (issue #59). The installed config carries DEFAULT menu.c32, PROMPT 0, +// MENU HIDDEN and TIMEOUT 10, one second. syslinux cancels TIMEOUT and draws +// the hidden menu when input arrives during that second, and then waits with +// no countdown left. TOTALTIMEOUT fires whatever the user typed. +// +// update-extlinux.conf has no key for TOTALTIMEOUT, so the line is appended +// to the generated file. A kernel upgrade that reruns update-extlinux drops +// it again. +const extlinuxTimeoutFix = ` conf="$target/boot/extlinux.conf" + if [ -f "$conf" ] && ! grep -q '^TOTALTIMEOUT ' "$conf"; then + echo 'TOTALTIMEOUT 100' >> "$conf" + echo "stoat: added TOTALTIMEOUT to extlinux.conf" + fi +` + +// workMountFix and issueFix are stubs. Tasks 9 and 10 each replace one body +// with the sh fragment that closes its issue. const workMountFix = `` const issueFix = `` From d36557c0b7536e83119d8a872c97556b81007997 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:43:07 +0300 Subject: [PATCH 16/20] fix(apkovl): repair the installed 9p fstab lines Signed-off-by: NovusEdge --- internal/apkovl/postinstall.go | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/internal/apkovl/postinstall.go b/internal/apkovl/postinstall.go index 469fc077..402b3117 100644 --- a/internal/apkovl/postinstall.go +++ b/internal/apkovl/postinstall.go @@ -49,8 +49,26 @@ const extlinuxTimeoutFix = ` conf="$target/boot/extlinux.conf" fi ` -// workMountFix and issueFix are stubs. Tasks 9 and 10 each replace one body -// with the sh fragment that closes its issue. -const workMountFix = `` +// workMountFix repairs the installed fstab's 9p lines (issue #60). +// +// setup-disk writes the target's fstab from the live system's mounts, and +// stoat's shares sit under /mnt, the same directory setup-disk mounts the +// target on, so the work share lands as "work /work 9p ... 0 2". busybox +// fsck then has no fsck.9p helper, and /work does not exist on the target, +// so the guest prints a failed mount on every boot. +// +// awk rewrites only lines whose type field is 9p; every other line prints +// unchanged, comments included. +const workMountFix = ` fstab="$target/etc/fstab" + if [ -f "$fstab" ]; then + awk '$3 == "9p" { $2 = "/mnt/" $1; $5 = "0"; $6 = "0" } { print }' "$fstab" > "$fstab.stoat" && + mv "$fstab.stoat" "$fstab" + awk '$3 == "9p" { print $2 }' "$fstab" | while read -r d; do + mkdir -p "$target$d" + done + fi +` +// issueFix is a stub. Task 10 replaces its body with the sh fragment that +// closes issue #61. const issueFix = `` From f25c6e7d0435ee9a88c0e273458ed62331d43595 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:45:50 +0300 Subject: [PATCH 17/20] fix(apkovl): restore the stock issue on the target Signed-off-by: NovusEdge --- docs/design/core-api.md | 28 ++++++++++++++++++++++++---- docs/reference/json.md | 14 ++++++++++++++ internal/apkovl/postinstall.go | 19 ++++++++++++++++--- 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/docs/design/core-api.md b/docs/design/core-api.md index 03acf9a3..4837e705 100644 --- a/docs/design/core-api.md +++ b/docs/design/core-api.md @@ -230,14 +230,34 @@ All of these are wanted. Ordered by effort, so the cheap wins land early and the Typed errors, because every caller branches on them and string matching is how that goes wrong: ``` -ErrNotFound ErrNameTaken ErrImmutableField -ErrImageNotDownloaded ErrRecipeNotApplicable ErrNotRunning -ErrAlreadyRunning ErrTimeout ErrDependencyMissing -ErrBroken ErrDiskShrink +core: ErrNotFound ErrNameTaken ErrImmutableField + ErrImageNotDownloaded ErrRecipeNotApplicable ErrNotRunning + ErrAlreadyRunning ErrTimeout ErrDependencyMissing + ErrBroken ErrDiskShrink ErrCannotReach + ErrNoDisk ErrUnknownWhich + +qemu: ErrBinaryMissing ErrKVMUnusable ErrStartFailed + ErrMonitorUnreachable ErrMonitorRejected ErrNoConsolePassword + ErrShareInvalid ErrNoXattr ErrScreenshotFailed + ErrNotRunning ErrAlreadyRunning + +iso: ErrDownloadFailed ErrDownloadStalled ErrChecksumMismatch + ErrNoSuchImage ``` Each carries the specific subject (which field, which recipe, which dependency) rather than only a message. +`qemu` and `iso` declare their own sentinels beside the code that raises +them. `internal/cli/wire` maps each to a code in `codeTable`; a sentinel with +no row reaches a consumer as `internal` with prose. + +`qemu.ErrNotRunning` and `qemu.ErrAlreadyRunning` duplicate `core`'s two by +necessity: `core` imports `qemu`, so `qemu` cannot name them. They add no +code, and `codeTable` maps each pair to one. + +Wrapping a sentinel prefixes its text to the message: `Preflight` now returns +`qemu binary not found: qemu-system-x86_64 not found in PATH`. + The CLI reports these two ways, and the choice is not a style question. `a.fail` takes an error `core` returned and maps its sentinel to the JSON error code. `a.failMsg` takes a sentinel the CLI picked itself, for a condition `core` never saw: a bad flag combination, an diff --git a/docs/reference/json.md b/docs/reference/json.md index 6e68ff29..3cf0d8b1 100644 --- a/docs/reference/json.md +++ b/docs/reference/json.md @@ -114,6 +114,19 @@ bump the contract version. Do not write code that requires them. | `cannot_reach` | `wait` was asked for a state this VM can never reach | | `applied_at_boot` | reserved; no command emits it. A cloud VM whose recipes ran at boot answers `ok:true` with `applied: []` | | `unknown_log` | bad `--which` | +| `qemu_missing` | `qemu-system-x86_64` is not on `PATH` | +| `kvm_unusable` | `/dev/kvm` cannot be opened; the user is usually not in the `kvm` group | +| `qemu_start_failed` | qemu ran and refused to start the VM | +| `monitor_unreachable` | the VM's qemu monitor socket does not answer | +| `monitor_rejected` | qemu answered the monitor command with an error | +| `no_console_password` | the VM has no console password to type | +| `share_invalid` | the configured share is not a directory | +| `no_xattr` | the share's filesystem cannot store `user.*` extended attributes | +| `screenshot_failed` | qemu refused the screendump | +| `download_failed` | a mirror or a checksum file did not answer with the image | +| `download_stalled` | the download stopped producing bytes | +| `checksum_mismatch` | the downloaded bytes do not match the published digest | +| `no_such_image` | the requested image is not in the index | | `timeout` | the deadline expired | | `canceled` | the context was cancelled | | `usage` | a bad flag, a missing argument, an unknown subcommand | @@ -123,6 +136,7 @@ bump the contract version. Do not write code that requires them. **Codes are only ever added.** Never renamed, never repurposed, never removed. A consumer MUST treat an unrecognized code as a generic failure rather than crashing on it, because that is what makes adding one a non-breaking change. +The list above is `wire.Codes()`, which returns every declared code, sorted. ## Exit codes diff --git a/internal/apkovl/postinstall.go b/internal/apkovl/postinstall.go index 402b3117..d722cd42 100644 --- a/internal/apkovl/postinstall.go +++ b/internal/apkovl/postinstall.go @@ -69,6 +69,19 @@ const workMountFix = ` fstab="$target/etc/fstab" fi ` -// issueFix is a stub. Task 10 replaces its body with the sh fragment that -// closes issue #61. -const issueFix = `` +// issueFix restores the stock /etc/issue on the target (issue #61). +// +// setup-disk -m sys copies the live system's /etc, so the installed login +// screen still reads "Installing Alpine. Unattended. Do not log in." The +// guard on the banner text means a rerun never overwrites an issue the user +// has since edited. +// +// printf's format is single-quoted, so \r, \m and \l reach the file as the +// getty escapes \r, \m and \l rather than as control characters. +const issueFix = ` issue="$target/etc/issue" + if [ -f "$issue" ] && grep -q 'Installing Alpine' "$issue"; then + printf 'Welcome to Alpine Linux %s\nKernel \\r on an \\m (\\l)\n\n' \ + "$(cut -d. -f1,2 "$target/etc/alpine-release")" > "$issue" + echo "stoat: restored the stock /etc/issue" + fi +` From 072502a265de6e3f4f34f9cb9921ea8e78abc908 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:58:08 +0300 Subject: [PATCH 18/20] test(core): pin the screenshot name collision suffix The default name resolves to the second, and a caller polling a boot takes several frames inside one. shotPath's -2/-3 fallback shipped unpinned. Signed-off-by: NovusEdge --- internal/core/screenshot_test.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/internal/core/screenshot_test.go b/internal/core/screenshot_test.go index e77c380e..7826936f 100644 --- a/internal/core/screenshot_test.go +++ b/internal/core/screenshot_test.go @@ -2,7 +2,10 @@ package core import ( "errors" + "os" + "path/filepath" "testing" + "time" "github.com/novusedge/stoat/internal/config" ) @@ -18,3 +21,27 @@ func TestScreenshotRefusesAStoppedVM(t *testing.T) { t.Errorf("Screenshot() = %v, want ErrNotRunning", err) } } + +// Polling a boot is the use this command exists for, and such a caller takes +// several frames a second. The name resolves to the second, so two frames in +// one second must not become one file. shotPath takes the time as a parameter +// because the collision cannot be reached through Screenshot without a +// running VM and a clock the test controls. +func TestScreenshotNamesDoNotCollideWithinASecond(t *testing.T) { + dir := t.TempDir() + at := time.Date(2026, 9, 5, 14, 3, 2, 0, time.UTC) + + for _, want := range []string{ + "2026-09-05T140302Z.png", + "2026-09-05T140302Z-2.png", + "2026-09-05T140302Z-3.png", + } { + got := shotPath(dir, at) + if filepath.Base(got) != want { + t.Fatalf("shotPath() = %q, want %q", filepath.Base(got), want) + } + if err := os.WriteFile(got, nil, 0o644); err != nil { + t.Fatal(err) + } + } +} From 514b8d41e3eb75e510070e1ac4cb58834a5937a4 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:58:40 +0300 Subject: [PATCH 19/20] test(cli): assert every error code is one of Codes() The envelope test compared the code against the row's expectation only, so a code the contract never publishes would have passed. Also drop a Code conversion that predates the typed field. Signed-off-by: NovusEdge --- internal/cli/json_test.go | 9 ++++++++- internal/cli/wire/errors_test.go | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/internal/cli/json_test.go b/internal/cli/json_test.go index ef5f7986..ddfed2a0 100644 --- a/internal/cli/json_test.go +++ b/internal/cli/json_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "encoding/json" + "slices" "strings" "testing" @@ -142,9 +143,15 @@ func TestJSONEnvelopeEveryCommand(t *testing.T) { if errObj == nil { t.Fatalf("failed result has no error object: %v", res) } - if errObj["code"] != string(tt.code) { + got, _ := errObj["code"].(string) + if got != string(tt.code) { t.Errorf("error.code = %v, want %q", errObj["code"], tt.code) } + // A code absent from Codes() is one no consumer can generate a + // switch for, whatever the message beside it says. + if !slices.Contains(wire.Codes(), wire.Code(got)) { + t.Errorf("error.code = %q, which wire.Codes() does not declare", got) + } if msg, _ := errObj["message"].(string); msg == "" { t.Errorf("error.message is empty: %v", errObj) } diff --git a/internal/cli/wire/errors_test.go b/internal/cli/wire/errors_test.go index 77367327..3dd6e6d0 100644 --- a/internal/cli/wire/errors_test.go +++ b/internal/cli/wire/errors_test.go @@ -112,7 +112,7 @@ func TestCodesCoversTheTable(t *testing.T) { declared[c] = true } for _, row := range codeTable { - if !declared[Code(row.code)] { + if !declared[row.code] { t.Errorf("codeTable has %q, Codes() does not", row.code) } } From 47c42bad8cd35d2afbf095ef8526aeb6e7ffd655 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sat, 5 Sep 2026 03:59:00 +0300 Subject: [PATCH 20/20] docs(cli): document stoat screenshot The command reference table and its per-command sections list every subcommand; screenshot was added to the grammar without an entry. Signed-off-by: NovusEdge --- docs/reference/cli.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 5281d7d5..2e6db57a 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -45,6 +45,7 @@ usage: stoat [flags] | [`guest ls`](#stoat-guest-ls) | List loaded guest OS definitions | 0 | | [`guest show`](#stoat-guest-show-name) | Print one guest's merged definition | 0, 1 | | [`logs`](#stoat-logs-name--n-n) | Tail a VM's log, or stoat's own | 0, 1 | +| [`screenshot`](#stoat-screenshot-name--o-path) | Write the VM's screen to a PNG | 0, 1 | | [`doctor`](#stoat-doctor) | Check host prerequisites | 0, 1 | | [`version`](#stoat-version) | Print the stoat version | 0 | | [`help`](#stoat-help) | Show the usage message | 0 | @@ -540,6 +541,21 @@ $ stoat logs -n 20 **Exit codes:** 0 on success (including an empty log, which prints nothing); 1 if the log can't be opened or read. +## `stoat screenshot [-o path]` + +Writes the VM's screen to a PNG and prints the path, the pixel size and the byte count. qemu dumps its own framebuffer over the monitor socket, so the image is the same whether the display is a GTK window or a VNC socket, and a VM stuck at a boot prompt still answers. + +``` +$ stoat screenshot work +/home/u/.stoat/work/screenshots/2026-09-05T140302Z.png (1280x800, 48213 bytes) +``` + +Without `-o`, the file lands in `/screenshots/`, named for the second it was taken: RFC3339 with the colons stripped, since a colon in a filename breaks scp's `host:path` split. A second shot inside the same second gets `-2`, then `-3`, so a caller polling a boot never overwrites the frame it just took. + +`-o` names the file instead. qemu writes it as its own user, so a relative path resolves against the caller's working directory before qemu sees it. + +**Exit codes:** 0 on success; 1 if the VM does not exist, is not running (`not_running`), or qemu refuses the dump (`screenshot_failed`). + ## `stoat doctor` Checks host prerequisites: qemu/KVM, `qemu-img`, `ssh`, `xorriso` and `/dev/kvm`, the same set the installer's own checklist runs, so `stoat doctor` and `just setup` can't disagree about whether the host is ready.