feat(mcp): job_wait, write_file as user, leaner outputs

CHANGELOG.md

@@ -1,5 +1,43 @@
 # Changelog
 
+## Unreleased
+
+### Changed
+
+- **MCP `write_file` now writes as the guest ssh user, not as root.** A file
+  under the ssh user's home now belongs to that user. A path only root can
+  write fails with `access_denied`. Pass `as_root=true` to write as root; it
+  needs `agent_access = exec`, and `manage` is refused with `access_denied`.
+- MCP `job_status` omits `exit_code` until the job has exited. It was `0`
+  while the job ran.
+- MCP `job_kill` returns `{job_id, state, exit_code, signaled}` instead of the
+  command result. A job that is not running is not signaled.
+- MCP `destroy` returns `{name, destroyed}`. `write_file` returns
+  `{path, bytes, mode}`. `wait` reports `healthy` only after `healthy=true`.
+- MCP `stat` and `list_dir` report `mode` in octal, such as `0644`, and add
+  `owner`. `list_dir` names entries by basename. `logs` removes terminal
+  escape sequences and carriage returns. `list_recipes` and the recipe index
+  tools return `has_health` in place of the health-check script.
+- MCP `update` works on a running VM and lists the fields that wait for the
+  next start in `pending_restart`. It refuses an unknown recipe name with
+  `not_found`.
+
+### Added
+
+- MCP `job_wait` blocks until a background job exits, and returns its state,
+  exit code, and the tail of stdout and stderr.
+- MCP `list_jobs` reports each job's state. MCP `write_file` takes `parents`.
+  `svc_status` reports `active` and `enabled`.
+
+### Fixes
+
+- MCP guest tools answer `not_found` for a missing path and `access_denied`
+  for a path the ssh user cannot use, where they answered `internal`. An
+  unknown job id answers `not_found` and a malformed one answers `usage`.
+- A VM's `health` is no longer `ok` while a configured recipe is unapplied.
+- The MCP `create` description states the `mode` values and the ram, cpu and
+  disk defaults.
+
 ## v0.6.2
 
 The recipe index gets its first entry, plus two fixes found by driving the

docs/reference/mcp.md

@@ -150,7 +150,7 @@ permissions below them:
 | `none` | No guest operation. Host-side status, lifecycle, snapshots, logs, forwarding, recipe management and project operations remain available. |
 | `observe` | `read_file`, `list_dir`, `stat`, `ps`, `svc_status`, and `tail_log`. |
 | `manage` | Observe operations plus `write_file`, `copy_to`, `copy_from`, `pkg_install`, `svc`, `useradd`, and `apply_recipes`. |
-| `exec` | Manage operations plus `exec`, `exec_bg`, `job_status`, `job_output`, `job_kill`, and `list_jobs`. |
+| `exec` | Manage operations plus `exec`, `exec_bg`, `job_status`, `job_wait`, `job_output`, `job_kill`, and `list_jobs`. |
 
 `create` defaults to `manage`. The CLI and TUI can raise or lower a VM's
 level. The MCP `update` tool can lower a level but refuses to raise it. The
@@ -176,13 +176,13 @@ unavailable fork and continuation proposals. Discovery does not mutate a VM.
 |---|---|
 | `list_vms`, `vm_status` | List VMs or inspect one VM, including recipe state and health. |
 | `list_images` | List catalog and downloaded images. |
-| `list_recipes`, `check_recipes`, `recipe_schema`, `search_recipes` | Inspect recipe availability, contract, or index entries. |
+| `list_recipes`, `check_recipes`, `recipe_schema`, `search_recipes` | Inspect recipe availability, contract, or index entries. `list_recipes` sets `has_health` for a recipe with a health check and leaves out the check script. `recipe_schema` returns the script. |
 | `plan_recipes` | Show what `apply_recipes` would run or skip without running it. |
 | `add_recipe`, `update_recipe`, `remove_recipe` | Add, repin, or remove remote recipes. `add_recipe` accepts curated index names only; it refuses Git URLs. `remove_recipe` has no force option. |
 | `list_guests`, `guest_info` | Inspect loaded guest definitions and their package/service commands. |
-| `doctor`, `logs` | Check host prerequisites or tail a VM's console/apply log. |
+| `doctor`, `logs` | Check host prerequisites or tail a VM's console/apply log. `logs` removes terminal escape sequences and carriage returns from the lines. |
 | `capabilities` | Read host checks and optional stored VM metadata; report current capabilities and limits. |
-| `create`, `start`, `stop`, `destroy`, `update`, `clone` | Manage VM definitions and lifecycle. `destroy` deletes the VM, its disk and its shared directory. |
+| `create`, `start`, `stop`, `destroy`, `update`, `clone` | Manage VM definitions and lifecycle. `destroy` deletes the VM, its disk and its shared directory, and returns `{name, destroyed}`. |
 | `snapshot`, `list_snapshots`, `restore`, `delete_snapshot`, `forward`, `wait`, `prune` | Manage disk-only snapshots, port forwards, state waits, and stale files. `snapshot` and `delete_snapshot` work on a running VM; `restore` needs a stopped VM. `prune` is dry-run unless `apply=true`. |
 | `project_status`, `project_up`, `project_down`, `project_apply`, `project_wait` | Inspect or operate on every VM declared by the server working directory's `stoat.toml`, in declaration order. A failure stops the run and later VMs are marked skipped. |
 
@@ -195,7 +195,8 @@ to start the VM with `stoat up -y`. Inputs with a fixed set of values
 
 `wait` accepts `reachable`, `applied`, or `stopped`; `healthy=true` waits for
 the applied recipes' health checks. Its `timeout_seconds` is a count of
-seconds, not a duration string. It defaults to 300 and is capped at 600.
+seconds, not a duration string. It defaults to 300 and is capped at 600. The
+result has `healthy` only after a `healthy=true` wait.
 
 Call `wait` after `start` or `create` and before any guest tool. sshd needs
 tens of seconds to come up, and a guest tool called earlier fails with
@@ -206,15 +207,33 @@ output collected so far with `timed_out: true` and a message that points at
 `exec_bg`. The guest kills the command through its own `timeout` when the guest
 has one.
 
+`create` fills `ram_mb`, `cpus`, and `disk` from the defaults in its tool
+description when you omit them. `mode` is `live` or `disk` for an image that
+offers both. A cloud image is always `cloud`, an installer image is always
+`disk`, and a `mode` that contradicts the image is refused.
+
+`update` works on a running VM. `ram_mb`, `cpus`, and `ssh_port` take effect
+at the next start, and the result lists the ones a running VM has not picked up
+in `pending_restart`. `update` refuses a recipe name that no recipe root
+declares, with the code `not_found`. A VM's `health` is not `ok` while a
+configured recipe is unapplied.
+
 ### Guest tools
 
 | Tool | Required level | Purpose |
 |---|---:|---|
 | `apply_recipes` | `manage` | Run the VM's configured recipe scripts, optionally with a named subset. |
-| `read_file`, `list_dir`, `stat`, `ps`, `svc_status`, `tail_log` | `observe` | Read guest files, directories, process state, service state, or logs. Guest paths are absolute. |
+| `read_file`, `list_dir`, `stat`, `ps`, `svc_status`, `tail_log` | `observe` | Read guest files, directories, process state, service state, or logs. Guest paths are absolute. `mode` is octal permission bits such as `0644`, and `list_dir` names entries by basename. `svc_status` adds `active` and `enabled`. |
 | `write_file`, `copy_to`, `copy_from`, `pkg_install`, `svc`, `useradd` | `manage` | Modify files, copy data under the VM's shared directory, install packages, manage services, or add a user. |
-| `exec`, `exec_bg`, `job_kill` | `exec` | Run or signal arbitrary guest commands. `argv` is an argument array, not a shell string. |
-| `job_status`, `job_output`, `list_jobs` | `exec` | Inspect background jobs created by `exec_bg`. A job reads `starting` until the guest records its pid, then `running`, then `exited`. A guest reboot clears the job files and a job becomes `unknown`. |
+| `exec`, `exec_bg`, `job_kill` | `exec` | Run or signal arbitrary guest commands. `argv` is an argument array, not a shell string. `job_kill` does not signal a job that is not running. It returns the job's state with `signaled` false. |
+| `job_status`, `job_wait`, `job_output`, `list_jobs` | `exec` | Inspect background jobs created by `exec_bg`. A job reads `starting` until the guest records its pid, then `running`, then `exited`, and `exit_code` appears only then. A guest reboot clears the job files and a job becomes `unknown`. `job_wait` blocks until the job exits or `timeout_seconds` passes, and returns the state with the tail of stdout and stderr. `list_jobs` includes each job's state. |
+
+`write_file` writes as the guest ssh user, so the file belongs to that user.
+`as_root=true` writes as root and needs the `exec` level. At `manage` it is
+refused with `access_denied`. `parents=true` creates missing parent
+directories as the same user. It returns `{path, bytes, mode}`. A missing
+path answers `not_found` and a path the user cannot write answers
+`access_denied`. Earlier versions always wrote as root.
 
 Guest operations require a running VM. `copy_to` and `copy_from` restrict the
 host side to the VM's configured shared directory. `pkg_install` uses the

internal/cli/wire/dto.go

@@ -812,6 +812,39 @@ type RecipeCatalog struct {
 	Recipes []Recipe `json:"recipes"`
 }
 
+// RecipeBrief is one recipe as the MCP listing tools return it. It leaves out
+// the health-check script, which is most of a Recipe's bytes; recipe_schema
+// returns the full contract for one recipe.
+type RecipeBrief struct {
+	Name        string         `json:"name"`
+	Description string         `json:"description"`
+	Schema      int            `json:"schema"`
+	Params      []RecipeParam  `json:"params"`
+	Outputs     []RecipeOutput `json:"outputs"`
+	HasHealth   bool           `json:"has_health"`
+	Reboot      bool           `json:"reboot"`
+	Depends     []string       `json:"depends"`
+	Runtime     string         `json:"runtime"`
+}
+
+// RecipeBriefList is the recipe listing tools' output.
+type RecipeBriefList struct {
+	Recipes []RecipeBrief `json:"recipes"`
+}
+
+func FromRecipeBriefs(rs []core.Recipe) RecipeBriefList {
+	out := make([]RecipeBrief, len(rs))
+	for i, r := range rs {
+		out[i] = RecipeBrief{
+			Name: r.Name, Description: r.Description, Schema: r.Schema,
+			Params: fromRecipeParams(r.Params), Outputs: fromRecipeOutputs(r.Outputs),
+			HasHealth: r.Health != nil, Reboot: r.Reboot,
+			Depends: nonNil(r.Depends), Runtime: r.Runtime,
+		}
+	}
+	return RecipeBriefList{Recipes: nonNil(out)}
+}
+
 // RecipeIssueList is the check_recipes tool's output.
 type RecipeIssueList struct {
 	Issues []RecipeIssue `json:"issues"`
@@ -871,10 +904,13 @@ type PruneList struct {
 }
 
 // WaitResult is the wait tool's output.
+//
+// Healthy is set only after an until=healthy wait: every other wait says
+// nothing about recipe health, and a false there read as a failed check.
 type WaitResult struct {
 	VM      string `json:"vm"`
 	Until   string `json:"until"`
-	Healthy bool   `json:"healthy"`
+	Healthy bool   `json:"healthy,omitempty"`
 }
 
 // ApplyResult is what an apply left behind, so a caller does not have to
@@ -898,13 +934,23 @@ type FileContent struct {
 
 // DirEntry is the list_dir and stat tools' per-file output.
 type DirEntry struct {
-	Name  string `json:"name"`
-	Type  string `json:"type"`
-	Size  int64  `json:"size"`
+	Name string `json:"name"`
+	Type string `json:"type"`
+	Size int64  `json:"size"`
+	// Mode is the permission bits in octal, such as "0644".
 	Mode  string `json:"mode"`
+	Owner string `json:"owner"`
 	MTime int64  `json:"mtime"`
 }
 
+// FileWritten is the write_file tool's output. Mode is empty for an append
+// that did not set one, which leaves the file's mode alone.
+type FileWritten struct {
+	Path  string `json:"path"`
+	Bytes int    `json:"bytes"`
+	Mode  string `json:"mode,omitempty"`
+}
+
 // DirListing is the list_dir tool's output.
 type DirListing struct {
 	Entries   []DirEntry `json:"entries"`
@@ -949,26 +995,69 @@ type CopyResult struct {
 	ToRemote bool   `json:"to_remote"`
 }
 
+// ServiceStatus is the svc_status tool's output: the init system's own status
+// output, plus what a caller would otherwise parse out of it. Enabled is nil
+// for a guest whose init system has no boot-enabled query here.
+type ServiceStatus struct {
+	CommandResult
+	Active  bool  `json:"active"`
+	Enabled *bool `json:"enabled,omitempty"`
+}
+
+// Destroyed is the destroy tool's output.
+type Destroyed struct {
+	Name      string `json:"name"`
+	Destroyed bool   `json:"destroyed"`
+}
+
+// VMUpdated is the update tool's output. PendingRestart names the fields that
+// were saved but that a running VM only picks up at its next start.
+type VMUpdated struct {
+	VM
+	PendingRestart []string `json:"pending_restart,omitempty"`
+}
+
 // JobStarted is the exec_bg tool's output.
 type JobStarted struct {
 	JobID string `json:"job_id"`
 	Dir   string `json:"dir"`
 }
 
-// JobStatus is the job_status tool's output.
+// JobStatus is the job_status tool's output. ExitCode is set only once the
+// job has exited; a running job has no exit code, and a zero there read as
+// success.
 type JobStatus struct {
 	JobID    string `json:"job_id"`
 	State    string `json:"state"`
-	ExitCode int    `json:"exit_code"`
+	ExitCode *int   `json:"exit_code,omitempty"`
+}
+
+// JobWait is the job_wait tool's output. TimedOut is true when the job was
+// still starting or running at the deadline. Stdout and Stderr are the tail
+// of each stream.
+type JobWait struct {
+	JobStatus
+	TimedOut bool   `json:"timed_out,omitempty"`
+	Stdout   string `json:"stdout"`
+	Stderr   string `json:"stderr"`
+}
+
+// JobKill is the job_kill tool's output. State is what the job was in when
+// the tool looked, and Signaled is false when nothing was left to signal.
+type JobKill struct {
+	JobStatus
+	Signaled bool `json:"signaled"`
 }
 
 // Job is one list_jobs row.
 type Job struct {
-	JobID   string    `json:"job_id"`
-	Argv    []string  `json:"argv"`
-	User    string    `json:"user"`
-	CWD     string    `json:"cwd,omitempty"`
-	Started time.Time `json:"started"`
+	JobID    string    `json:"job_id"`
+	State    string    `json:"state"`
+	ExitCode *int      `json:"exit_code,omitempty"`
+	Argv     []string  `json:"argv"`
+	User     string    `json:"user"`
+	CWD      string    `json:"cwd,omitempty"`
+	Started  time.Time `json:"started"`
 }
 
 // MCPInstall is the `mcp install` command's output.

internal/core/vm.go

@@ -400,6 +400,17 @@ func fromConfigChecked(v *config.VM) (VM, error) {
 		verdicts = append(verdicts, RecipeHealth{Name: state.Name, Status: status})
 	}
 	out.Health = VMHealth(verdicts)
+	// VMHealth lets one ok verdict outweigh unknown ones, which is right for
+	// a wait over applied recipes. A configured recipe that never applied,
+	// or that does not exist, has no verdict at all and must not read as ok.
+	if out.Health == HealthOK {
+		for _, state := range states {
+			if !state.Applied {
+				out.Health = HealthUnknown
+				break
+			}
+		}
+	}
 	return out, nil
 }
 

internal/core/vm_test.go

@@ -211,6 +211,27 @@ func TestGetUsesStoredRecipeHealthWithoutSSH(t *testing.T) {
 	}
 }
 
+// One applied ok recipe must not vouch for a configured recipe that never
+// applied or does not exist.
+func TestGetHealthIsNotOKWithAnUnappliedRecipe(t *testing.T) {
+	root(t)
+	v := &config.VM{
+		Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2201,
+		Recipes: []string{"docker", "no-such-recipe"},
+		Applied: map[string]config.AppliedRecipe{"docker": {Health: string(HealthOK)}},
+	}
+	if err := v.Save(); err != nil {
+		t.Fatal(err)
+	}
+	got, err := Get(v.Name)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got.Health != HealthUnknown {
+		t.Errorf("Health = %q, want %q", got.Health, HealthUnknown)
+	}
+}
+
 func TestGetUnknownVM(t *testing.T) {
 	root(t)
 	if _, err := Get("nope"); !errors.Is(err, ErrNotFound) {

internal/mcpsrv/access.go

@@ -15,7 +15,7 @@ import (
 //	observe read_file, list_dir, stat, ps, svc_status, tail_log
 //	manage  write_file, copy_to, copy_from, pkg_install, svc, useradd,
 //	        apply_recipes
-//	exec    exec, exec_bg, job_status, job_output, job_kill, list_jobs
+//	exec    exec, exec_bg, job_status, job_wait, job_output, job_kill, list_jobs
 type Level int
 
 const (

internal/mcpsrv/access_test.go

@@ -88,7 +88,7 @@ func accessArgsFor(tool, vm string) map[string]any {
 		args["name"] = "bob"
 	case "exec", "exec_bg":
 		args["argv"] = []string{"true"}
-	case "job_status", "job_output", "job_kill":
+	case "job_status", "job_wait", "job_output", "job_kill":
 		args["job_id"] = "j-00000000"
 	}
 	return args

internal/mcpsrv/jobs.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/BurntSushi/toml"
+	"github.com/novusedge/stoat/internal/cli/wire"
 	"github.com/novusedge/stoat/internal/config"
 	"github.com/novusedge/stoat/internal/tomlx"
 )
@@ -42,7 +43,7 @@ func newJobID() string {
 
 func checkJobID(id string) (string, error) {
 	if !jobIDRE.MatchString(id) {
-		return "", fmt.Errorf("invalid job id %q: must match %s", id, jobIDRE)
+		return "", wire.WithSentinel(fmt.Errorf("invalid job id %q: must match %s", id, jobIDRE), wire.ErrBadInput)
 	}
 	return id, nil
 }

internal/mcpsrv/redact_test.go

@@ -127,7 +127,7 @@ func argsFor(name string) map[string]any {
 		return map[string]any{"vm": "dev", "name": "sshd"}
 	case "exec", "exec_bg":
 		return map[string]any{"vm": "dev", "argv": []string{"true"}}
-	case "job_status", "job_output", "job_kill":
+	case "job_status", "job_wait", "job_output", "job_kill":
 		return map[string]any{"vm": "dev", "job_id": "j-00000001"}
 	}
 	if strings.Contains(name, "vm") || name == "logs" || name == "wait" ||

internal/mcpsrv/table_test.go

@@ -62,6 +62,7 @@ var toolTable = []toolSpec{
 	{"exec_bg", classExec, LevelExec},
 	{"job_status", classRead, LevelExec},
 	{"job_output", classRead, LevelExec},
+	{"job_wait", classRead, LevelExec},
 	{"job_kill", classExec, LevelExec},
 	{"list_jobs", classRead, LevelExec},