diff --git a/.changeset/access-names-charger-and-car.md b/.changeset/access-names-charger-and-car.md new file mode 100644 index 000000000..060ac67cf --- /dev/null +++ b/.changeset/access-names-charger-and-car.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +The Access tab names the car and the charger in an assistant's request by what the box calls them, instead of the car's id. diff --git a/docs/agents.md b/docs/agents.md index ec91012b8..78689ef0d 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -592,6 +592,12 @@ above and its approval key, and an agent can never reach them. and the longest grant the box takes for each role. It also carries `registry {agentOps, writeScopes, agentArgs}`, so the page names no scope, op or argument by hand. +- **The page names the charger and the car.** A request's `args` hold the + box's own ids, as an owner's `agent.list` does. On this route alone each + request also carries `names`: for every argument of kind `loadpoint` or + `car` that the box still knows, what the box calls it. A charger's name is + its id; a car's is the name the owner gave it. The page is local and its + caller an owner; nothing an agent or FTW Cloud reads carries `names`. - **The box says why it will not take an offer.** `POST /api/agents/offer` and `POST /api/agents` answer an offer the box will not take with 400 and `{error, reason, boxMs}`: `reason` is `no_offer_keys`, `unsigned`, diff --git a/go/cmd/ftw/agents.go b/go/cmd/ftw/agents.go index 458d103f4..3b692ba35 100644 --- a/go/cmd/ftw/agents.go +++ b/go/cmd/ftw/agents.go @@ -411,13 +411,18 @@ func (b boxPageAgents) Agents() []protocol.AgentRow { return b.p.access.Agents() func (b boxPageAgents) ApprovalKeys() []protocol.ApprovalKeyInfo { return b.p.access.ApprovalKeys() } -func (b boxPageAgents) Requests() []protocol.Request { +func (b boxPageAgents) Requests() []api.AgentRequest { h, err := b.p.boxPage("box") if err != nil { return nil } defer h.Close() - return h.Requests() + reqs := h.Requests() + out := make([]api.AgentRequest, 0, len(reqs)) + for _, r := range reqs { + out = append(out, api.AgentRequest{Request: r, Names: h.ArgNames(r)}) + } + return out } func (b boxPageAgents) Home() string { diff --git a/go/internal/api/api_agent_reads.go b/go/internal/api/api_agent_reads.go index 5931f652b..d96e59763 100644 --- a/go/internal/api/api_agent_reads.go +++ b/go/internal/api/api_agent_reads.go @@ -122,7 +122,7 @@ func (s *Server) serveAgentHTTP(w http.ResponseWriter, r *http.Request) { } facts := a.Facts() facts.Widen = false - writeJSON(w, 200, agentsPage{AgentList: protocol.AgentList{Agents: nonNil(a.Agents()), Requests: nonNil(a.Requests()), Keys: nonNil(a.ApprovalKeys())}, Box: facts, Registry: registryForPage()}) + writeJSON(w, 200, agentsPage{Agents: nonNil(a.Agents()), Requests: nonNil(a.Requests()), Keys: nonNil(a.ApprovalKeys()), Box: facts, Registry: registryForPage()}) case "/api/cloud", "/api/cloud/preview": link, ok := s.cloudLink(w) if !ok { diff --git a/go/internal/api/api_agents.go b/go/internal/api/api_agents.go index bf7b0f005..b2cdba8c5 100644 --- a/go/internal/api/api_agents.go +++ b/go/internal/api/api_agents.go @@ -32,7 +32,7 @@ import ( // for the audit log: "box" for the box itself, or "lan:
". type AgentAdmin interface { Agents() []protocol.AgentRow - Requests() []protocol.Request + Requests() []AgentRequest ApprovalKeys() []protocol.ApprovalKeyInfo // Home is this box's home id at FTW Cloud, or "" before it links. Home() string @@ -57,6 +57,17 @@ type AgentAdmin interface { Facts() AgentFacts } +// AgentRequest is a request as the box page lists it: the owner's view, +// with the box's own ids. +type AgentRequest struct { + protocol.Request + // Names maps each arg that names a charger or car the box still knows + // to what the box calls it. The page is local and its caller an owner, + // so it may name them; nothing an agent or FTW Cloud reads carries + // this. + Names map[string]string `json:"names,omitempty"` +} + // AgentFacts say whether this box can take a grant now, and where the owner's // browser looks a code up. type AgentFacts struct { @@ -175,12 +186,15 @@ func agentError(w http.ResponseWriter, err error) { writeJSON(w, status, map[string]any{"error": r.Code, "args": r.Args}) } -// agentsPage is agent.list as the box page reads it, with the box's facts and -// the registry's view of agent ops, so the page names nothing by hand. +// agentsPage is agent.list as the box page reads it, its requests with their +// chargers' and cars' names, the box's facts and the registry's view of +// agent ops, so the page names nothing by hand. type agentsPage struct { - protocol.AgentList - Box AgentFacts `json:"box"` - Registry agentsRegistry `json:"registry"` + Agents []protocol.AgentRow `json:"agents"` + Requests []AgentRequest `json:"requests"` + Keys []protocol.ApprovalKeyInfo `json:"keys"` + Box AgentFacts `json:"box"` + Registry agentsRegistry `json:"registry"` } type agentsRegistry struct { @@ -220,9 +234,7 @@ func (s *Server) handleAgentsList(w http.ResponseWriter, r *http.Request) { facts := a.Facts() facts.Widen, _ = lanSecretFrom(r.Context()) writeJSON(w, http.StatusOK, agentsPage{ - AgentList: protocol.AgentList{ - Agents: nonNil(a.Agents()), Requests: nonNil(a.Requests()), Keys: nonNil(a.ApprovalKeys()), - }, + Agents: nonNil(a.Agents()), Requests: nonNil(a.Requests()), Keys: nonNil(a.ApprovalKeys()), Box: facts, Registry: registryForPage(), }) diff --git a/go/internal/api/api_agents_test.go b/go/internal/api/api_agents_test.go index 90f640a13..df119ffe4 100644 --- a/go/internal/api/api_agents_test.go +++ b/go/internal/api/api_agents_test.go @@ -27,11 +27,12 @@ type stubAgents struct { unsynced bool resets int // noKeys is a build that pins no offer key. - noKeys bool + noKeys bool + requests []AgentRequest } func (a *stubAgents) Agents() []protocol.AgentRow { return nil } -func (a *stubAgents) Requests() []protocol.Request { return nil } +func (a *stubAgents) Requests() []AgentRequest { return a.requests } func (a *stubAgents) ApprovalKeys() []protocol.ApprovalKeyInfo { return nil } func (a *stubAgents) Home() string { return a.home } func (a *stubAgents) Wall() (int64, bool) { return 1_000_000, !a.unsynced } @@ -338,6 +339,38 @@ func TestTheListSaysWhetherThisRequestCanWiden(t *testing.T) { } } +// A request on the box page keeps its args and adds the names of the +// chargers and cars they name; one with nothing to name carries no names. +func TestTheAgentListNamesARequestsChargerAndCar(t *testing.T) { + agents := newStubAgents("") + agents.requests = []AgentRequest{ + {Request: protocol.Request{ReqID: "r1", Op: protocol.OpCarGoalSet, State: protocol.RequestPending, + Args: map[string]any{"id": "0123456789abcdef0123456789abcdef", "charger_id": "garage", "target": 0.8}}, + Names: map[string]string{"id": "Volvo", "charger_id": "garage"}}, + {Request: protocol.Request{ReqID: "r2", Op: protocol.OpSetMode, State: protocol.RequestPending, + Args: map[string]any{"mode": "idle"}}}, + } + s := New(&Deps{Agents: agents}) + w := httptest.NewRecorder() + s.handleAgentsList(w, localRequest(http.MethodGet, "/api/agents", "")) + var got struct { + Requests []map[string]any `json:"requests"` + } + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil || len(got.Requests) != 2 { + t.Fatalf("%d %v %s", w.Code, err, w.Body.String()) + } + first := got.Requests[0] + args, _ := first["args"].(map[string]any) + names, _ := first["names"].(map[string]any) + if first["reqId"] != "r1" || args["id"] != "0123456789abcdef0123456789abcdef" || names["id"] != "Volvo" || + names["charger_id"] != "garage" { + t.Errorf("a car's goal on the box page: %v", first) + } + if _, ok := got.Requests[1]["names"]; ok { + t.Errorf("a request with nothing to name: %v", got.Requests[1]) + } +} + func TestALostApprovalKeyIsReplacedOnlyWithThePassword(t *testing.T) { agents := newStubAgents("") s := New(&Deps{Agents: agents}) diff --git a/go/internal/appproto/agentargs.go b/go/internal/appproto/agentargs.go index 1d65b9fea..c4d961ac1 100644 --- a/go/internal/appproto/agentargs.go +++ b/go/internal/appproto/agentargs.go @@ -190,6 +190,45 @@ func (h *Handler) resolveLoadpoint(agentID string) (string, bool) { return "", false } +// ArgNames names each charger and car in a request as the owner's view +// holds it, by the box's own id, with what the box calls it: a car by its +// name, a charger by its id, which is its name on the box. An id the box no +// longer knows gets none. Only the box page reads these; answers to agents +// name nothing, since FTW Cloud reads them. +func (h *Handler) ArgNames(req protocol.Request) map[string]string { + var out map[string]string + for _, a := range protocol.RegistryAgentArgs[req.Op] { + id, _ := req.Args[a.Name].(string) + if id == "" { + continue + } + var name string + switch a.Kind { + case "loadpoint": + if h.cfg.Loadpoints != nil && h.cfg.Loadpoints.Exists(id) { + name = id + } + case "car": + if h.cfg.Cars != nil { + for _, c := range h.cfg.Cars.Cars() { + if c.ID == id { + name = c.Name + break + } + } + } + } + if name == "" { + continue + } + if out == nil { + out = map[string]string{} + } + out[a.Name] = name + } + return out +} + // holdRules are the hold rules beyond the args: an agent's hold always ends, // and an agent never touches a hold someone else set. func (h *Handler) holdRules(cmd Cmd) *ErrorBody { diff --git a/go/internal/appproto/agents_cars_test.go b/go/internal/appproto/agents_cars_test.go index ceeae9813..a6545aa20 100644 --- a/go/internal/appproto/agents_cars_test.go +++ b/go/internal/appproto/agents_cars_test.go @@ -108,3 +108,53 @@ func TestAnAgentsPlanNamesNoChargerOrCar(t *testing.T) { t.Fatalf("a phone's plan chargers %+v", got) } } + +// The box page names the car and the charger in a request by what the box +// calls them. One the box no longer knows keeps no name, and the args stay +// the box's own ids. +func TestTheBoxPageNamesARequestsCarAndCharger(t *testing.T) { + r := newAgentRig(t) + r.cars = newFakeCars() + row, _ := r.grant(apiauth.RoleAgent, []string{protocol.ScopeGoalsRequest}, + []string{OpCarGoalSet, OpCarSoCSet}, protocol.Caps{PerMin: 6}, time.Hour, protocol.Principal{Client: "Claude"}) + h, rec := r.agentSession(row) + deliver(t, h, protocol.MsgAgentRequest, ptrU32(1), protocol.AgentRequest{Op: OpCarGoalSet, + Args: map[string]any{"id": carOf("c2"), "target": 0.7, "ready_by_min": int64(420)}}) + deliver(t, h, protocol.MsgAgentRequest, ptrU32(2), protocol.AgentRequest{Op: OpCarSoCSet, + Args: map[string]any{"charger_id": lp1(), "soc": 0.4}}) + page, _ := r.ownerSession() + if n := len(page.Requests()); n != 2 { + t.Fatalf("requests taken: %d, %s", n, rec.types()) + } + named := func() map[string]map[string]string { + out := map[string]map[string]string{} + for _, req := range page.Requests() { + out[req.Op] = page.ArgNames(req) + } + return out + } + got := named() + if len(got[OpCarGoalSet]) != 1 || got[OpCarGoalSet]["id"] != "Volvo" { + t.Errorf("the car: %v", got[OpCarGoalSet]) + } + if len(got[OpCarSoCSet]) != 1 || got[OpCarSoCSet]["charger_id"] != "lp1" { + t.Errorf("the charger: %v", got[OpCarSoCSet]) + } + + // The car and the charger go while the requests wait. + delete(r.cars.cars, "c2") + r.lp.ids["lp1"] = false + for op, names := range named() { + if names != nil { + t.Errorf("%s names what the box no longer knows: %v", op, names) + } + } + for _, req := range page.Requests() { + if id, _ := req.Args["id"].(string); req.Op == OpCarGoalSet && id != "c2" { + t.Errorf("the car's id in the args: %v", req.Args) + } + if id, _ := req.Args["charger_id"].(string); req.Op == OpCarSoCSet && id != "lp1" { + t.Errorf("the charger's id in the args: %v", req.Args) + } + } +} diff --git a/web/index.html b/web/index.html index a0fe4d7ef..aba81f627 100644 --- a/web/index.html +++ b/web/index.html @@ -1004,7 +1004,7 @@

Price bars (top of the chart)

- + diff --git a/web/settings/tabs/access.js b/web/settings/tabs/access.js index 6758cad17..d4738d5a9 100644 --- a/web/settings/tabs/access.js +++ b/web/settings/tabs/access.js @@ -342,16 +342,19 @@ } // A request's arguments, in the registry's order and by the labels it - // gives them, as the box sent them. One the registry does not name comes - // last, by its own name. + // gives them, as the box sent them. A charger or car goes by the name the + // box gave it, and by its id only when the box no longer knows it. One the + // registry does not name comes last, by its own name. function requestArgs(req, registry) { var known = ((registry && registry.agentArgs) || {})[req.op] || []; var args = req.args || {}; + var names = req.names || {}; var out = []; known.forEach(function (a) { - if (Object.prototype.hasOwnProperty.call(args, a.name)) { - out.push({ label: a.label || a.name, value: argValue(args[a.name], a) }); - } + if (!Object.prototype.hasOwnProperty.call(args, a.name)) return; + var named = (a.kind === "loadpoint" || a.kind === "car") && Object.prototype.hasOwnProperty.call(names, a.name); + var name = named ? cleanText(names[a.name], 80) : ""; + out.push({ label: a.label || a.name, value: name || argValue(args[a.name], a) }); }); Object.keys(args).sort().forEach(function (name) { if (!known.some(function (a) { return a.name === name; })) out.push({ label: cleanText(name, 40), value: argValue(args[name], {}) }); diff --git a/web/settings/tabs/access.test.mjs b/web/settings/tabs/access.test.mjs index 619097229..64027e71d 100644 --- a/web/settings/tabs/access.test.mjs +++ b/web/settings/tabs/access.test.mjs @@ -299,6 +299,23 @@ describe("a request waiting for the owner", () => { assert.equal(rows[0].label, "evilgnp"); }); + it("names a charger or car by what the box calls it, and by its id when the box has no name", () => { + const car = P.requestArgs({ + op: "car.goal.set", + args: { id: "0123456789abcdef0123456789abcdef", target: 0.8 }, + names: { id: "Volvo" + String.fromCharCode(0x200b) + " XC40" }, + }, registry); + assert.deepEqual(car.map((r) => r.value), ["Volvo XC40", "80 %"]); + const charger = P.requestArgs({ + op: "loadpoint.hold", + args: { id: "garage", power_w: 7400 }, + names: { id: "Carport Easee", power_w: "not a name" }, + }, registry); + assert.deepEqual(charger.map((r) => r.value), ["Carport Easee", "7.4 kW"]); + const gone = P.requestArgs({ op: "car.goal.set", args: { id: "0123456789abcdef0123456789abcdef" }, names: { id: "" } }, registry); + assert.equal(gone[0].value, "0123456789abcdef0123456789abcdef"); + }); + it("shows power, length and odd numbers plainly", () => { const rows = P.requestArgs({ op: "loadpoint.hold", args: { power_w: 7400, hold_s: 900, id: "lp_1" } }, registry); assert.deepEqual(rows.map((r) => r.value), ["lp_1", "7.4 kW", "15 min"]);