Merge pull request 'Say a read-only mesh-cli call in no line of its own, and count them hourly (hq issue 762)' (#341) from fix/762-a-read-call-writes-no-journal-line into main

cmd/mesh-controller/meshcli.go

@@ -7,7 +7,10 @@ import (
 	"fmt"
 	"os/exec"
 	"slices"
+	"sort"
 	"strings"
+	"sync"
+	"time"
 
 	"github.com/novox/mesh-controller/internal/inventory"
 	"github.com/novox/mesh-controller/internal/link"
@@ -123,8 +126,82 @@ func answerMeshCLI(inv *inventory.Inventory) link.CLIHandler {
 // cliJournal says one line in the controller's journal (its standard output); a variable so a test can read it.
 var cliJournal = func(line string) { fmt.Println(line) }
 
-// runForMeshCLI runs a judged line and answers what it said. Every line is said in the journal first: its call,
+// onlyReads says a line only reads, judged by the generic `command` verb's reading forms (novox/hq ADR 0266) — the
-// who asked on which node, its command word only, and how it runs (review of ADR 0272).
+// same allow list that decides what an ordinary call may run, never a second list kept here — judged on the line's
+// every word, so `plans go`, `conditions silence` and `settings set` are not reads while `plans`, `conditions` and
+// `settings show` are. A line those forms do not name is not judged a read, and is said (novox/hq issue 762).
+func onlyReads(asked link.CLIAsked, v cliVerdict) bool {
+	if v.refused != "" || len(asked.Line) == 0 || len(asked.Stdin) > 0 || cliServers[asked.Line[0]] {
+		return false
+	}
+	return commandReads(asked.Line) == nil && terminalOnly(asked.Line) == nil
+}
+
+// readCalls counts the read calls said in no line, per caller, until the next hourly line says them.
+type readCalls struct {
+	mu     sync.Mutex
+	counts map[string]int
+}
+
+// cliReads are this controller's read calls not yet said.
+var cliReads = &readCalls{}
+
+func (r *readCalls) add(caller string) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	if r.counts == nil {
+		r.counts = map[string]int{}
+	}
+	r.counts[caller]++
+}
+
+// said is the line that says the read calls counted since the last, and starts counting again; empty when there
+// were none, so an idle hour says nothing.
+func (r *readCalls) said(over time.Duration) string {
+	r.mu.Lock()
+	counts := r.counts
+	r.counts = nil
+	r.mu.Unlock()
+	if len(counts) == 0 {
+		return ""
+	}
+	callers := make([]string, 0, len(counts))
+	for c := range counts {
+		callers = append(callers, c)
+	}
+	sort.Strings(callers)
+	parts := make([]string, len(callers))
+	for i, c := range callers {
+		parts[i] = fmt.Sprintf("%s %d", c, counts[c])
+	}
+	return fmt.Sprintf("mesh-cli read calls in the last %s, said in no line of their own: %s", over,
+		strings.Join(parts, ", "))
+}
+
+// readCallsEvery is how often the count of read calls is said.
+const readCallsEvery = time.Hour
+
+// sayReadCalls says, every readCallsEvery until ctx ends, how many read calls each caller made — when any did — so
+// a runaway poller stays visible while its calls write no line each (novox/hq issue 762).
+func sayReadCalls(ctx context.Context) {
+	tick := time.NewTicker(readCallsEvery)
+	defer tick.Stop()
+	for {
+		select {
+		case <-ctx.Done():
+			return
+		case <-tick.C:
+			if line := cliReads.said(readCallsEvery); line != "" {
+				cliJournal(line)
+			}
+		}
+	}
+}
+
+// runForMeshCLI runs a judged line and answers what it said. Every line that changes something, is refused or fails
+// is said in the journal: its call, who asked on which node, its command word only, and how it runs (review of ADR
+// 0272). A line that only reads and succeeds is counted instead, and the count said hourly (novox/hq issue 762): a
+// page polling `plans` every few seconds would otherwise write thousands of lines a day.
 func runForMeshCLI(ctx context.Context, node string, asked link.CLIAsked, v cliVerdict) link.CLIAnswer {
 	how := "as an ordinary call"
 	switch {
@@ -137,7 +214,23 @@ func runForMeshCLI(ctx context.Context, node string, asked link.CLIAsked, v cliV
 	if call == "" {
 		call = "(no call)"
 	}
-	cliJournal(fmt.Sprintf("mesh-cli %s: %s on %s asked %q, %s", call, asked.Account, node, asked.Line[0], how))
+	line := fmt.Sprintf("mesh-cli %s: %s on %s asked %q, %s", call, asked.Account, node, asked.Line[0], how)
+	if !onlyReads(asked, v) {
+		cliJournal(line)
+		return runJudgedLine(ctx, node, asked, v)
+	}
+	answer := runJudgedLine(ctx, node, asked, v)
+	if answer.Exit != 0 || answer.Refused != "" {
+		// A read that was refused or failed is said, as a change is.
+		cliJournal(fmt.Sprintf("%s, and it failed (exit %d)", line, answer.Exit))
+	} else {
+		cliReads.add(fmt.Sprintf("%s on %s", asked.Account, node))
+	}
+	return answer
+}
+
+// runJudgedLine runs a line runForMeshCLI has said or counted.
+func runJudgedLine(ctx context.Context, node string, asked link.CLIAsked, v cliVerdict) link.CLIAnswer {
 	if v.refused != "" {
 		return link.CLIRefusal(v.refused)
 	}

cmd/mesh-controller/meshcli_reads_test.go

@@ -0,0 +1,123 @@
+package main
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// echoFails makes the echoing test binary end with a failure, as a read that failed would.
+const echoFails = "MESH_TEST_ECHO_FAILS"
+
+// journalled runs lines through runForMeshCLI and answers what the journal said, with a fresh count of read calls.
+func journalled(t *testing.T) *[]string {
+	t.Helper()
+	t.Setenv(echoEnvironment, "1")
+	said := &[]string{}
+	was, wasReads := cliJournal, cliReads
+	cliJournal = func(line string) { *said = append(*said, line) }
+	cliReads = &readCalls{}
+	t.Cleanup(func() { cliJournal, cliReads = was, wasReads })
+	return said
+}
+
+// **A call that only reads writes no journal line; one that changes something does** (novox/hq issue 762).
+func TestAReadCallWritesNoJournalLine(t *testing.T) {
+	said := journalled(t)
+	ctx := link.WithCallID(context.Background(), "call-1")
+	ordinary := cliVerdict{why: "not the terminal"}
+	for _, line := range [][]string{{"plans"}, {"status"}, {"conditions"}, {"node", "list"}, {"builds"}, {"queue"},
+		{"settings", "show", "zsh"}, {"plans", "--json"}} {
+		if a := runForMeshCLI(ctx, "g14", cliAsked("jochen", 1000, line...), ordinary); a.Exit != 0 || a.Refused != "" {
+			t.Fatalf("%q did not run: %+v", line, a)
+		}
+	}
+	runForMeshCLI(ctx, "control", cliAsked("jochen", 1000, "plans"), cliVerdict{terminal: true, why: "the terminal"})
+	if len(*said) != 0 {
+		t.Fatalf("read calls wrote journal lines: %q", *said)
+	}
+	runForMeshCLI(ctx, "control", cliAsked("jochen", 1000, "assign", "g14", "zsh"), cliVerdict{terminal: true, why: "the terminal"})
+	if len(*said) != 1 || !strings.Contains((*said)[0], `asked "assign"`) {
+		t.Fatalf("a call that changes something was not said: %q", *said)
+	}
+}
+
+// **A refused or failed read keeps its line** (novox/hq issue 762).
+func TestAFailedOrRefusedReadIsSaid(t *testing.T) {
+	said := journalled(t)
+	runForMeshCLI(context.Background(), "g14", cliAsked("root", 0, "plans"), cliVerdict{refused: "root is not answered"})
+	t.Setenv(echoFails, "1")
+	a := runForMeshCLI(context.Background(), "g14", cliAsked("jochen", 1000, "plans"), cliVerdict{why: "not the terminal"})
+	if a.Exit == 0 {
+		t.Fatalf("the read did not fail: %+v", a)
+	}
+	if len(*said) != 2 || !strings.Contains((*said)[0], "refused") || !strings.Contains((*said)[1], "failed (exit 3)") {
+		t.Fatalf("the journal said %q", *said)
+	}
+	if line := cliReads.said(readCallsEvery); line != "" {
+		t.Fatalf("a failed read was counted as a read: %q", line)
+	}
+}
+
+// **A verb is classed by what the call does, not its name** (novox/hq issue 762): the same command word reads with
+// some arguments and writes with others, and a line no reading form names is said.
+func TestAVerbIsClassedByItsArguments(t *testing.T) {
+	ordinary := cliVerdict{why: "not the terminal"}
+	for _, c := range []struct {
+		line  []string
+		reads bool
+	}{
+		{[]string{"settings", "show", "zsh"}, true},
+		{[]string{"settings", "preferences"}, true},
+		{[]string{"settings", "set", "zsh", `{"a":1}`}, false},
+		{[]string{"settings", "clear", "zsh"}, false},
+		{[]string{"plans"}, true},
+		{[]string{"plans", "p-1"}, true},
+		{[]string{"plans", "go", "p-1"}, false},
+		{[]string{"plans", "--json", "stop", "p-1"}, false},
+		{[]string{"plans", "close", "p-1"}, false},
+		{[]string{"plans", "retry", "p-1"}, false},
+		{[]string{"conditions"}, true},
+		{[]string{"conditions", "show", "x"}, true},
+		{[]string{"conditions", "silence", "x"}, false},
+		{[]string{"doctor", "run"}, false},
+		{[]string{"node", "account", "g14", "x"}, false},
+		{[]string{"assign", "g14", "zsh"}, false},
+		{[]string{"a-command-added-later"}, false},
+		{[]string{"serve"}, false},
+	} {
+		if got := onlyReads(cliAsked("jochen", 1000, c.line...), ordinary); got != c.reads {
+			t.Errorf("%q judged a read: %v, want %v", c.line, got, c.reads)
+		}
+	}
+	if onlyReads(cliAsked("jochen", 1000, "plans"), cliVerdict{refused: "no"}) {
+		t.Error("a refused line judged a read")
+	}
+	stdin := cliAsked("jochen", 1000, "status")
+	stdin.Stdin = []byte("x")
+	if onlyReads(stdin, cliVerdict{terminal: true}) {
+		t.Error("a line carrying standard input judged a read")
+	}
+}
+
+// **Once an hour the read calls are counted per caller, only if there were any** (novox/hq issue 762).
+func TestTheReadCallsAreCountedPerCaller(t *testing.T) {
+	said := journalled(t)
+	ordinary := cliVerdict{why: "not the terminal"}
+	for range 3 {
+		runForMeshCLI(context.Background(), "g14", cliAsked("jochen", 1000, "plans"), ordinary)
+	}
+	runForMeshCLI(context.Background(), "ace", cliAsked("jochen", 1000, "status"), ordinary)
+	if len(*said) != 0 {
+		t.Fatalf("read calls wrote lines: %q", *said)
+	}
+	line := cliReads.said(readCallsEvery)
+	if !strings.Contains(line, "jochen on ace 1, jochen on g14 3") || !strings.Contains(line, "1h0m0s") {
+		t.Fatalf("the hourly line said %q", line)
+	}
+	if again := cliReads.said(readCallsEvery); again != "" {
+		t.Fatalf("an hour with no read calls said %q", again)
+	}
+}

cmd/mesh-controller/push.go

@@ -297,6 +297,8 @@ func serve(ctx context.Context) (err error) {
 		return err
 	}
 	defer stopCLI()
+	// Its read calls write no line each; how many each caller made is said once an hour (novox/hq issue 762).
+	go sayReadCalls(ctx)
 	// And says so on the bus (novox/hq ADR 0197): what it serves, as the NATS services protocol asks.
 	stopAnnouncing, err := bus.Announce(seatAnnouncement(handlers), log.New(os.Stdout, "", log.LstdFlags))
 	if err != nil {

cmd/mesh-controller/sayable_test.go

@@ -43,6 +43,9 @@ func TestMain(m *testing.M) {
 	}
 	if os.Getenv(echoEnvironment) != "" {
 		fmt.Printf("verb=%q caller=%q terminal=%v\n", os.Getenv("MESH_VERB"), os.Getenv("MESH_CALLER"), startedAtTheTerminal())
+		if os.Getenv(echoFails) != "" {
+			os.Exit(3)
+		}
 		os.Exit(0)
 	}
 	conditions.Unsayable = func(o conditions.Observation, field string, r outward.Refusal) {