diff --git a/cmd/mesh-controller/delivery_conditions.go b/cmd/mesh-controller/delivery_conditions.go index b8f4cbd0..4fe74503 100644 --- a/cmd/mesh-controller/delivery_conditions.go +++ b/cmd/mesh-controller/delivery_conditions.go @@ -33,12 +33,14 @@ var deliveryOwnerWithin = 10 * time.Second // stalledLine is one delivery past its bound, as mesh-delivery's `stalled` says it. type stalledLine struct { - ID string `json:"id"` - State string `json:"state"` - For string `json:"for"` - Bound string `json:"bound"` - H2 string `json:"h2"` - Says string `json:"says"` + ID string `json:"id"` + // Number is the pull request's, for a line of a head the forge never announced (novox/hq issue 347). + Number int `json:"number,omitempty"` + State string `json:"state"` + For string `json:"for"` + Bound string `json:"bound"` + H2 string `json:"h2"` + Says string `json:"says"` } // operatorsOnly is whether the table leaves H2 nothing to do for the line: the state is the operator's. diff --git a/cmd/mesh-controller/meshcli.go b/cmd/mesh-controller/meshcli.go index 97c979d5..c0255895 100644 --- a/cmd/mesh-controller/meshcli.go +++ b/cmd/mesh-controller/meshcli.go @@ -71,8 +71,14 @@ func judgeCLI(node string, asked link.CLIAsked, nodes []inventory.Node, control "terminal is the one control-node's operator account", len(control))} } if control[0] == node { - return cliVerdict{terminal: true, why: fmt.Sprintf("the controller's terminal: %s on the control-node %s", - asked.Account, node)} + // **A person at a terminal, not a service of the operator's** (review of ADR 0272): the tool runner and the + // account's user units run as the operator too, and only a login session is the terminal. + if asked.Session == "" { + return cliVerdict{why: fmt.Sprintf("not the controller's terminal: %s on %s asked from no login session — a "+ + "service or a user unit running as the operator, not a person at a terminal", asked.Account, node)} + } + return cliVerdict{terminal: true, why: fmt.Sprintf("the controller's terminal: %s on the control-node %s, "+ + "login session %s", asked.Account, node, asked.Session)} } return cliVerdict{why: fmt.Sprintf("not the controller's terminal: agents on %s may run as %s, so a "+ "terminal-only change is made from the control-node %s (novox/hq ADR 0272)", node, asked.Account, control[0])} @@ -148,7 +154,7 @@ func runForMeshCLI(ctx context.Context, node string, asked link.CLIAsked, v cliV verb, line = cliVerb, composed } cmd := selfCommand(ctx, line) - cmd.Env = commandEnvironment(fmt.Sprintf("%s through mesh-cli on %s", asked.Account, node), verb) + cmd.Env = commandEnvironment(fmt.Sprintf("%s through mesh-cli on %s", asked.Account, node), verb, v.terminal) // No standard input: a command that reads one gets nothing, and fails saying so (ADR 0272 §5). cmd.Stdin = nil var stdout, stderr bytes.Buffer diff --git a/cmd/mesh-controller/meshcli_test.go b/cmd/mesh-controller/meshcli_test.go index 8baf28a3..8dcc5522 100644 --- a/cmd/mesh-controller/meshcli_test.go +++ b/cmd/mesh-controller/meshcli_test.go @@ -25,7 +25,7 @@ var cliNodes = []inventory.Node{ } func asked(account string, uid uint32, line ...string) link.CLIAsked { - return link.CLIAsked{Line: line, Account: account, UID: uid} + return link.CLIAsked{Line: line, Account: account, UID: uid, Session: "session-1.scope"} } // Who is the controller's terminal, and who is an ordinary call or refused (novox/hq ADR 0272 §4). @@ -264,3 +264,42 @@ func TestTheCallsVerbNeverShowsAMeshCLILinesAnswer(t *testing.T) { t.Fatalf("calls showed a mesh-cli line's answer: %s", said) } } + +// **Only the terminal's line carries the terminal's mark** (review of ADR 0272): a mark the serving controller's +// environment holds — leaked, or set by anything — is stripped from every other command line it runs, a verb's and +// an ordinary mesh-cli line alike, so neither reads as the terminal. +func TestTheTerminalsMarkIsStrippedFromEveryOtherLine(t *testing.T) { + t.Setenv(echoEnvironment, "1") + t.Setenv(cliTerminalVar, "1") + t.Setenv(servedVar, "1") + for _, env := range [][]string{commandEnvironment("someone", "status", false)} { + for _, kv := range env { + if strings.HasPrefix(kv, cliTerminalVar+"=") { + t.Fatalf("a verb's line carries the terminal's mark: %s", kv) + } + } + } + a := runForMeshCLI(context.Background(), "laptop", asked("operator", 1000, "status"), cliVerdict{why: "not the terminal"}) + if got := string(a.Stdout); !strings.Contains(got, "terminal=false") || !strings.Contains(got, `verb="mesh-cli"`) { + t.Fatalf("an ordinary line with the mark in the serving environment ran as %s", got) + } + a = runForMeshCLI(context.Background(), "control", asked("operator", 1000, "status"), cliVerdict{terminal: true}) + if got := string(a.Stdout); !strings.Contains(got, "terminal=true") { + t.Fatalf("the terminal's line ran as %s", got) + } +} + +// **Only a login session is the terminal** (review of ADR 0272): the operator's account on the control-node, asking +// from a service — the tool runner, a user unit — and not a login session, is an ordinary call. +func TestOnlyALoginSessionIsTheTerminal(t *testing.T) { + control := []string{"control"} + v := judgeCLI("control", link.CLIAsked{Line: []string{"status"}, Account: "operator", UID: 1000}, cliNodes, control) + if v.terminal || v.refused != "" || !strings.Contains(v.why, "login session") { + t.Fatalf("a line from no login session reads %+v", v) + } + v = judgeCLI("control", link.CLIAsked{Line: []string{"status"}, Account: "operator", UID: 1000, Session: "session-3.scope"}, + cliNodes, control) + if !v.terminal { + t.Fatalf("a line from a login session on the control-node reads %+v", v) + } +} diff --git a/cmd/mesh-controller/plain_words.go b/cmd/mesh-controller/plain_words.go index 0599d6f2..c48d9f05 100644 --- a/cmd/mesh-controller/plain_words.go +++ b/cmd/mesh-controller/plain_words.go @@ -758,11 +758,14 @@ func stalledWords(l stalledLine, o conditions.Observation) (headline, explanatio if o.Resolver == conditions.ResolverOperator { needs = "push a new commit to its branch; the forge announces it and the mesh checks it." } - return fmt.Sprintf("A pull request of %s has had no merge check %s", name, long), - fmt.Sprintf("A pull request of %s has been open %s on a branch that requires the merge check, and the "+ - "mesh was never asked to check it: the forge never announced it. It cannot merge until it is checked.", - name, long), - fmt.Sprintf("The pull request of %s has a merge check now, or is closed", name), needs, nil + pull := "A pull request of " + name + if l.Number > 0 { + pull = fmt.Sprintf("Pull request %s #%d", name, l.Number) + } + return fmt.Sprintf("%s has had no merge check %s", pull, long), + fmt.Sprintf("%s has been open %s on a branch that requires the merge check, and the mesh was never asked "+ + "to check it: the forge never announced it. It cannot merge until it is checked.", pull, long), + fmt.Sprintf("%s has a merge check now, or is closed", pull), needs, nil } held := l.State if held == "" { diff --git a/cmd/mesh-controller/plain_words_test.go b/cmd/mesh-controller/plain_words_test.go index 41665ee2..d045d651 100644 --- a/cmd/mesh-controller/plain_words_test.go +++ b/cmd/mesh-controller/plain_words_test.go @@ -314,7 +314,7 @@ func TestANeedNoNotificationAnswersNamesTheMeshMCPServer(t *testing.T) { // as a stalled line in state `unannounced` — no delivery exists, so there is nothing to stop, release or close — // and the operator's one act is a new commit on its branch, which the forge announces. func TestAnUnannouncedPullRequestSaysToPushANewCommit(t *testing.T) { - l := stalledLine{ID: "novox/hq@bfe82315f42c", State: "unannounced", For: "11m0s", Bound: "10m0s", + l := stalledLine{ID: "novox/hq@bfe82315f42c", Number: 243, State: "unannounced", For: "11m0s", Bound: "10m0s", H2: "none: the forge never announced it, so there is no delivery to close — the operator's", Says: "novox/hq#243 is open on main, which requires the merge check, and its head has had no merge check"} obs := stalledObservations([]stalledLine{l}) @@ -325,7 +325,8 @@ func TestAnUnannouncedPullRequestSaysToPushANewCommit(t *testing.T) { if strings.Contains(o.Needs, "stop") || strings.Contains(o.Needs, "release") || !strings.Contains(o.Needs, "commit") { t.Fatalf("it says to %q", o.Needs) } - if !strings.Contains(o.Headline, "no merge check") || !strings.Contains(o.Explanation, "never") { + if !strings.Contains(o.Headline, "no merge check") || !strings.Contains(o.Headline, "#243") || + !strings.Contains(o.Explanation, "never") { t.Fatalf("it reads %q / %q", o.Headline, o.Explanation) } } diff --git a/cmd/mesh-controller/seatverbs.go b/cmd/mesh-controller/seatverbs.go index 039b84e4..53478519 100644 --- a/cmd/mesh-controller/seatverbs.go +++ b/cmd/mesh-controller/seatverbs.go @@ -931,27 +931,26 @@ func isWhyFlag(word string) bool { // commandEnvironment is the environment of a command line this controller runs for someone: its own — the // stores' credentials, the bus, the broker, everything a command run from a shell beside it would have, because it -// is that — with who asked, and the verb it came through. An empty verb is the controller's terminal (novox/hq ADR -// 0272 §4): no `MESH_VERB` at all, whatever this process was started with. +// is that — with who asked, and how it came. // -// The terminal's line is also not the serving controller's (servedVar), and carries cliTerminalVar, so what reads -// whether it was started at the terminal (startedAtTheTerminal) reads yes; every other line is stripped of that mark. -func commandEnvironment(caller, verb string) []string { +// **terminal** is said, never inferred (novox/hq ADR 0272): only a line mesh-cli asked as the controller's terminal +// passes true. Its line has no `MESH_VERB`, not the served mark ADR 0266 puts on everything the serving controller +// starts, and the terminal's mark (cliTerminalVar), so startedAtTheTerminal reads yes. Every other line names its +// verb, and is stripped of the terminal's mark whatever this process's environment holds. +func commandEnvironment(caller, verb string, terminal bool) []string { env := make([]string, 0, len(os.Environ())+3) for _, kv := range os.Environ() { if strings.HasPrefix(kv, verbVar+"=") || strings.HasPrefix(kv, link.CallerVar+"=") || - strings.HasPrefix(kv, cliTerminalVar+"=") || (verb == "" && strings.HasPrefix(kv, servedVar+"=")) { + strings.HasPrefix(kv, cliTerminalVar+"=") || (terminal && strings.HasPrefix(kv, servedVar+"=")) { continue } env = append(env, kv) } env = append(env, link.CallerVar+"="+caller) - if verb != "" { - env = append(env, verbVar+"="+verb) - } else { - env = append(env, cliTerminalVar+"=1") + if terminal { + return append(env, cliTerminalVar+"=1") } - return env + return append(env, verbVar+"="+verb) } // runVerb runs this binary with the given command line and gathers what it said. @@ -977,7 +976,7 @@ func runVerb(ctx context.Context, argv []string) (verbAnswer, error) { if verb == "" { verb = argv[0] } - cmd.Env = commandEnvironment(caller+", through the "+catalogue.ControllerSeatName+" seat", verb) + cmd.Env = commandEnvironment(caller+", through the "+catalogue.ControllerSeatName+" seat", verb, false) // Two buffers, one answer. What the command *says* is both streams, in the order a person at // a shell would read them; what it *answers as data* is standard output alone — `status --json` // prints its warnings beside the document, and a JSON parsed from the two together parsed diff --git a/internal/link/meshcli.go b/internal/link/meshcli.go index 27dcc6f3..d18ef939 100644 --- a/internal/link/meshcli.go +++ b/internal/link/meshcli.go @@ -37,6 +37,10 @@ type CLIAsked struct { Account string `json:"account"` UID uint32 `json:"uid"` Follow string `json:"follow,omitempty"` + // Session is the login session the asking process runs in, as the node-engine read it from the kernel's cgroup + // (`session-.scope` under the account's own slice), or empty: a service, a user unit. Only a login session is + // the terminal (novox/hq ADR 0272). + Session string `json:"session,omitempty"` } // CLIAnswer is what the controller answers, as the `result` of a call's answer: what the command printed, how it diff --git a/internal/link/meshcli_test.go b/internal/link/meshcli_test.go index 2a9f9e0b..9a7f79d9 100644 --- a/internal/link/meshcli_test.go +++ b/internal/link/meshcli_test.go @@ -17,7 +17,11 @@ import ( // The node-engine's request and the answer it hands mesh-cli hold these field names; the engine's side holds the // same list (mesh-host internal/meshcli, TestTheRequestToTheControllerKeepsItsFieldNames). func TestTheMeshCLIRequestAndAnswerKeepTheirFieldNames(t *testing.T) { - body, _ := json.Marshal(CLIAsked{Line: []string{"status"}, Account: "a", UID: 1}) + body, _ := json.Marshal(CLIAsked{Line: []string{"status"}, Account: "a", UID: 1, Session: "session-3.scope"}) + if got := keysIn(t, body); got != "account line session uid" { + t.Fatalf("the request's fields are %q", got) + } + body, _ = json.Marshal(CLIAsked{Line: []string{"status"}, Account: "a", UID: 1}) if got := keysIn(t, body); got != "account line uid" { t.Fatalf("the request's fields are %q", got) }