toolplane: say what a create would make, so a person can approve it

internal/toolplane/allowlist.go

@@ -38,6 +38,8 @@ import (
 	"fmt"
 	"strings"
 	"time"
+
+	"github.com/bigstack-oss/cube-advisor-agent/pkg/tunnelproto"
 )
 
 // Impact is what a tool does to the cluster it runs against. It mirrors the
@@ -208,15 +210,20 @@ func (b Backend) String() string {
 	return fmt.Sprintf("backend(%d)", int(b))
 }
 
-// ControlOp identifies a built-in probe-plane operation. Like Impact it starts
-// at 1, so a tool that sets nothing is not accidentally a control tool.
+// ControlOp identifies a built-in operation — one this process answers from
+// its own state rather than by running a command or calling an API. Like
+// Impact it starts at 1, so a tool that sets nothing is not accidentally a
+// control tool.
 type ControlOp int
 
 const (
 	// ControlProbeStart begins a probe run and returns its id.
 	ControlProbeStart ControlOp = iota + 1
 	// ControlProbeStatus reports a run's state and, when finished, its metrics.
 	ControlProbeStatus
+	// ControlInstanceProfile reports what a create would make, for the
+	// sentence a person approves.
+	ControlInstanceProfile
 )
 
 func (c ControlOp) String() string {
@@ -225,6 +232,8 @@ func (c ControlOp) String() string {
 		return "probe_start"
 	case ControlProbeStatus:
 		return "probe_status"
+	case ControlInstanceProfile:
+		return "describe_instance_profile"
 	}
 	return fmt.Sprintf("control(%d)", int(c))
 }
@@ -237,6 +246,18 @@ type Tool struct {
 	// Description is for the operator reading the allowlist, not the model.
 	Description string
 
+	// Unlisted marks a tool the SaaS calls on its own account and never
+	// offers to the model.
+	//
+	// It is not a security boundary — the executor serves the name to whoever
+	// holds the tunnel, listed or not, and the level and impact checks are
+	// what decide whether a call runs. It is an honesty flag for the
+	// catalogue: without it, a SaaS that compares its model-facing tool list
+	// against this allowlist must either advertise a tool the model has no
+	// use for, or carry an exception typed out on that side — which is the
+	// hand-kept copy toolcatalog exists to abolish.
+	Unlisted bool
+
 	// Argv is the exact command to run. Elements equal to a parameter
 	// placeholder (see Params) are replaced; everything else is literal.
 	// The first element is the executable — resolved from PATH, never a shell.
@@ -599,6 +620,23 @@ var Allowlist = []Tool{
 		// boots afterwards. This bounds the acceptance, not the boot.
 		Timeout: 60 * time.Second,
 	},
+	// The create above takes its flavour, image and network from a file on
+	// this node, and until now the SaaS had no way to learn them. So the
+	// sentence a person approved said the values were "the cluster's own
+	// settings" — true, and not something anyone can consent to. This reports
+	// them, so the sentence can name what will exist.
+	//
+	// Read class: it discloses configuration this node's own operator wrote,
+	// creates nothing and changes nothing. Unlisted, because the model cannot
+	// choose these values and has no use for them; the SaaS calls it while
+	// composing an approval prompt.
+	{
+		Name:        tunnelproto.DescribeInstanceProfile,
+		Description: "Report the flavour, image and network a create would use, for the approval prompt.",
+		Control:     ControlInstanceProfile,
+		Impact:      ImpactRead,
+		Unlisted:    true,
+	},
 }
 
 // ProbeControls is the probe plane's half of the allowlist, kept separate
@@ -785,14 +823,17 @@ func (t Tool) validateControl() error {
 	if len(t.Params) > 0 {
 		return fmt.Errorf("tool %q is a control tool and must declare no parameters; its argument is checked by the probe runner, not substituted", t.Name)
 	}
-	if t.Control != ControlProbeStart && t.Control != ControlProbeStatus {
+	switch t.Control {
+	case ControlProbeStart, ControlProbeStatus, ControlInstanceProfile:
+	default:
 		return fmt.Errorf("tool %q declares an unknown control operation", t.Name)
 	}
-	// The probe plane creates and destroys its own scratch resources and
-	// changes no configuration; a control tool claiming a configuring class is
-	// claiming to be something the probe runner cannot do.
+	// A control tool answers from this process's own state — a probe run it
+	// started, a setting its operator wrote. None of that reaches a cluster's
+	// configuration, so a control tool claiming a configuring class is
+	// claiming to be something no control op can do.
 	if t.Impact != ImpactRead && t.Impact != ImpactScratch {
-		return fmt.Errorf("tool %q is a probe-plane control but declares impact %s; the probe plane changes no configuration", t.Name, t.Impact)
+		return fmt.Errorf("tool %q is a control tool but declares impact %s; a control operation changes no configuration", t.Name, t.Impact)
 	}
 	return nil
 }

internal/toolplane/describe_test.go

@@ -0,0 +1,129 @@
+package toolplane
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"testing"
+
+	"github.com/bigstack-oss/cube-advisor-agent/pkg/tunnelproto"
+)
+
+func describe(t *testing.T, r *Registry) tunnelproto.InstanceProfile {
+	t.Helper()
+	out, err := r.Call(context.Background(), tunnelproto.DescribeInstanceProfile, nil)
+	if err != nil {
+		t.Fatalf("describe: %v", err)
+	}
+	var got tunnelproto.InstanceProfile
+	if err := json.Unmarshal(out, &got); err != nil {
+		t.Fatalf("describe returned %s: %v", out, err)
+	}
+	return got
+}
+
+// The values a person is asked to approve come from this call, so it has to
+// report the ones a create would actually use — not a summary, not a subset.
+func TestDescribeReportsTheProfileACreateWouldUse(t *testing.T) {
+	r, _, _ := newTestRegistry(t)
+	r.ConfigureInstanceProfile(InstanceProfile{
+		Flavor: "m1.large", Image: "ubuntu-24.04", Network: "tenant-net", Project: "acme-prod",
+	})
+
+	got := describe(t, r)
+	want := tunnelproto.InstanceProfile{
+		Configured: true,
+		Flavor:     "m1.large", Image: "ubuntu-24.04", Network: "tenant-net", Project: "acme-prod",
+	}
+	if got != want {
+		t.Errorf("describe = %+v, want %+v", got, want)
+	}
+}
+
+// An agent nobody has configured says so and invents nothing. The caller is
+// composing a sentence for a person: empty strings rendered into it would
+// promise a machine with no image.
+func TestAnUnconfiguredAgentDescribesNoProfile(t *testing.T) {
+	r, _, _ := newTestRegistry(t)
+
+	got := describe(t, r)
+	if got.Configured {
+		t.Errorf("an unconfigured agent reports a profile: %+v", got)
+	}
+	if got.Flavor != "" || got.Image != "" || got.Network != "" {
+		t.Errorf("an unconfigured agent named values: %+v", got)
+	}
+}
+
+// A half-configured profile refuses every create, so reporting it as
+// configured would put values in front of a person for a call that cannot run.
+func TestAPartialProfileIsNotReportedAsConfigured(t *testing.T) {
+	for _, p := range []InstanceProfile{
+		{Image: "ubuntu-24.04", Network: "tenant-net"},
+		{Flavor: "m1.large", Network: "tenant-net"},
+		{Flavor: "m1.large", Image: "ubuntu-24.04"},
+	} {
+		r, _, _ := newTestRegistry(t)
+		r.ConfigureInstanceProfile(p)
+		if got := describe(t, r); got.Configured {
+			t.Errorf("profile %+v reported as configured", p)
+		}
+	}
+}
+
+// It answers one question and takes nothing. A caller passing an argument has
+// misunderstood what this reports, and ignoring it would let that
+// misunderstanding reach an approval prompt.
+func TestDescribeRefusesArguments(t *testing.T) {
+	r, rec, _ := newTestRegistry(t)
+	r.ConfigureInstanceProfile(InstanceProfile{Flavor: "f", Image: "i", Network: "n"})
+
+	_, err := r.Call(context.Background(), tunnelproto.DescribeInstanceProfile,
+		map[string]string{"cluster": "other"})
+	if !errors.Is(err, ErrBadArgument) {
+		t.Fatalf("describe with an argument = %v, want ErrBadArgument", err)
+	}
+	if len(rec.calls) == 0 || rec.calls[len(rec.calls)-1].Allowed {
+		t.Error("the refusal was not audited")
+	}
+}
+
+// The describing tool reaches no probe runner, and an agent built without one
+// still has to say what a create would make. This is the guard on the
+// restructure that let a non-probe control op past the runner check.
+func TestDescribeWorksWithoutAProbeRunner(t *testing.T) {
+	r, err := New(Allowlist, &recorder{})
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+	if r.probes != nil {
+		t.Fatal("a registry built without WithProbes has a runner")
+	}
+	r.ConfigureInstanceProfile(InstanceProfile{Flavor: "f", Image: "i", Network: "n"})
+
+	if got := describe(t, r); !got.Configured {
+		t.Errorf("describe without a probe runner = %+v, want the profile", got)
+	}
+}
+
+// The model cannot choose a flavour or an image, so a tool reporting them has
+// no business in its tool list. Unlisted is what lets the SaaS know that
+// absence is deliberate rather than a list gone stale.
+func TestDescribeIsNotOfferedToTheModel(t *testing.T) {
+	var found bool
+	for _, tool := range Allowlist {
+		if tool.Name != tunnelproto.DescribeInstanceProfile {
+			continue
+		}
+		found = true
+		if !tool.Unlisted {
+			t.Error("describe_instance_profile is listed to the model")
+		}
+		if tool.Impact != ImpactRead {
+			t.Errorf("describe_instance_profile impact = %s, want read", tool.Impact)
+		}
+	}
+	if !found {
+		t.Fatal("the allowlist has no describe_instance_profile")
+	}
+}

internal/toolplane/registry.go

@@ -448,13 +448,20 @@ func (r *Registry) Call(ctx context.Context, name string, args map[string]string
 	return out, runErr
 }
 
-// callControl dispatches a probe-plane control call.
+// callControl dispatches a control call — one this process answers from its
+// own state rather than by running a command or calling an API.
 //
-// Both operations are short by construction — starting a probe returns once
-// the goroutine is launched, polling one reads a map — so neither needs the
-// timeout ladder stretched to fit the measurement it controls. Results go back
-// as JSON so the SaaS forwards numbers rather than prose.
+// Every operation is short by construction: starting a probe returns once the
+// goroutine is launched, polling one reads a map, describing the profile reads
+// a struct. None needs the timeout ladder stretched to fit what it reports on.
+// Results go back as JSON so the SaaS forwards values rather than prose.
 func (r *Registry) callControl(ctx context.Context, tool Tool, args map[string]string) ([]byte, error) {
+	// The profile description is answered before the probe runner is
+	// considered, because it needs none: an agent with no probe plane still
+	// has to be able to say what a create would make.
+	if tool.Control == ControlInstanceProfile {
+		return r.callDescribeProfile(tool, args)
+	}
 	if r.probes == nil {
 		// Unreachable: the scratch class is refused at registration without a
 		// runner. Kept so a future control tool that is not scratch-class
@@ -509,6 +516,45 @@ func (r *Registry) callControl(ctx context.Context, tool Tool, args map[string]s
 	return nil, fmt.Errorf("%w: %q", ErrUnknownTool, tool.Name)
 }
 
+// callDescribeProfile reports what a create would make.
+//
+// It takes no arguments at all, and says so rather than ignoring them: a caller
+// passing one has misunderstood what this answers, and silence would let that
+// misunderstanding reach an approval prompt.
+//
+// An unconfigured agent reports Configured false rather than failing. The
+// caller is composing a sentence for a person, and "this cluster has not been
+// told what to create" is something worth saying — a failure here would leave
+// it with nothing and no reason.
+func (r *Registry) callDescribeProfile(tool Tool, args map[string]string) ([]byte, error) {
+	for k := range args {
+		r.audit.RecordToolCall(ToolCall{
+			Tool: tool.Name, Args: args, Allowed: false,
+			Reason: fmt.Sprintf("unexpected argument %q", k), At: time.Now().UTC(),
+		})
+		return nil, fmt.Errorf("%w: unexpected argument %q", ErrBadArgument, k)
+	}
+	p := r.profile
+	out, err := json.Marshal(tunnelproto.InstanceProfile{
+		// A profile is usable only with all three; a half-configured one
+		// refuses creates, and reporting it as configured would put values in
+		// an approval prompt for a call that cannot run.
+		Configured: p.Flavor != "" && p.Image != "" && p.Network != "",
+		Flavor:     p.Flavor,
+		Image:      p.Image,
+		Network:    p.Network,
+		Project:    p.Project,
+	})
+	if err != nil {
+		return nil, err
+	}
+	r.audit.RecordToolCall(ToolCall{
+		Tool: tool.Name, Args: args, Allowed: true,
+		At: time.Now().UTC(), Bytes: len(out),
+	})
+	return out, nil
+}
+
 // callGet resolves a Get tool's path and fetches it from cube-cos-api.
 //
 // The path is the whole request: the method is GET (the getter offers nothing

pkg/toolcatalog/toolcatalog.go

@@ -38,6 +38,16 @@ type Entry struct {
 	// comparison that ignored the distinction would demand the two lists
 	// match when they legitimately do not.
 	Probe bool
+	// Unlisted reports whether this entry is one the SaaS calls on its own
+	// account rather than offering to the model — describing the instance
+	// profile for an approval prompt, for instance.
+	//
+	// Published for the same reason Probe is. A SaaS comparing its tool list
+	// against this catalogue has to know which absences are deliberate, and
+	// the alternative is a list of exceptions typed out over there: a
+	// hand-kept copy of a fact this side already knows, which is the shape of
+	// failure this package exists to remove.
+	Unlisted bool
 
 	// Reads is the sorted set of catalogue keys this entry accepts, empty for
 	// a tool that is not a catalogue read.
@@ -59,7 +69,10 @@ type Entry struct {
 func Entries() []Entry {
 	out := make([]Entry, 0, len(toolplane.Allowlist)+len(toolplane.ProbeControls))
 	for _, t := range toolplane.Allowlist {
-		out = append(out, Entry{Name: t.Name, Impact: t.Impact.String(), Reads: readsOf(t)})
+		out = append(out, Entry{
+			Name: t.Name, Impact: t.Impact.String(),
+			Unlisted: t.Unlisted, Reads: readsOf(t),
+		})
 	}
 	for _, t := range toolplane.ProbeControls {
 		out = append(out, Entry{Name: t.Name, Impact: t.Impact.String(), Probe: true})

pkg/toolcatalog/toolcatalog_test.go

@@ -29,6 +29,14 @@ func TestEveryAllowlistEntryIsPublished(t *testing.T) {
 		if e.Probe {
 			t.Errorf("%s is not a probe control but is published as one", tool.Name)
 		}
+		// Whether a tool is offered to the model is the SaaS's only reason to
+		// tolerate an absence from its own list, so a mismatch here would
+		// leave that side unable to tell a deliberate omission from a stale
+		// copy — which is the whole failure this package removes.
+		if e.Unlisted != tool.Unlisted {
+			t.Errorf("%s: catalogue says unlisted=%v, allowlist says %v",
+				tool.Name, e.Unlisted, tool.Unlisted)
+		}
 	}
 	for _, tool := range toolplane.ProbeControls {
 		if e := got[tool.Name]; !e.Probe {

pkg/tunnelproto/toolcall.go

@@ -133,3 +133,32 @@ func ReadToolResult(r io.Reader) (ToolResult, error) {
 // MaxToolOutputBytes bounds a result frame. Generous next to `cluster check`
 // output, small next to memory exhaustion from a hostile or broken peer.
 const MaxToolOutputBytes = 1 << 20
+
+// DescribeInstanceProfile is the tool the SaaS calls to learn what a create
+// would make, so the sentence a person approves can name it.
+//
+// It is not offered to the model. The model already cannot choose a flavour or
+// an image — that is what the profile is for — so advertising a tool that
+// reports them would widen what the model sees without widening what it can do.
+const DescribeInstanceProfile = "describe_instance_profile"
+
+// InstanceProfile is what a cluster creates, as the SaaS needs to state it.
+//
+// It crosses the tool channel as that tool's result. Defined here because both
+// repositories read it: the executor renders it from the file its operator
+// wrote, the SaaS renders a sentence from it. A shape only one side could
+// import gets restated on the other, which is how two definitions of one
+// protocol come to disagree — cube-ai-advisor#121 and cube-advisor-agent#24
+// were both green while doing exactly that.
+//
+// Configured separates "this cluster has no profile" from "its fields are
+// empty". Those are different things to tell a person: the first is an operator
+// who has not opted in, the second would be a bug. A caller reading only the
+// fields could not tell them apart.
+type InstanceProfile struct {
+	Configured bool   `json:"configured"`
+	Flavor     string `json:"flavor,omitempty"`
+	Image      string `json:"image,omitempty"`
+	Network    string `json:"network,omitempty"`
+	Project    string `json:"project,omitempty"`
+}