diff --git a/docs/public-plugin.md b/docs/public-plugin.md index 2a077a3..54e4389 100644 --- a/docs/public-plugin.md +++ b/docs/public-plugin.md @@ -18,8 +18,12 @@ The profile exposes these names in both supported MCP protocol families: - `homes_list` - `status`, `chargers_list`, `cars_list` - `plan_get`, `plan_explain`, `history_query` -- `health_get`, `drivers_status`, `logs_tail`, `config_get` -- `ev_charge_now`, `goal_request`, `requests_list` +- `health_get`, `drivers_status`, `logs_tail`, `config_get`, + `measurement_diagnostics` +- `ev_charge_now`, `goal_request`, `car_goal_request`, `requests_list` +- `setup_begin`, `setup_schema`, `setup_status`, `setup_discover` +- `setup_edit`, `setup_reset`, `setup_validate`, `setup_test`, + `setup_connect`, `setup_commit`, `setup_restart` It rejects every other tool, including a direct call to a name absent from its listing. The list is explicit, so adding a technical tool does not add @@ -29,8 +33,43 @@ fail instead of falling back to the full tool set. Both `initialize` and The initial authentication challenge asks for live, plan, history and health reads. Configure the package's default OAuth scopes to those same four. -Logs, redacted settings and controls still need the relevant owner grant. -The profile never grants a right by itself. +Logs, redacted settings, diagnostics, setup and controls still need the +relevant owner grant. The profile never grants a right by itself. + +## Tool annotations + +Every tool sets `readOnlyHint`, `destructiveHint` and `openWorldHint` to +true or false, following the +[plugin guidelines](https://developers.openai.com/plugins/plugin-guidelines). +`internal/mcp/hints_test.go` holds the same table, so changing a hint means +changing that table and this one. + +- **Read-only** means the tool changes nothing: not the box, its devices, + its drafts or the owner's records. +- **Destructive** means a write that can overwrite, delete or cancel + something, at once or when the owner approves it. A hold that ends by + itself still counts. +- **Open world** means the tool reaches past the one home the owner + granted: the local network, an address the user typed into the draft, or + a vendor's service. Every other tool talks only to that home's box. + +| Tool | Read-only | Destructive | Open world | Why | +|---|---|---|---|---| +| `homes_list` | true | false | false | Returns this connection's grant. It does not reach the box. | +| `status`, `chargers_list`, `cars_list`, `plan_get`, `plan_explain`, `history_query`, `health_get`, `logs_tail`, `drivers_status`, `config_get`, `measurement_diagnostics`, `requests_list` | true | false | false | Each asks the owner's box for data and returns it. Nothing on the box changes. | +| `setup_begin`, `setup_schema`, `setup_status` | true | false | false | Core serves these as GET reads of the setup schema, draft and guide. `setup_begin` needs the setup grant, so the client may ask the owner to approve it first; the tool itself changes nothing. | +| `setup_discover` | true | false | true | Scans the home's network and may fingerprint devices with reads. Core fails the call if the draft changed meanwhile. Open world: it probes whatever hosts answer, not a fixed list. | +| `ev_charge_now` | false | true | false | Overrides the plan at one charger for at most the grant's `holdS`; `stop` cancels this assistant's hold. Only chargers the box lists for this home. | +| `goal_request` | false | true | false | Stores a request that waits for the owner. Approved, it replaces the home's mode, planning style, solar-only setting or the car's charge level. | +| `car_goal_request` | false | true | false | Stores a request that waits for the owner. Approved, it replaces the car's schedule, or removes it with `clear`. | +| `setup_edit` | false | true | false | Changes the draft. A merge patch replaces whole arrays; JSON Patch can remove entries. | +| `setup_reset` | false | true | false | Discards the unfinished draft and starts a new one from live settings. | +| `setup_validate` | false | true | false | Saves Core's normalized copy of the draft as a new revision, in place of the stored one. | +| `setup_test` | false | true | true | Runs the draft's driver against the device or vendor service the draft names, with device writes and control blocked. A rotated vendor token replaces the old one in the draft, and the vendor can revoke the old one. Open world: the user supplies the address. | +| `setup_connect` | false | true | true | Starts a vendor sign-in (myUplink today) on the vendor's site. When it finishes, the vendor's token replaces the driver's token in the draft. Open world: a third-party service. | +| `setup_commit` | false | true | false | Replaces live settings with the draft the user confirmed. | +| `setup_restart` | false | true | false | Restarts Core to apply saved settings, which briefly stops control and readings. | +| `api_catalog`, `api_read` | true | false | false | Base `/mcp` only. GET reads of the box's own diagnostic API, from the paths the box lists. | ## Limits @@ -38,9 +77,8 @@ The profile never grants a right by itself. profile excludes them because the directory's [plugin guidelines](https://developers.openai.com/plugins/plugin-guidelines) require each model-callable operation to appear as a separate reviewed tool. -The profile also excludes setup writes and the `events_list` and `diag_run` -operations that Core does not yet serve. Installation help remains a skill -that guides the owner through the maintained installer and local UI. +The profile also excludes the `events_list` and `diag_run` operations that +Core does not yet serve. `history_query` returns downsampled power readings. It does not establish exact energy totals or savings. Skills and listing text must state that diff --git a/internal/mcp/car_goals.go b/internal/mcp/car_goals.go index 4c41fb1..765da76 100644 --- a/internal/mcp/car_goals.go +++ b/internal/mcp/car_goals.go @@ -11,6 +11,8 @@ import ( func carGoalTool() *tool { return &tool{ name: "car_goal_request", title: "Ask for a car charging goal", scope: oauth.ScopeGoalsRequest, + // Approved, it replaces or clears the car's schedule. + destructive: true, description: "Ask for a lasting car charging goal after the user confirms the car, target and departure time. Use the opaque car id from cars_list or charger_id from chargers_list, exactly one. Confirm the box time zone before converting local HH:MM into minutes after midnight. During setup, read runtime_timezone in setup_schema; otherwise use the known installation time zone or ask the owner to check it. Do not assume the assistant client time zone. days is a weekday mask: Monday=1 through Sunday=64; 0 means once at the next ready time, 127 means every day. A new goal replaces the previous schedule. Nothing changes until the owner approves in Settings → Access on the local box or in the FTW app. Read requests_list, then cars_list to check the outcome.", input: object(map[string]any{ "car": map[string]any{"type": "string", "description": "Opaque car id from cars_list"}, diff --git a/internal/mcp/control.go b/internal/mcp/control.go index 6d54677..5fe5d60 100644 --- a/internal/mcp/control.go +++ b/internal/mcp/control.go @@ -110,6 +110,8 @@ func controlTools() []*tool { { name: "goal_request", title: "Ask for a lasting change", + // Approved, it replaces a setting. + destructive: true, description: "Ask the owner for a lasting change: the home's mode, the planning style, solar-only charging at a charger, or a correction of the car's charge level. " + "Nothing changes until an owner confirms it in Settings → Access on the local box or in the FTW app; requests_list shows how it ended. Put your reason, in your own words, in why.", scope: oauth.ScopeGoalsRequest, diff --git a/internal/mcp/control_test.go b/internal/mcp/control_test.go index 46bf8ea..3da781d 100644 --- a/internal/mcp/control_test.go +++ b/internal/mcp/control_test.go @@ -301,22 +301,6 @@ func TestARetryIsTheSameCommand(t *testing.T) { } } -// ev_charge_now is marked as a tool that changes what a home does. -func TestChargeNowIsMarkedDestructive(t *testing.T) { - _, _, a := withControl(t) - r := a.rpc("tools/list", nil, true) - for _, x := range r.Body["result"].(map[string]any)["tools"].([]any) { - tl := x.(map[string]any) - ann := tl["annotations"].(map[string]any) - if tl["name"] == "ev_charge_now" && ann["destructiveHint"] != true { - t.Fatal("ev_charge_now is not destructive") - } - if tl["name"] == "status" && ann["destructiveHint"] != false { - t.Fatal("status is destructive") - } - } -} - // A box without a clock refuses control at once, in words. func TestControlOnABoxWithoutAClock(t *testing.T) { _, b, a := withControl(t) diff --git a/internal/mcp/hints_test.go b/internal/mcp/hints_test.go new file mode 100644 index 0000000..c7d468d --- /dev/null +++ b/internal/mcp/hints_test.go @@ -0,0 +1,77 @@ +package mcp + +import ( + "slices" + "testing" +) + +// hints is each tool's readOnlyHint, destructiveHint and openWorldHint, +// written out so a change to one is a reviewed change to this table. The +// reasons are in docs/public-plugin.md, "Tool annotations". +var hints = map[string][3]bool{ + "homes_list": {true, false, false}, + "status": {true, false, false}, + "chargers_list": {true, false, false}, + "cars_list": {true, false, false}, + "plan_get": {true, false, false}, + "plan_explain": {true, false, false}, + "history_query": {true, false, false}, + "health_get": {true, false, false}, + "logs_tail": {true, false, false}, + "drivers_status": {true, false, false}, + "config_get": {true, false, false}, + "measurement_diagnostics": {true, false, false}, + "api_catalog": {true, false, false}, + "api_read": {true, false, false}, + "requests_list": {true, false, false}, + "ev_charge_now": {false, true, false}, + "goal_request": {false, true, false}, + "car_goal_request": {false, true, false}, + "setup_begin": {true, false, false}, + "setup_schema": {true, false, false}, + "setup_status": {true, false, false}, + "setup_discover": {true, false, true}, + "setup_edit": {false, true, false}, + "setup_reset": {false, true, false}, + "setup_validate": {false, true, false}, + "setup_test": {false, true, true}, + "setup_connect": {false, true, true}, + "setup_commit": {false, true, false}, + "setup_restart": {false, true, false}, +} + +// Every listed tool carries all three hints as booleans, and they are the +// ones in the table. A new tool fails here until it has a row. +func TestToolHintsMatchTheTable(t *testing.T) { + _, _, a := withControl(t) + for _, modern := range []bool{false, true} { + r := a.rpc("tools/list", nil, modern) + var names []string + for _, x := range r.Body["result"].(map[string]any)["tools"].([]any) { + tl := x.(map[string]any) + name := tl["name"].(string) + names = append(names, name) + want, ok := hints[name] + if !ok { + t.Errorf("%s has no row in hints", name) + continue + } + ann := tl["annotations"].(map[string]any) + for i, key := range []string{"readOnlyHint", "destructiveHint", "openWorldHint"} { + got, isBool := ann[key].(bool) + if !isBool || got != want[i] { + t.Errorf("%s %s = %v, want %v", name, key, ann[key], want[i]) + } + } + // A read-only tool changes nothing, so it destroys nothing. + if ann["readOnlyHint"] == true && ann["destructiveHint"] == true { + t.Errorf("%s is read-only and destructive", name) + } + } + for name := range hints { + if !slices.Contains(names, name) { + t.Errorf("hints lists %s, which is not served", name) + } + } + } +} diff --git a/internal/mcp/setup.go b/internal/mcp/setup.go index df84992..be95f67 100644 --- a/internal/mcp/setup.go +++ b/internal/mcp/setup.go @@ -28,7 +28,7 @@ func setupTools() []*tool { commit["confirmed"] = map[string]any{"type": "boolean", "description": "True only after the user reviewed this exact checked draft, its changes and any restart, and confirmed saving it"} definitions := []setupToolDef{ {"setup_begin", "Begin owner-approved system setup", protocol.MsgSetupStatus, - "Use when the user asks to configure a new or existing box. Request the separate setup and diagnostic grant together, then read the box's current draft and guide. This read changes nothing and never resets existing work. If no box is connected, guide the owner through Start assistant setup on its local page and approve this connection there. Then read setup_schema and follow the returned guide.", object(nil), true}, + "Use when the user asks to configure a new or existing box. Reads the box's current draft and guide; it changes nothing and never resets existing work. It needs the separate setup and diagnostic grant, so the client may first ask the owner to approve that access. If no box is connected, guide the owner through Start assistant setup on its local page and approve this connection there. Then read setup_schema and follow the returned guide.", object(nil), true}, {"setup_schema", "Start or understand system setup", protocol.MsgSetupSchema, "Use only when the user wants to set up or change an installation. Read the complete configuration schema, driver catalogue and workflow from the connected box. Ordinary status/history/control need no setup interview.", object(nil), true}, {"setup_status", "Resume system setup", protocol.MsgSetupStatus, @@ -52,8 +52,8 @@ func setupTools() []*tool { definitions = append(definitions, setupToolDef{ "setup_discover", "Find equipment for setup", protocol.MsgSetupEdit, - "Only after the user agrees: scan the connected box's local network and optionally fingerprint compatible devices. Bound to the current draft revision. Results are observations to review; this does not select equipment or apply configuration.", - object(map[string]any{"revision": revision, "confirmed": confirmed, "fingerprint": map[string]any{"type": "boolean", "description": "Also run read-only device fingerprints"}}, "revision", "confirmed"), false}, + "Only after the user agrees: scan the connected box's local network and optionally fingerprint compatible devices. Bound to the current draft revision. Results are observations to review; this changes neither the draft nor any device, and does not select equipment or apply configuration.", + object(map[string]any{"revision": revision, "confirmed": confirmed, "fingerprint": map[string]any{"type": "boolean", "description": "Also run read-only device fingerprints"}}, "revision", "confirmed"), true}, setupToolDef{ "setup_test", "Test a draft equipment connection", protocol.MsgSetupEdit, "After the user agrees, briefly run the selected draft driver to read equipment. Uses this draft's credentials and keeps any token rotation in the draft. Device writes and control hooks are blocked; a driver that needs an undeclared authentication or subscription write may need attention. Connection and readings do not prove actuation. Read setup_status afterward because credential rotation can advance the draft revision.", @@ -70,8 +70,13 @@ func setupTools() []*tool { if d.name == "setup_begin" { scope = protocol.ScopeSetupWrite } + // Every setup write overwrites or deletes something: draft answers, + // the whole draft, a draft credential, live settings or the running + // process. The three preparations reach past the box's records: the + // local network, the device address in the draft, a vendor's sign-in. + _, openWorld := preparation[d.name] result = append(result, &tool{name: d.name, title: d.title, description: d.description + dataNote, - scope: scope, input: d.input, readOnly: d.readOnly, destructive: d.message == protocol.MsgSetupCommit || d.message == protocol.MsgSetupRestart, + scope: scope, input: d.input, readOnly: d.readOnly, destructive: !d.readOnly, openWorld: openWorld, run: func(ctx context.Context, cc *callCtx, args json.RawMessage) (any, error) { if !json.Valid(args) || len(args) > protocol.SetupRequestBytes { return nil, userError("Keep each setup patch below 10 KiB and use successive draft revisions.") diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 361ee3d..8806a1a 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -26,9 +26,14 @@ type tool struct { scope string input map[string]any readOnly bool - // destructive is a tool that can change what a home does. + // destructive is a write that can overwrite, delete or cancel + // something, at once or when the owner approves it. destructive bool - run func(ctx context.Context, cc *callCtx, args json.RawMessage) (any, error) + // openWorld is a tool that reaches past the home's box and its records: + // the local network, a device address the draft names, or a vendor's + // sign-in. + openWorld bool + run func(ctx context.Context, cc *callCtx, args json.RawMessage) (any, error) } // Setup writes need the diagnostic reads that let the assistant understand @@ -116,7 +121,7 @@ func (s *Server) listing() []map[string]any { "readOnlyHint": t.readOnly, "destructiveHint": t.destructive, "idempotentHint": t.readOnly, - "openWorldHint": false, + "openWorldHint": t.openWorld, }, }) }