diff --git a/CLAUDE.md b/CLAUDE.md index 2f9493a..cb84b78 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -152,7 +152,8 @@ own camera, same-person similarity p05 0.719 vs 0.620) → `arcface_int8.onnx` the chain. AdaFace slots are wired but empty: drop a converted `adaface_ir50.onnx` in and it is picked up, BGR channel order already handled (see `color_order_for`). The dev machine is -memory-starved (16 GB, often < 1.5 GB free): the 260 MB model fails with +memory-starved (**8 GB**, measured 0.45 GB free with a browser and Docker +open): the 260 MB model fails with "bad allocation"; int8 quantization of it segfaulted (OOM). w600k_mbf comes from InsightFace buffalo_sc; genderage.onnx from buffalo_l. On failure, the encoder retries loading with `ORT_DISABLE_ALL` graph optimization. @@ -2257,3 +2258,57 @@ Verified against the real office camera with no object storage configured: a 90,587-byte frame stored in Postgres, served as `image/jpeg` to a signed-in user, **401 without a session**, and rendered on both the Cameras and Shops cards. + +## Claiming a headless PC, and telling a refused broker from an absent one + +Both found by the local stack falling over on a memory-starved machine and +needing to be brought back — the kind of thing that only surfaces when the +software is operated rather than written. + +### `behavision-agent claim ` + +The desktop app has had a Setup screen since enrolment was built. The **headless +agent had nothing**: `Bootstrap` lived only in `desktop/internal/cloud`, so the +one configuration the agent binary exists for — a back-office PC with no window +— could not be claimed at all. The only route was hand-editing `agent.json`, +which is exactly the state that Setup screen was built to end. + +`agent/pkg/enrol` is that call, and the CLI joins its arguments rather than +demanding quotes: the code is printed in groups so it can be read aloud, and an +operator pasting it will paste the spaces too. It clears `Standalone`, and a +config that fails to save is **reported** — a claim that is not on disk works +until the next restart and then silently is not claimed, which looks exactly +like a wrong code. + +### A rejected connection and an unreachable broker are not the same fault + +Measured, on the real stack: after the site's broker password was re-rolled, +mosquitto logged `not authorised` while the agent logged **`connect to +tcp://... timed out`**. Those need opposite actions — re-link this PC, or go and +look at the network — and paho's `SetConnectRetry` is why they collapse into +one: it retries internally, so the connect token never completes and *every* +failure arrives as a timeout. + +`describeStall` asks the one question that separates them: can a TCP socket be +opened to the broker at all? Reachable-but-not-accepted names the likely cause +and the command to fix it; unreachable says to check the network. It does not +claim to know the exact reason — the broker does not tell a rejected client why, +and a TLS failure looks the same from here — so it reports what is known rather +than guessing. Same rule as `artifact` vs `no_faces` in the commissioning +verdicts, and the `connected` pointer being three states rather than two. + +`brokerHostPort` parses with `net/url`, never by scanning for the first `:` — +this package has already been bitten once by IPv6 literals being bracketed and +full of them. + +### The dev machine is 8 GB, not 16 + +Corrected in this file, because it feeds a real decision. Measured while the +stack was up: **0.45 GB free** with a browser and Docker Desktop open, and +Docker alone is allocated 4 GB of the 8. The engine's steady state is only +~260 MB, so the OOM kill happened during a build (npm + go + Docker at once), +not in normal running — but the margin is what makes `/api/health` reporting +`recognition_model` worth checking after every restart. The local gallery +already holds **17 embeddings tagged `w600k_mbf` and 19 tagged `w600k_r50`**: +proof that the fallback has silently fired before, and that model-tagging is +what stopped it corrupting anything. diff --git a/RUN.md b/RUN.md index a4ae43c..0d55c0f 100644 --- a/RUN.md +++ b/RUN.md @@ -251,7 +251,7 @@ the frozen engine once to prove it runs, and compiles the installer. 3. Launch from the Start menu. **Set this PC up on its own** — no code needed. 4. Cameras → Add camera → pick the make → Test connection → Save. The feed must appear with no restart. -5. Check `/api/health` reports `recognition_model`. On a 16 GB machine the +5. Check `/api/health` reports `recognition_model`. On a small machine the 166 MB r50 can lose the fallback chain to the 13 MB mbf, and embeddings are model-tagged, so which one wins decides whether a gallery carries over. 6. Sign out of the tray (Quit) — recognition must stop with it. Reboot; the app diff --git a/agent/main.go b/agent/main.go index ff65af6..928e1c7 100644 --- a/agent/main.go +++ b/agent/main.go @@ -29,6 +29,7 @@ import ( "github.com/loyaly/behavision-agent/pkg/cameras" "github.com/loyaly/behavision-agent/pkg/config" "github.com/loyaly/behavision-agent/pkg/engine" + "github.com/loyaly/behavision-agent/pkg/enrol" "github.com/loyaly/behavision-agent/pkg/mqtt" "github.com/loyaly/behavision-agent/pkg/paths" "github.com/loyaly/behavision-agent/pkg/spool" @@ -38,8 +39,15 @@ var version = "dev" func main() { flag.Usage = func() { - fmt.Fprintf(os.Stderr, "behavision-agent %s\n\nusage: %s \n", - version, filepath.Base(os.Args[0])) + fmt.Fprintf(os.Stderr, `behavision-agent %s + +usage: %s + + run supervise the engine and report to head office (default) + claim link this PC to a shop, using an installation code + status what this PC is and whether it is claimed + paths where this install reads and writes +`, version, filepath.Base(os.Args[0])) } flag.Parse() @@ -53,6 +61,8 @@ func main() { err = cmdRun() case "status": err = cmdStatus() + case "claim": + err = cmdClaim(flag.Args()[1:]) case "paths": err = cmdPaths() default: @@ -64,6 +74,67 @@ func main() { } } +// cmdClaim is the headless half of onboarding. +// +// The desktop app has had a Setup screen for this; a back-office PC with no +// window had nothing at all, so the only way to claim one was to hand-edit +// agent.json - which is the state that screen was built to end. +func cmdClaim(args []string) error { + if len(args) == 0 { + return fmt.Errorf("usage: behavision-agent claim \n" + + "Ask whoever manages your shops for one - they can create it from\n" + + "the Behavision platform, under the shop.") + } + // Joined rather than requiring quotes: the code is printed in groups for + // reading aloud, and an operator pasting it will paste the spaces too. + code := strings.Join(args, "") + + if err := paths.EnsureState(); err != nil { + return err + } + cfg, err := config.Load(paths.AgentConfig()) + if err != nil { + return err + } + base := cfg.CloudBase + if v := os.Getenv("BEHAVISION_CLOUD"); v != "" { + base = v + } + if base == "" { + base = "https://mcp.loyaly.ai" + } + + b, err := enrol.Claim(context.Background(), base, code) + if err != nil { + return err + } + + // The slugs, not the uuids: the topic prefix is . and the + // broker's ACL is written against exactly that username. + cfg.ClientID = b.ClientSlug + cfg.SiteID = b.SiteSlug + cfg.SiteName = b.SiteName + cfg.BrokerURL = b.MQTTURL + cfg.BrokerUsername = b.MQTTUser + cfg.BrokerPassword = b.MQTTPass + cfg.AgentToken = b.AgentToken + cfg.CloudBase = base + // A PC that was running on its own and has now been linked is no longer + // standalone. + cfg.Standalone = false + if err := cfg.Save(paths.AgentConfig()); err != nil { + // Reported, never swallowed: a claim that is not on disk works until + // the next restart and then silently is not claimed any more, which + // looks exactly like a wrong code. + return fmt.Errorf("could not save the settings: %w", err) + } + + fmt.Printf("linked to %s (%s.%s)\n", b.SiteName, b.ClientSlug, b.SiteSlug) + fmt.Printf("settings written to %s\n", paths.AgentConfig()) + fmt.Println("restart the agent for it to take effect.") + return nil +} + func cmdPaths() error { return json.NewEncoder(os.Stdout).Encode(map[string]string{ "version": version, diff --git a/agent/pkg/enrol/enrol.go b/agent/pkg/enrol/enrol.go new file mode 100644 index 0000000..4d944e6 --- /dev/null +++ b/agent/pkg/enrol/enrol.go @@ -0,0 +1,92 @@ +// Package enrol links a PC to a shop, using the one-shot code an operator is +// given. +// +// It existed only inside the desktop app, which meant a HEADLESS install - a +// back-office PC with no window, the configuration the agent binary is for - +// could not be claimed at all. The only route was hand-editing agent.json, +// which is exactly the state the desktop's Setup screen was built to end. +// +// The endpoint behind this is deliberately unauthenticated: the PC doing it has +// nobody signed in yet, and requiring a login would mean shipping a password to +// every shop that installs the software. +package enrol + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + "time" +) + +// Bootstrap is what the server hands back: which shop this PC is, and the +// credentials it needs to say so. +type Bootstrap struct { + ClientSlug string `json:"client_slug"` + SiteSlug string `json:"site_slug"` + SiteName string `json:"site_name"` + MQTTURL string `json:"mqtt_url"` + MQTTUser string `json:"mqtt_username"` + MQTTPass string `json:"mqtt_password"` + CAPem string `json:"ca_pem,omitempty"` + AgentToken string `json:"agent_token"` +} + +// Claim redeems an installation code. +// +// The code is read aloud down a phone and photographed off screens, so what is +// typed here can be as untidy as it needs to be: the server strips spaces, +// dashes and case at its end. Sending it as typed keeps ONE implementation of +// that normalisation, on the side that also issued the code - two would +// eventually disagree and hash to something the redeemer never produces. +func Claim(ctx context.Context, base, code string) (Bootstrap, error) { + var out Bootstrap + base = strings.TrimRight(base, "/") + if base == "" { + return out, fmt.Errorf("no server address configured (set cloud_base or BEHAVISION_CLOUD)") + } + body, err := json.Marshal(map[string]string{"site_token": code}) + if err != nil { + return out, err + } + ctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, + base+"/api/agent/enrol", bytes.NewReader(body)) + if err != nil { + return out, err + } + req.Header.Set("Content-Type", "application/json") + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return out, fmt.Errorf("could not reach %s: %w", base, err) + } + defer resp.Body.Close() + blob, _ := io.ReadAll(io.LimitReader(resp.Body, 64<<10)) + if resp.StatusCode != http.StatusOK { + // The server answers unknown, expired and already-used identically on + // purpose - the difference only helps somebody guessing codes, and the + // operator's next step is the same in all three cases. Its own words + // are passed through rather than reworded here. + var e struct { + Message string `json:"message"` + } + _ = json.Unmarshal(blob, &e) + if e.Message != "" { + return out, fmt.Errorf("%s", e.Message) + } + return out, fmt.Errorf("head office: %s", resp.Status) + } + if err := json.Unmarshal(blob, &out); err != nil { + return out, err + } + if out.SiteSlug == "" || out.MQTTURL == "" { + return out, fmt.Errorf("head office returned an incomplete setup") + } + return out, nil +} diff --git a/agent/pkg/mqtt/client.go b/agent/pkg/mqtt/client.go index 356e168..45cf7be 100644 --- a/agent/pkg/mqtt/client.go +++ b/agent/pkg/mqtt/client.go @@ -16,6 +16,8 @@ import ( "errors" "fmt" "log" + "net" + "net/url" neturl "net/url" "os" "strings" @@ -104,7 +106,13 @@ func NewClient(opts ClientOptions) (*Client, error) { tok := c.client.Connect() if !tok.WaitTimeout(20 * time.Second) { - return c, fmt.Errorf("mqtt: connect to %s timed out", opts.BrokerURL) + // SetConnectRetry means paho retries internally and this token never + // completes, so a REFUSED connection and an UNREACHABLE broker both + // arrive here as a timeout. They need opposite actions - re-link this + // PC, or go and look at the network - and reporting both as "timed + // out" sent the diagnosis to the wrong place. Measured: mosquitto + // logged "not authorised" while the agent logged a timeout. + return c, fmt.Errorf("mqtt: %s", describeStall(opts.BrokerURL)) } if err := tok.Error(); err != nil { return c, fmt.Errorf("mqtt: connect to %s: %w", opts.BrokerURL, err) @@ -112,6 +120,47 @@ func NewClient(opts ClientOptions) (*Client, error) { return c, nil } +// describeStall says which of the two failures this is, by asking the one +// question that separates them: can we open a socket to the broker at all? +// +// It cannot name the exact reason - the broker does not tell a rejected client +// why, and a TLS failure looks the same from here - so it says what is known +// and what to check, rather than guessing. Being reachable but not accepted is +// overwhelmingly a credential this PC no longer has, which is what happens when +// a site is re-provisioned. +func describeStall(brokerURL string) string { + host := brokerHostPort(brokerURL) + if host == "" { + return fmt.Sprintf("connect to %s timed out", brokerURL) + } + conn, err := net.DialTimeout("tcp", host, 5*time.Second) + if err != nil { + return fmt.Sprintf("cannot reach the broker at %s: %v - check the "+ + "network and that the broker is running", host, err) + } + _ = conn.Close() + return fmt.Sprintf("the broker at %s is reachable but did not accept this "+ + "PC - usually its credentials are no longer valid; re-link it with "+ + "`behavision-agent claim `", host) +} + +// brokerHostPort extracts host:port for the reachability probe. Parsed with +// net/url, never by scanning for the first ":" - an IPv6 literal is bracketed +// and full of them. +func brokerHostPort(brokerURL string) string { + u, err := url.Parse(brokerURL) + if err != nil || u.Host == "" { + return "" + } + if u.Port() != "" { + return u.Host + } + if strings.HasPrefix(brokerURL, "tls://") || strings.HasPrefix(brokerURL, "ssl://") { + return net.JoinHostPort(u.Hostname(), "8883") + } + return net.JoinHostPort(u.Hostname(), "1883") +} + // Publish sends one message at QoS 1 and waits for the broker's PUBACK. // // QoS 1, not 0 or 2. At QoS 0 the broker never confirms, so the pump would ack diff --git a/agent/pkg/mqtt/client_test.go b/agent/pkg/mqtt/client_test.go index c112197..fc1f616 100644 --- a/agent/pkg/mqtt/client_test.go +++ b/agent/pkg/mqtt/client_test.go @@ -1,6 +1,7 @@ package mqtt import ( + "net" "strings" "testing" ) @@ -102,3 +103,66 @@ func TestPublishOnADeadClientErrorsRatherThanPanics(t *testing.T) { func writeFile(path, content string) error { return osWriteFile(path, []byte(content), 0o600) } + +// "The broker refused this PC" and "the broker is not there" need opposite +// actions - re-link this PC, or go and look at the network - and paho's +// connect-retry makes both arrive as a timeout. Measured on a real broker: +// mosquitto logged "not authorised" while the agent logged a timeout, which +// sent the diagnosis to the wrong place. +func TestARefusedBrokerIsNotDescribedAsUnreachable(t *testing.T) { + // A listener that accepts TCP and then says nothing is exactly what a + // broker rejecting a client looks like from out here. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer ln.Close() + go func() { + for { + c, err := ln.Accept() + if err != nil { + return + } + _ = c + } + }() + + got := describeStall("tcp://" + ln.Addr().String()) + if !strings.Contains(got, "reachable but did not accept") { + t.Fatalf("a reachable broker was described as unreachable: %s", got) + } + if !strings.Contains(got, "claim") { + t.Errorf("the message does not say what to do about it: %s", got) + } +} + +func TestAnAbsentBrokerIsDescribedAsUnreachable(t *testing.T) { + // Bound and immediately closed, so the port is certainly nobody's. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + addr := ln.Addr().String() + ln.Close() + + got := describeStall("tcp://" + addr) + if !strings.Contains(got, "cannot reach the broker") { + t.Fatalf("an absent broker was not described as unreachable: %s", got) + } +} + +// An IPv6 literal is bracketed and full of colons, so scanning for the first +// one gives "[". The same bug this package already fixed once for broker URLs. +func TestTheProbeAddressHandlesIPv6AndDefaultPorts(t *testing.T) { + for _, tc := range []struct{ in, want string }{ + {"tcp://127.0.0.1:51883", "127.0.0.1:51883"}, + {"tcp://[::1]:1883", "[::1]:1883"}, + {"tcp://broker.example", "broker.example:1883"}, + {"tls://broker.example", "broker.example:8883"}, + {"tls://[2001:db8::1]:8884", "[2001:db8::1]:8884"}, + } { + if got := brokerHostPort(tc.in); got != tc.want { + t.Errorf("brokerHostPort(%q) = %q, want %q", tc.in, got, tc.want) + } + } +}