From 6026378e9dcf76faff82043cb8a846e40da880b1 Mon Sep 17 00:00:00 2001 From: Jonathon Leight Date: Mon, 6 Jul 2026 19:13:35 -0400 Subject: [PATCH] Add region command placeholder --- internal/core/config_profile.go | 29 +++++- internal/core/config_profile_test.go | 30 ++++++ internal/core/console_config.go | 20 ++-- internal/core/console_config_test.go | 76 ++++++++++++++- internal/core/repeater_setup.go | 96 ++++++++++--------- internal/core/repeater_setup_test.go | 45 +++++++++ .../core/templates/config_profile_edit.html | 2 +- internal/store/config_profiles.go | 32 +++++++ internal/store/config_profiles_test.go | 36 +++++++ 9 files changed, 309 insertions(+), 57 deletions(-) diff --git a/internal/core/config_profile.go b/internal/core/config_profile.go index 31cd992..0f98a2e 100644 --- a/internal/core/config_profile.go +++ b/internal/core/config_profile.go @@ -426,6 +426,7 @@ func regionGeofence(zv configRegionView, name string, errs *[]string) ([]byte, b // errs and risky commands in risky. It returns view steps (with rendered fields) // and the store steps to persist. func parseConfigSteps(text string, catalog []*store.Command, label string, errs, risky *[]string) (view, persist []store.ConfigStep) { + markerSeen := false for _, raw := range strings.Split(text, "\n") { line := strings.TrimSpace(strings.TrimRight(raw, "\r")) if line == "" { @@ -438,6 +439,17 @@ func parseConfigSteps(text string, catalog []*store.Command, label string, errs, persist = append(persist, step) continue } + if isRegionMarkerLine(line) { + if markerSeen { + *errs = append(*errs, fmt.Sprintf("%s: the %s placeholder can only appear once.", label, store.RegionMarker)) + continue + } + markerSeen = true + step := store.ConfigStep{CommandLine: store.RegionMarker} + view = append(view, step) + persist = append(persist, step) + continue + } if !validCommandText(line) { *errs = append(*errs, fmt.Sprintf("%s: invalid command %q.", label, line)) continue @@ -458,8 +470,23 @@ func parseConfigSteps(text string, catalog []*store.Command, label string, errs, return view, persist } +// isRegionMarkerLine reports whether a profile line is the region placeholder, +// tolerating internal spacing and case (e.g. "{{region}}", "{{ REGION }}"). The +// canonical stored form is store.RegionMarker. +func isRegionMarkerLine(line string) bool { + inner, ok := strings.CutPrefix(line, "{{") + if !ok { + return false + } + inner, ok = strings.CutSuffix(inner, "}}") + if !ok { + return false + } + return strings.EqualFold(strings.TrimSpace(inner), "region") +} + // stepsToText renders stored steps back into editable textarea content: commands -// as-is, comment steps prefixed with "# ". +// (and the region marker) as-is, comment steps prefixed with "# ". func stepsToText(steps []store.ConfigStep) string { var b strings.Builder for _, s := range steps { diff --git a/internal/core/config_profile_test.go b/internal/core/config_profile_test.go index fd0d294..4662cb9 100644 --- a/internal/core/config_profile_test.go +++ b/internal/core/config_profile_test.go @@ -51,6 +51,36 @@ func TestParseConfigSteps(t *testing.T) { } } +func TestParseConfigStepsRegionMarker(t *testing.T) { + t.Parallel() + catalog := configCatalog() + + // A marker (in flexible spelling) becomes one marker step that canonicalizes + // and round-trips; it is neither a comment nor a runnable command. + var errs, risky []string + _, persist := parseConfigSteps("set tx 22\n{{region}}\nset tx 23", catalog, "base", &errs, &risky) + if len(errs) != 0 { + t.Fatalf("errs = %v, want none", errs) + } + if len(persist) != 3 || !persist[1].IsRegionMarker() { + t.Fatalf("steps = %+v, want a region marker at index 1", persist) + } + if persist[1].CommandLine != store.RegionMarker { + t.Fatalf("marker canonical form = %q, want %q", persist[1].CommandLine, store.RegionMarker) + } + if got := stepsToText(persist); !strings.Contains(got, store.RegionMarker) { + t.Fatalf("round-trip lost the marker: %q", got) + } + + // A second marker is rejected. + errs, risky = nil, nil + _, _ = parseConfigSteps("{{ region }}\n{{ REGION }}", catalog, "base", &errs, &risky) + if len(errs) != 1 || !strings.Contains(errs[0], "once") { + t.Fatalf("errs = %v, want one 'only once' error", errs) + } + _ = risky +} + func TestRegionGeofence(t *testing.T) { t.Parallel() diff --git a/internal/core/console_config.go b/internal/core/console_config.go index 019dd5d..d9628b1 100644 --- a/internal/core/console_config.go +++ b/internal/core/console_config.go @@ -27,7 +27,6 @@ type consoleConfig struct { type consoleConfigCommand struct { Line string `json:"line"` // CLI text; empty for a comment-only note Comment string `json:"comment,omitempty"` // optional note text - Kind string `json:"kind"` // "profile" | "region" Runnable bool `json:"runnable"` Reason string `json:"reason,omitempty"` // why it isn't runnable } @@ -117,9 +116,12 @@ func (s *Handlers) consoleConfigJSON(w http.ResponseWriter, r *http.Request) { return true, "" } - // Profile base settings (verbatim; comment-only steps are notes, not commands). - for _, step := range cv.SelectedSteps { - c := consoleConfigCommand{Kind: "profile", Line: step.CommandLine, Comment: step.Comment} + // Profile base settings (verbatim; comment-only steps are notes, not commands), + // with the location-derived region commands spliced at the profile's + // {{ region }} marker or appended after all steps when it has none. The marker + // itself is dropped by SplitAtRegionMarker. + emitStep := func(step store.ConfigStep) { + c := consoleConfigCommand{Line: step.CommandLine, Comment: step.Comment} if step.IsComment() { c.Reason = "note" } else { @@ -127,12 +129,18 @@ func (s *Handlers) consoleConfigJSON(w http.ResponseWriter, r *http.Request) { } resp.Commands = append(resp.Commands, c) } - // Region commands derived from the location. + before, after := store.SplitAtRegionMarker(cv.SelectedSteps) + for _, step := range before { + emitStep(step) + } for _, line := range cv.RegionDef { - c := consoleConfigCommand{Kind: "region", Line: line} + c := consoleConfigCommand{Line: line} c.Runnable, c.Reason = runnable(line) resp.Commands = append(resp.Commands, c) } + for _, step := range after { + emitStep(step) + } resp.Location = consoleConfigLocation{ Known: lat != nil && lon != nil, diff --git a/internal/core/console_config_test.go b/internal/core/console_config_test.go index fb0bcb5..0e6cd22 100644 --- a/internal/core/console_config_test.go +++ b/internal/core/console_config_test.go @@ -29,6 +29,11 @@ func cmdIDByKey(t *testing.T, st *store.Store, key string) int64 { return 0 } +// isRegionLine reports whether a recommended-config line came from the region +// commands (every one starts with "region "), used by tests to tell region lines +// apart from profile steps now that the payload carries no per-line kind. +func isRegionLine(line string) bool { return strings.HasPrefix(line, "region ") } + func getConsoleConfig(t *testing.T, ts *httptest.Server, host, path string, cookie *http.Cookie) consoleConfig { t.Helper() resp := do(t, ts, host, path, cookie) @@ -101,7 +106,7 @@ func TestConsoleConfigJSON(t *testing.T) { var regionLines, noteCount int for _, c := range cc.Commands { byLine[c.Line] = c - if c.Kind == "region" { + if isRegionLine(c.Line) { regionLines++ } if c.Line == "" && c.Comment != "" { @@ -109,8 +114,8 @@ func TestConsoleConfigJSON(t *testing.T) { } } // Profile command + the note both appear. - if cmd, ok := byLine["set tx 22"]; !ok || cmd.Kind != "profile" || !cmd.Runnable { - t.Fatalf("set tx 22 = %+v, want profile+runnable", cmd) + if cmd, ok := byLine["set tx 22"]; !ok || !cmd.Runnable { + t.Fatalf("set tx 22 = %+v, want present + runnable", cmd) } if noteCount != 1 { t.Fatalf("note count = %d, want 1 (the comment step)", noteCount) @@ -120,7 +125,7 @@ func TestConsoleConfigJSON(t *testing.T) { t.Fatal("no region commands emitted for a covered location") } for _, c := range cc.Commands { - if c.Kind == "region" && !c.Runnable { + if isRegionLine(c.Line) && !c.Runnable { t.Fatalf("region line %q not runnable for owner", c.Line) } } @@ -138,7 +143,7 @@ func TestConsoleConfigJSON(t *testing.T) { switch { case c.Line == "set tx 22" && !c.Runnable: t.Error("member should be allowed to run member-tier set tx 22") - case c.Kind == "region": + case isRegionLine(c.Line): sawRegion = true if c.Runnable { t.Errorf("member should NOT be allowed to run admin-tier %q", c.Line) @@ -153,6 +158,67 @@ func TestConsoleConfigJSON(t *testing.T) { } } +// TestConsoleConfigRegionMarkerInline covers a profile with a {{ region }} marker: +// the region commands are spliced at the marker (between the surrounding steps), +// the marker itself is never emitted as a command. +func TestConsoleConfigRegionMarkerInline(t *testing.T) { + t.Parallel() + st, ctx, ts, h := splitServer(t) + + owner, ownerCookie := appLogin(t, ts, st, ctx, h.app, "cc-marker") + rep, err := st.CreateRepeater(ctx, &store.Repeater{ + OwnerID: owner.ID, Name: "R", PublicKeyHex: strings.Repeat("d", 64), + RadioFreqHz: 1, RadioBwHz: 1, RadioSF: 11, RadioCR: 5, + }) + if err != nil { + t.Fatal(err) + } + org, err := st.CreateOrg(ctx, "MarkerOrg", owner.ID) + if err != nil { + t.Fatal(err) + } + profiles := []store.ProfileInput{{Name: "P", Steps: []store.ConfigStep{ + {CommandLine: "set tx 22"}, + {CommandLine: store.RegionMarker}, + {CommandLine: "set tx 23"}, + }}} + regions := []store.RegionInput{{ + Token: "buf", DisplayName: "Buffalo", Layer: 0, AllowFlood: true, + GeofenceJSON: geo.Rectangle(40, -80, 45, -75), + }} + if err := st.ReplaceOrgConfig(ctx, org.ID, profiles, regions); err != nil { + t.Fatal(err) + } + + cc := getConsoleConfig(t, ts, h.app, "/repeaters/"+rep.PublicID+"/config.json?lat=42&lon=-78", ownerCookie) + + idx := func(line string) int { + for i, c := range cc.Commands { + if c.Line == line { + return i + } + } + return -1 + } + pre, post := idx("set tx 22"), idx("set tx 23") + region := -1 + for i, c := range cc.Commands { + if isRegionLine(c.Line) { + region = i + break + } + } + if pre < 0 || post < 0 || region < 0 { + t.Fatalf("missing lines: pre=%d region=%d post=%d\n%+v", pre, region, post, cc.Commands) + } + if pre >= region || region >= post { + t.Fatalf("region not spliced between steps: pre=%d region=%d post=%d", pre, region, post) + } + if idx(store.RegionMarker) != -1 { + t.Fatalf("marker leaked into commands:\n%+v", cc.Commands) + } +} + // TestSetRepeaterLocation covers the map-pick persistence endpoint used when the // repeater has no location: valid coordinates save and surface via config.json; // invalid coordinates are rejected. diff --git a/internal/core/repeater_setup.go b/internal/core/repeater_setup.go index d805430..ed74003 100644 --- a/internal/core/repeater_setup.go +++ b/internal/core/repeater_setup.go @@ -62,20 +62,18 @@ func (s *Handlers) handleSetupCommands(w http.ResponseWriter, r *http.Request) { } req.Name = name - var cmds []string - cmds = append(cmds, "set name "+req.Name) - // Resolve the authoritative radio settings (echoed back so the client can - // store them on the repeater record). Radio + region come from the chosen org - // config; a standalone repeater (no org) uses the radio preset and gets no - // region hierarchy. + // store them on the repeater record) and gather the org's profile + region + // commands. Radio + region come from the chosen org config; a standalone + // repeater (no org) uses the request's radio and gets no region hierarchy. radio := setupRadio{FreqMHz: req.FreqMHz, BwKHz: req.BwKHz, SF: req.SF, CR: req.CR} + var steps []store.ConfigStep + var regionCmds []string if req.OrgID != 0 { if _, isMember, err := s.Store.OrgRole(r.Context(), req.OrgID, uid); err != nil || !isMember { http.Error(w, "no access to that organization", http.StatusForbidden) return } - var steps []string if req.Profile != "" { s2, err := s.profileSteps(r.Context(), req.OrgID, req.Profile) if err != nil { @@ -84,15 +82,13 @@ func (s *Handlers) handleSetupCommands(w http.ResponseWriter, r *http.Request) { } steps = s2 } - cmds = append(cmds, steps...) // Prefer the radio the profile sets on the device; if it sets none, fall - // back to the default preset and add the command so the device still ends - // up tunable (and the record radio is accurate). - if rad, ok := parseProfileRadio(steps); ok { + // back to the default preset so the device still ends up tunable (and the + // record radio is accurate). + if rad, ok := parseProfileRadio(profileCommandLines(steps)); ok { radio = rad } else { radio = defaultSetupRadio() - cmds = append(cmds, radioCommand(radio)) } regions, err := s.Store.ListRegions(r.Context(), req.OrgID) if err != nil { @@ -104,27 +100,13 @@ func (s *Handlers) handleSetupCommands(w http.ResponseWriter, r *http.Request) { s.ServerError(w, r, "could not load regions", err) return } - cmds = append(cmds, store.RegionDefCommands(regions, rootAllow, req.Lat, req.Lon)...) - } else { - if radio.FreqMHz <= 0 || radio.BwKHz <= 0 { - http.Error(w, "radio settings are required", http.StatusBadRequest) - return - } - cmds = append(cmds, radioCommand(radio)) + regionCmds = store.RegionDefCommands(regions, rootAllow, req.Lat, req.Lon) + } else if radio.FreqMHz <= 0 || radio.BwKHz <= 0 { + http.Error(w, "radio settings are required", http.StatusBadRequest) + return } - // Location: only set when the user picked a point. - if req.Lat != nil && req.Lon != nil { - cmds = append(cmds, - "set lat "+strconv.FormatFloat(*req.Lat, 'f', 6, 64), - "set lon "+strconv.FormatFloat(*req.Lon, 'f', 6, 64)) - } - - // Identity (client-spliced), then grant MeshTender admin, then reboot to - // apply the new identity and radio. - cmds = append(cmds, identityPlaceholder) - cmds = append(cmds, s.Identity.SetPermCommand()) - cmds = append(cmds, "reboot") + cmds := buildSetupCommands(req.Name, identityPlaceholder, s.Identity.SetPermCommand(), req.Lat, req.Lon, radio, steps, regionCmds) w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(setupCommandsResponse{ @@ -197,25 +179,51 @@ func radioCommand(rad setupRadio) string { strconv.Itoa(rad.SF) + "," + strconv.Itoa(rad.CR) } -// profileSteps returns the runnable command lines of a named profile in an org -// (comment-only steps are skipped). -func (s *Handlers) profileSteps(ctx context.Context, orgID int64, name string) ([]string, error) { +// buildSetupCommands assembles the ordered from-scratch USB setup command list: +// the header (name, client-spliced identity, location, radio, admin grant), then +// the profile's steps with the org's region commands spliced at the profile's +// {{ region }} marker (or appended after all steps when it has none), then a +// final reboot to apply the new identity and radio. Comment steps and the marker +// itself are not runnable, so they never reach the device. +func buildSetupCommands(name, identityCmd, setpermCmd string, lat, lon *float64, radio setupRadio, steps []store.ConfigStep, regionCmds []string) []string { + cmds := []string{"set name " + name, identityCmd} + if lat != nil && lon != nil { + cmds = append(cmds, + "set lat "+strconv.FormatFloat(*lat, 'f', 6, 64), + "set lon "+strconv.FormatFloat(*lon, 'f', 6, 64)) + } + cmds = append(cmds, radioCommand(radio), setpermCmd) + before, after := store.SplitAtRegionMarker(steps) + cmds = append(cmds, profileCommandLines(before)...) + cmds = append(cmds, regionCmds...) + cmds = append(cmds, profileCommandLines(after)...) + return append(cmds, "reboot") +} + +// profileCommandLines returns the runnable command lines of a step slice, +// skipping comment steps and the region marker. +func profileCommandLines(steps []store.ConfigStep) []string { + var out []string + for _, st := range steps { + if st.IsComment() || st.IsRegionMarker() { + continue + } + out = append(out, st.CommandLine) + } + return out +} + +// profileSteps returns the ordered steps of a named profile in an org (nil when +// no profile by that name exists). +func (s *Handlers) profileSteps(ctx context.Context, orgID int64, name string) ([]store.ConfigStep, error) { profiles, err := s.Store.ListProfiles(ctx, orgID) if err != nil { return nil, err } for _, p := range profiles { - if p.Name != name { - continue + if p.Name == name { + return p.Steps, nil } - var out []string - for _, st := range p.Steps { - if st.IsComment() { - continue - } - out = append(out, st.CommandLine) - } - return out, nil } return nil, nil } diff --git a/internal/core/repeater_setup_test.go b/internal/core/repeater_setup_test.go index b446bf1..007d632 100644 --- a/internal/core/repeater_setup_test.go +++ b/internal/core/repeater_setup_test.go @@ -5,8 +5,11 @@ import ( "io" "net/http" "net/url" + "slices" "strings" "testing" + + "github.com/jleight/meshtender/internal/store" ) // jsonPost issues a JSON POST with an explicit Host header and cookies. @@ -159,6 +162,48 @@ func TestSerialSetupFlow(t *testing.T) { } } +// TestBuildSetupCommands locks the from-scratch command order (header → profile +// → region → reboot) and the two region-placement modes: appended when the +// profile has no marker, spliced in place when it does. +func TestBuildSetupCommands(t *testing.T) { + t.Parallel() + radio := setupRadio{FreqMHz: 910.525, BwKHz: 62.5, SF: 7, CR: 5} + lat, lon := 15.0, 35.0 + region := []string{"region def zone", "region denyf *", "region allowf zone", "region save"} + header := []string{ + "set name Hilltop", "set prv.key ", + "set lat 15.000000", "set lon 35.000000", + "set radio 910.525,62.5,7,5", "setperm srv 3", + } + + // No marker: region commands land after all profile steps, before reboot. The + // comment step is not runnable and is skipped. + steps := []store.ConfigStep{{CommandLine: "set tx 22"}, {Comment: "note"}} + got := buildSetupCommands("Hilltop", "set prv.key ", "setperm srv 3", &lat, &lon, radio, steps, region) + want := append(append(append([]string{}, header...), "set tx 22"), append(append([]string{}, region...), "reboot")...) + if !slices.Equal(got, want) { + t.Fatalf("no-marker order:\n got %v\nwant %v", got, want) + } + + // With a marker: region commands splice between the surrounding steps. + steps = []store.ConfigStep{{CommandLine: "set tx 22"}, {CommandLine: store.RegionMarker}, {CommandLine: "set repeat on"}} + got = buildSetupCommands("Hilltop", "set prv.key ", "setperm srv 3", &lat, &lon, radio, steps, region) + want = append(append([]string{}, header...), "set tx 22") + want = append(want, region...) + want = append(want, "set repeat on", "reboot") + if !slices.Equal(got, want) { + t.Fatalf("marker-splice order:\n got %v\nwant %v", got, want) + } + + // No location + empty region (standalone): header skips lat/lon and nothing is + // spliced. + got = buildSetupCommands("Solo", "set prv.key ", "setperm srv 3", nil, nil, radio, nil, nil) + want = []string{"set name Solo", "set prv.key ", "set radio 910.525,62.5,7,5", "setperm srv 3", "reboot"} + if !slices.Equal(got, want) { + t.Fatalf("standalone order:\n got %v\nwant %v", got, want) + } +} + func TestParseProfileRadio(t *testing.T) { steps := []string{"set tx 22", "set radio 910.525,62.5,7,5", "set repeat on"} rad, ok := parseProfileRadio(steps) diff --git a/internal/core/templates/config_profile_edit.html b/internal/core/templates/config_profile_edit.html index a8958dd..77f0544 100644 --- a/internal/core/templates/config_profile_edit.html +++ b/internal/core/templates/config_profile_edit.html @@ -37,7 +37,7 @@
- One command per line; lines starting with # are notes. + One command per line; lines starting with # are notes. Put {{"{{ region }}"}} on its own line to insert this org's region settings there — otherwise they run after all steps.
diff --git a/internal/store/config_profiles.go b/internal/store/config_profiles.go index f0f65c2..225052b 100644 --- a/internal/store/config_profiles.go +++ b/internal/store/config_profiles.go @@ -32,6 +32,38 @@ type ConfigStep struct { // IsComment reports whether the step is a note rather than a runnable command. func (s ConfigStep) IsComment() bool { return s.CommandLine == "" } +// RegionMarker is the placeholder a profile step can hold on its own line to +// control where an org's region commands are spliced into the profile. Without +// it, region commands are appended after all profile steps (the default). It is +// never sent to a device — SplitAtRegionMarker removes it before either assembly +// path emits commands. It is stored as a step's CommandLine (CommandID nil), so +// it round-trips through the profile text editor like any other line. +const RegionMarker = "{{ region }}" + +// IsRegionMarker reports whether the step is the region placeholder rather than a +// runnable command or a comment. +func (s ConfigStep) IsRegionMarker() bool { return s.CommandLine == RegionMarker } + +// SplitAtRegionMarker partitions a profile's steps around the region marker: the +// steps before the first marker and the steps after it, with the marker itself +// (and any duplicates) dropped. When no marker is present every step lands in +// before and after is empty — so a caller that emits before, then the region +// commands, then after, appends the region block at the end (the default). +func SplitAtRegionMarker(steps []ConfigStep) (before, after []ConfigStep) { + found := false + for _, st := range steps { + switch { + case st.IsRegionMarker(): + found = true + case found: + after = append(after, st) + default: + before = append(before, st) + } + } + return before, after +} + // Profile is a named set of base-setting steps. type Profile struct { ID int64 diff --git a/internal/store/config_profiles_test.go b/internal/store/config_profiles_test.go index 2ec2931..722154a 100644 --- a/internal/store/config_profiles_test.go +++ b/internal/store/config_profiles_test.go @@ -279,6 +279,42 @@ func TestRegionDefFloodCommands(t *testing.T) { } } +func TestSplitAtRegionMarker(t *testing.T) { + t.Parallel() + cmd := func(line string) ConfigStep { return ConfigStep{CommandLine: line} } + marker := ConfigStep{CommandLine: RegionMarker} + lines := func(steps []ConfigStep) []string { + out := []string{} + for _, s := range steps { + out = append(out, s.CommandLine) + } + return out + } + + cases := []struct { + name string + steps []ConfigStep + before, after []string + }{ + {"middle", []ConfigStep{cmd("a"), marker, cmd("b")}, []string{"a"}, []string{"b"}}, + {"no marker (region appends at end)", []ConfigStep{cmd("a"), cmd("b")}, []string{"a", "b"}, []string{}}, + {"marker first", []ConfigStep{marker, cmd("a")}, []string{}, []string{"a"}}, + {"marker last", []ConfigStep{cmd("a"), marker}, []string{"a"}, []string{}}, + {"duplicate markers dropped", []ConfigStep{cmd("a"), marker, cmd("b"), marker, cmd("c")}, []string{"a"}, []string{"b", "c"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + before, after := SplitAtRegionMarker(tc.steps) + if got := lines(before); !strings.EqualFold(strings.Join(got, ","), strings.Join(tc.before, ",")) { + t.Errorf("before = %v, want %v", got, tc.before) + } + if got := lines(after); !strings.EqualFold(strings.Join(got, ","), strings.Join(tc.after, ",")) { + t.Errorf("after = %v, want %v", got, tc.after) + } + }) + } +} + // TestListRepeaterConfigOrgs covers which orgs surface in the console config // picker: the repeater must participate (owner is a member, not excluded) and the // org must have config. Profile names come back per org (empty for region-only).