diff --git a/internal/mcp/api.go b/internal/mcp/api.go index f37f2ea..3402143 100644 --- a/internal/mcp/api.go +++ b/internal/mcp/api.go @@ -18,7 +18,7 @@ func apiTools() []*tool { return []*tool{ { name: "api_catalog", title: "Start technical troubleshooting", - description: "For technical troubleshooting, start here. Ask the connected box for every diagnostic read it supports, its running version, source-code links and a workflow for collecting bug-report evidence. Includes energy logs, original measurements, planner snapshots, configuration, driver source, logs, support reports and exports. Ordinary status, history and control use the existing tools. The owner-approved diagnostic grant covers these reads; there are no extra endpoint permissions." + dataNote, + description: "For technical troubleshooting, start here. Ask the connected box for every diagnostic read it supports, its running version, source-code links and a workflow for collecting bug-report evidence. Newer boxes include charger_connections with exact saved driver names and configured, missing or unverified_ocpp status; configured means a name matches, not verified equipment. Includes energy logs, original measurements, planner snapshots, configuration, driver source, logs, support reports and exports. The basic config_get and drivers_status tools redact names and cannot verify charger bindings. Ordinary status, history and control use the existing tools. The owner-approved diagnostic grant covers these reads; there are no extra endpoint permissions." + dataNote, scope: readScope(protocol.MsgAPIRead), input: object(nil), readOnly: true, run: func(ctx context.Context, cc *callCtx, args json.RawMessage) (any, error) { if err := decodeArgs(args, &struct{}{}); err != nil { @@ -119,7 +119,11 @@ func diagnosticAPIRead(ctx context.Context, cc *callCtx, read protocol.APIRead) } // Preserve the box's technical evidence, including exact json.Number // values. Existing typed reads retain their own display sanitization. - return map[string]any{"path": read.Path, "status": head.Status, "response": data}, nil + result := map[string]any{"path": read.Path, "status": head.Status, "response": data} + if head.Status >= 400 { + return failedToolData{result}, nil + } + return result, nil } // apiRead collects the bounded api.* stream. A missing head, a skipped diff --git a/internal/mcp/charger_diagnostics_test.go b/internal/mcp/charger_diagnostics_test.go new file mode 100644 index 0000000..01e9d09 --- /dev/null +++ b/internal/mcp/charger_diagnostics_test.go @@ -0,0 +1,68 @@ +package mcp + +import ( + "strings" + "testing" + + "github.com/srcfl/ftw-cloud/internal/oauth" + "github.com/srcfl/ftw/go/protocol" +) + +func TestDiagnosticReadsPreserveAnOldChargerBindingForComparison(t *testing.T) { + world := newWorld(t) + box := world.connectBox() + assistant := world.connect(box, oauth.ScopeDiagRun) + read := func(path string) map[string]any { + t.Helper() + result, failed, _ := assistant.call("api_read", map[string]any{"path": path}) + if failed { + t.Fatal("full diagnostic read failed") + } + transport := result["structuredContent"].(map[string]any)["data"].(map[string]any) + return transport["response"].(map[string]any)["body"].(map[string]any) + } + cfg := read("/api/config") + current := cfg["drivers"].([]any)[0].(map[string]any)["name"] + saved := cfg["loadpoints"].([]any)[0].(map[string]any)["driver_name"] + runtime := read("/api/drivers")["drivers"].([]any)[0].(map[string]any)["name"] + if current != "charger-current" || saved != "charger-old" || current != runtime { + t.Fatal("full diagnostic reads changed the names needed to identify an old binding") + } + _, _, asked := box.counts() + for _, request := range asked { + if strings.HasPrefix(request, protocol.MsgAPIReq+" ") { + t.Fatal("troubleshooting used an owner write path") + } + } +} + +func TestRefusedDiagnosticReadIsAnErrorWithItsBoxEvidence(t *testing.T) { + world := newWorld(t) + assistant := world.connect(world.connectBox(), oauth.ScopeDiagRun) + result, failed, _ := assistant.call("api_read", map[string]any{"path": "/api/test/refused"}) + if !failed { + t.Fatal("a refused diagnostic read looked successful") + } + transport := result["structuredContent"].(map[string]any)["data"].(map[string]any) + if transport["status"] != float64(403) || transport["response"].(map[string]any)["error"] != "diagnostic read refused" { + t.Fatal("the failed read lost its status or box evidence") + } +} + +func TestTroubleshootingGuidanceUsesFullReadsAndStatesRedactionLimits(t *testing.T) { + for _, text := range []string{"api_catalog", "api_read", "config_get", "/api/config", "/api/drivers", "/api/loadpoints", "refused"} { + if !strings.Contains(Instructions, text) { + t.Errorf("missing troubleshooting guidance: %s", text) + } + } + server := &Server{tools: readTools()} + description := server.tool("config_get").description + if !strings.Contains(description, "cannot verify") || !strings.Contains(description, "redacted") { + t.Fatal("basic config tool does not state its diagnostic limit") + } + for _, tool := range []string{"api_read", "api_catalog"} { + if strings.Contains(description, tool) { + t.Fatal("the public basic tool recommends an unavailable tool") + } + } +} diff --git a/internal/mcp/e2e_test.go b/internal/mcp/e2e_test.go index f42113b..37d1700 100644 --- a/internal/mcp/e2e_test.go +++ b/internal/mcp/e2e_test.go @@ -551,6 +551,21 @@ func (b *stubBox) handle(s *boxSession, env appwire.Envelope) { if q.Path == "/api/status" { data, _ = json.Marshal(map[string]any{"text": strings.Repeat("x", 8000)}) } + status := http.StatusOK + switch q.Path { + case "/api/config": + data, _ = json.Marshal(map[string]any{"encoding": "json", "complete": true, "body": map[string]any{ + "drivers": []any{map[string]any{"name": "charger-current"}}, + "loadpoints": []any{map[string]any{"id": "garage", "driver_name": "charger-old"}}, + }}) + case "/api/drivers": + data, _ = json.Marshal(map[string]any{"encoding": "json", "complete": true, "body": map[string]any{ + "drivers": []any{map[string]any{"name": "charger-current", "status": "ok"}}, + }}) + case "/api/test/refused": + status = http.StatusForbidden + data, _ = json.Marshal(map[string]any{"error": "diagnostic read refused"}) + } if q.Path == protocol.AgentAPICatalogPath { data = []byte(`{"routes":[{"path":"/api/energy/history"},{"path":"/api/series"},{"path":"/api/mpc/diagnose/at"},{"path":"/api/drivers/{id}/source"},{"path":"/api/future/read"}],"troubleshooting":{"core":{"version":"0.151.0-beta.1","source":"https://github.com/srcfl/ftw/tree/v0.151.0-beta.1"},"workflow":["Collect reproduction steps and incident time with timezone"]}}`) } @@ -569,7 +584,7 @@ func (b *stubBox) handle(s *boxSession, env appwire.Envelope) { if truncated { data = data[:q.MaxBytes] } - b.reply(s, protocol.MsgAPIHead, env.ID, protocol.APIHeadMsg{Status: 200, Headers: map[string]string{"Content-Type": "application/json"}}) + b.reply(s, protocol.MsgAPIHead, env.ID, protocol.APIHeadMsg{Status: status, Headers: map[string]string{"Content-Type": "application/json"}}) cut := len(data) / 2 b.mu.Lock() badSequence := b.apiBadSequence diff --git a/internal/mcp/mcp.go b/internal/mcp/mcp.go index 4e2f39f..2dedf13 100644 --- a/internal/mcp/mcp.go +++ b/internal/mcp/mcp.go @@ -62,7 +62,12 @@ const Instructions = "FTW Cloud reaches one home's FTW box as an assistant its o "Setup edits need a separate owner-approved setup grant. Saving does not prove measurement or control on hardware. " + "For ordinary status, history and control, use the existing tools. For technical troubleshooting, " + "start with api_catalog: the box lists its full read API, running version, source-code links and " + - "steps for gathering bug-report evidence. api_read keeps technical identifiers, addresses, logs and source code." + "steps for gathering bug-report evidence. api_read keeps technical identifiers, addresses, logs and source code. " + + "For charger faults, check charger_connections when present, then read /api/config, /api/drivers and /api/loadpoints. " + + "Compare loadpoints[].driver_name with drivers[].name; when OCPP is enabled also read /api/ocpp/chargers. " + + "A missing saved binding is a configuration problem, not evidence that a vendor driver failed. " + + "config_get and drivers_status redact names and cannot verify these bindings. " + + "If a read is refused or unavailable, report the exact path and error and what remains unverified; never guess the missing result." // JSON-RPC and MCP error codes. const ( diff --git a/internal/mcp/public.go b/internal/mcp/public.go index 3407b47..1cd426a 100644 --- a/internal/mcp/public.go +++ b/internal/mcp/public.go @@ -16,6 +16,7 @@ const publicInstructions = "FTW reaches one home's box with its owner's approval "from fresh measurements, power in W from energy in Wh, and plans from measured outcomes. " + "Control needs a separate grant and the user's request; lasting changes wait for the owner. " + "This connection cannot run arbitrary API reads, export backups or configure an installation. " + + "Redacted settings cannot verify exact charger-to-driver bindings; state that limit rather than guessing a driver fault. " + "For a new installation, guide the user through FTW's maintained install documentation." // publicTool is an explicit list: a new tool needs review before this profile diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 03bf736..38c5bd8 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -403,7 +403,7 @@ func readTools() []*tool { }, { name: "config_get", title: "Settings", - description: "The box's configuration, redacted on the box: secrets, addresses and names removed. Without a section, the list of sections; with one, that section." + dataNote, + description: "A basic redacted settings overview. Secrets, addresses and names are removed or changed, so this result cannot verify a charger's exact driver binding. Full technical diagnostic reads, when this connection provides them, are needed for that comparison. Otherwise state what you cannot verify rather than attributing a fault to a driver. Without a section, the list of sections; with one, that section." + dataNote, scope: readScope(protocol.MsgConfigGet), readOnly: true, input: object(map[string]any{ "section": map[string]any{"type": "string", "pattern": sectionName.String(), "maxLength": 64, "description": "One section, from the list. Leave out for the list."},