From 4c86558de86a86d134505fbab71df81371cb6e14 Mon Sep 17 00:00:00 2001 From: Bozhidar Batsov Date: Thu, 8 Oct 2026 11:04:39 +0300 Subject: [PATCH] Let proof proxy record a client as the start of a profile Writing a client profile starts with finding out what the client sends, which meant copying requests out of the -v transcript by hand. -record saves them as a client profile instead, with a check for each connection and a step for each request. The steps leave out the request ids, as proof gives requests ids of its own, and the sessions the client cloned get names (across connections, as sessions outlive them), so the steps use the sessions proof clones. What's left is reading the client's code to see what it needs from each reply. proof serve gets -record as well, since it grades clients the same way. The recording is written even when the JSON report can't be, as getting the client to do it all again isn't always easy. Only the session field of a step stands for a session now, so code that starts with $ goes to the server as it is. --- cmd/proof/proxy.go | 18 ++- cmd/proof/proxy_test.go | 18 +++ doc/design.md | 7 +- doc/hacking.md | 11 +- doc/usage.md | 3 +- internal/checks/client_profiles.go | 31 ++-- internal/checks/client_profiles_test.go | 6 +- internal/checks/record.go | 188 ++++++++++++++++++++++++ internal/checks/record_test.go | 74 ++++++++++ 9 files changed, 331 insertions(+), 25 deletions(-) create mode 100644 internal/checks/record.go create mode 100644 internal/checks/record_test.go diff --git a/cmd/proof/proxy.go b/cmd/proof/proxy.go index 492eb16..f166319 100644 --- a/cmd/proof/proxy.go +++ b/cmd/proof/proxy.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "flag" "fmt" "io" @@ -23,12 +24,14 @@ import ( type clientOptions struct { listen string json string + record string verbose bool } func (o *clientOptions) register(fs *flag.FlagSet) { fs.StringVar(&o.listen, "listen", "127.0.0.1:0", "accept clients on `host:port` (port 0 picks a free one)") fs.StringVar(&o.json, "json", "", "also write a JSON report to `file`") + fs.StringVar(&o.record, "record", "", "also write the requests the client sent to `file`, as the start of a client profile") fs.BoolVar(&o.verbose, "v", false, "show every message the client and the server exchanged") } @@ -143,11 +146,18 @@ func gradeClients(stdout, stderr io.Writer, r report.Run, traffic []check.Traffi } } showDeath(stdout, srv) + // A recording can't be made again, so it's written even when the + // report can't be. + var errs []error if o.json != "" { - if err := writeJSON(o.json, r); err != nil { - fmt.Fprintln(stderr, "proof:", err) - return 2 - } + errs = append(errs, writeJSON(o.json, r)) + } + if o.record != "" { + errs = append(errs, os.WriteFile(o.record, []byte(checks.RecordedProfile(traffic)), 0o666)) + } + if err := errors.Join(errs...); err != nil { + fmt.Fprintln(stderr, "proof:", err) + return 2 } if r.Counts()[check.Failed] > 0 { return 1 diff --git a/cmd/proof/proxy_test.go b/cmd/proof/proxy_test.go index c68c134..0727462 100644 --- a/cmd/proof/proxy_test.go +++ b/cmd/proof/proxy_test.go @@ -132,6 +132,24 @@ func TestProxyWritesJSONAndTranscripts(t *testing.T) { } } +// The recording is written even when the report can't be, as the client +// would have to do it all again. +func TestProxyRecordsTheClient(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "client.toml") + code, stdout, stderr := proxyRun(t, []string{"d2:id1:12:op8:describee"}, "-record", path, "-json", filepath.Join(dir, "missing", "client.json")) + if code != 2 || !strings.Contains(stderr, "missing") { + t.Errorf("exit status %d, want 2 with the JSON report's error:\n%s%s", code, stdout, stderr) + } + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(b), `send = { op = "describe" }`) { + t.Errorf("the recording doesn't have the request:\n%s", b) + } +} + func TestProxyNeedsAServer(t *testing.T) { var stderr bytes.Buffer if code := proxyUntil(context.Background(), nil, io.Discard, &stderr, nil); code != 2 { diff --git a/doc/design.md b/doc/design.md index f836a19..f2e3e54 100644 --- a/doc/design.md +++ b/doc/design.md @@ -134,6 +134,10 @@ use. Replies that only bother the client (e.g. an error the client just shows to the user) get a note. Right now there's a profile for CIDER, built from what CIDER sends with its default settings. +A new profile starts with a recording, as `proof proxy -record` saves +what a client sent as a profile with a check for each connection. What +the client needs from the replies comes from reading its code. + ## Strict About the Wire, Relaxed About the Rest proof has its own bencode implementation, as the popular Go libraries @@ -308,8 +312,7 @@ what's planned next: - robustness checks (malformed messages, fields of the wrong type, clients disconnecting in the middle of an evaluation) - client profiles for more clients (e.g. Calva, Conjure and - vim-fireplace), and a way to record them with `proof proxy`, which - already sees the traffic but doesn't save it yet + vim-fireplace) - more servers in the compatibility matrix and a proper home for the matrix itself - incorporating [Spec Changes](spec-changes.md) into the spec, so that diff --git a/doc/hacking.md b/doc/hacking.md index 61518dc..9fe20e7 100644 --- a/doc/hacking.md +++ b/doc/hacking.md @@ -261,9 +261,14 @@ like the links in `refs.go`). A check that sends code in some language should list the capability for it in `needs`, unless the client sends that code to any server (like CIDER's startup code). -To find out what a client sends, run it through `proof proxy` with `-v`, -which shows every request. Then read the client's code to see what it -does with each reply, and keep only what the client really needs. +To find out what a client sends, run it through `proof proxy -record +client.toml`. That gives you a profile with a check for each connection, +whose steps send what the client sent (and `-v` shows the replies as +well). Then read the client's code to see what it does with each reply, +keep only the steps the client really needs and fill in the rest. Keep +in mind that each step waits for its reply before the next one goes out, +so requests sent while another one is still running (e.g. `stdin` and +`interrupt`) won't work in a profile. `TestClientProfiles` makes sure a profile hangs together, and the fake server should get a quirk for anything a profile catches that the other checks don't. diff --git a/doc/usage.md b/doc/usage.md index bec8bb9..f361cba 100644 --- a/doc/usage.md +++ b/doc/usage.md @@ -277,6 +277,7 @@ Here are the options supported by `proof proxy`: | `-listen host:port` | Accept the client on this address. The default is `127.0.0.1:0`, which picks a free port. | | `-v` | Show every message exchanged between the client and the server. | | `-json file` | Write a JSON report as well. | +| `-record file` | Write the requests the client sent to a file, as the start of a [client profile](hacking.md#adding-a-client-profile). | The exit codes are the same as for `proof run`, except that 1 means that the client failed some checks and 3 means that no client sent anything. @@ -397,7 +398,7 @@ all you need for checking output, values, errors, input and interrupts. When you stop it, `proof serve` checks the requests your client sent, just like `proof proxy` does, with the same report, exit codes and -`-listen`, `-json` and `-v` options. A test suite can run it in CI like +`-listen`, `-json`, `-record` and `-v` options. A test suite can run it in CI like this: ```shell diff --git a/internal/checks/client_profiles.go b/internal/checks/client_profiles.go index 8559051..08d1b60 100644 --- a/internal/checks/client_profiles.go +++ b/internal/checks/client_profiles.go @@ -3,6 +3,7 @@ package checks import ( "embed" "fmt" + "io/fs" "slices" "strings" "sync" @@ -41,8 +42,8 @@ type clientCheck struct { // clientStep is a request and what the client needs from its reply, // besides done. type clientStep struct { - // Send is the request. A string starting with $ stands for the - // session an earlier step got. + // Send is the request. A session starting with $ stands for the one + // an earlier step got. Send request `toml:"send"` // Snippet names a snippet of the server's profile whose code goes in // the request, and whose value the reply has to have. It stands for @@ -81,19 +82,27 @@ var clientProfiles = sync.OnceValue(func() []clientProfile { } var profiles []clientProfile for _, f := range files { - var p clientProfile - md, err := toml.DecodeFS(clientFiles, "clients/"+f.Name(), &p) + p, err := readClientProfile(clientFiles, "clients/"+f.Name()) if err != nil { - panic(fmt.Sprintf("client profile %s: %v", f.Name(), err)) - } - if undecoded := md.Undecoded(); len(undecoded) > 0 { - panic(fmt.Sprintf("client profile %s: unknown keys %v", f.Name(), undecoded)) + panic(err) } profiles = append(profiles, p) } return profiles }) +func readClientProfile(fsys fs.FS, path string) (clientProfile, error) { + var p clientProfile + md, err := toml.DecodeFS(fsys, path, &p) + if err != nil { + return p, fmt.Errorf("client profile %s: %v", path, err) + } + if undecoded := md.Undecoded(); len(undecoded) > 0 { + return p, fmt.Errorf("client profile %s: unknown keys %v", path, undecoded) + } + return p, nil +} + func clientProfileChecks() []*check.Check { var checks []*check.Check for _, p := range clientProfiles() { @@ -127,11 +136,11 @@ func (c clientCheck) replay(t *check.T) { for i, s := range c.Steps { req := nrepl.Message{} for k, v := range s.Send { - if name, ok := v.(string); ok && strings.HasPrefix(name, "$") { - v = sessions[name[1:]] - } req[k] = v } + if name, ok := req["session"].(string); ok && strings.HasPrefix(name, "$") { + req["session"] = sessions[name[1:]] + } var want string if s.Snippet != "" { sn := t.Snippet(s.Snippet) diff --git a/internal/checks/client_profiles_test.go b/internal/checks/client_profiles_test.go index 8beeb9e..2d4a560 100644 --- a/internal/checks/client_profiles_test.go +++ b/internal/checks/client_profiles_test.go @@ -41,10 +41,8 @@ func TestClientProfiles(t *testing.T) { if (s.NewSession != "" || s.Snippet != "" || len(s.Dicts) > 0) && s.Why == "" { t.Errorf("%s step %d: doesn't say what happens to the client without what it needs", c.ID, i+1) } - for _, v := range s.Send { - if name, ok := v.(string); ok && strings.HasPrefix(name, "$") && !sessions[name[1:]] { - t.Errorf("%s step %d: uses %s before a step gets it", c.ID, i+1, name) - } + if name, ok := s.Send["session"].(string); ok && strings.HasPrefix(name, "$") && !sessions[name[1:]] { + t.Errorf("%s step %d: uses %s before a step gets it", c.ID, i+1, name) } if s.NewSession != "" { sessions[s.NewSession] = true diff --git a/internal/checks/record.go b/internal/checks/record.go new file mode 100644 index 0000000..ddffb38 --- /dev/null +++ b/internal/checks/record.go @@ -0,0 +1,188 @@ +package checks + +import ( + "fmt" + "regexp" + "slices" + "strconv" + "strings" + "unicode/utf8" + + "github.com/nrepl/proof/internal/check" + "github.com/nrepl/proof/nrepl" +) + +// RecordedProfile turns the requests in traffic into a client profile, +// with a check for each connection, to be turned into a real one: it says +// what the client sent, but not yet what the client needs from the +// replies. +func RecordedProfile(traffic []check.Traffic) string { + name := "" + var checks strings.Builder + sessions := &sessionNames{byID: map[string]string{}} + for _, tr := range traffic { + steps := recordedSteps(tr.Events, sessions) + if len(steps) == 0 { + continue + } + id := strings.ReplaceAll(tr.Label, " ", "-") + fmt.Fprintf(&checks, "\n[[checks]]\nid = %s\ntitle = %s\nwhy = \"\"\n", tomlString(id), tomlString(tr.Label)) + for _, s := range steps { + if n, ok := s.Send["client-name"].(string); ok && name == "" { + name = n + } + checks.WriteString(recordedStep(s)) + } + } + return recordedHeader + "name = " + tomlString(name) + "\ncode = \"\"\n" + checks.String() +} + +// sessionNames names the sessions clients cloned, across all connections, +// as sessions outlive them. +type sessionNames struct { + byID map[string]string + n int +} + +// add names a session the server handed out. One it handed out before +// gets a new name, as it's a new session for proof. +func (s *sessionNames) add(id string) string { + s.n++ + s.byID[id] = "s" + strconv.Itoa(s.n) + return s.byID[id] +} + +// recordedSteps turns each request of a connection into a step, without +// its id, as proof gives requests ids of its own. The sessions the client +// cloned get names, so that the steps can use the sessions proof clones. +func recordedSteps(events []nrepl.Event, sessions *sessionNames) []clientStep { + var steps []clientStep + clones := map[string]int{} // the ids of clone requests, and their steps + for _, ev := range events { + m := ev.Msg + if m == nil { + continue + } + if ev.Dir == nrepl.Received { + id, _ := idKey(m) + i, ok := clones[id] + if s := m.Str("new-session"); ok && s != "" && steps[i].NewSession == "" { + steps[i].NewSession = sessions.add(s) + } + continue + } + send := request{} + for k, v := range m { + if k != "id" { + send[k] = v + } + } + if name := sessions.byID[m.Str("session")]; name != "" { + send["session"] = "$" + name + } + if m.Str("op") == "clone" { + id, _ := idKey(m) + clones[id] = len(steps) + } + steps = append(steps, clientStep{Send: send}) + } + return steps +} + +const recordedHeader = `# The requests clients sent through proof, with a check for each +# connection. To make a client profile out of it, fill in the blanks, use +# snippets for the user's code and say what the client needs from each +# reply (see "Adding a Client Profile" in doc/hacking.md). +` + +// recordedStep writes a step the way the profiles in clients/ write them. +func recordedStep(s clientStep) string { + newSession := "" + if s.NewSession != "" { + newSession = "new-session = " + tomlString(s.NewSession) + "\n" + } + // The op goes first, as that's what tells the requests apart. + keys := nrepl.Message(s.Send).Keys() + if i := slices.Index(keys, "op"); i > 0 { + keys = slices.Insert(slices.Delete(keys, i, i+1), 0, "op") + } + if send := "send = " + tomlTable(s.Send, keys); len(send) <= 100 { + return "\n[[checks.steps]]\n" + send + "\n" + newSession + } + var b strings.Builder + b.WriteString("\n[[checks.steps]]\n") + if newSession != "" { + b.WriteString(newSession + "\n") + } + b.WriteString("[checks.steps.send]\n") + for _, k := range keys { + fmt.Fprintf(&b, "%s = %s\n", tomlKey(k), tomlValue(s.Send[k])) + } + return b.String() +} + +// tomlValue writes a value decoded from bencode. +func tomlValue(v any) string { + switch v := v.(type) { + case string: + return tomlString(v) + case int64: + return strconv.FormatInt(v, 10) + case []any: + items := make([]string, len(v)) + for i, item := range v { + items[i] = tomlValue(item) + } + return "[" + strings.Join(items, ", ") + "]" + case map[string]any: + return tomlTable(v, nrepl.Message(v).Keys()) + } + panic(fmt.Sprintf("can't write %T as TOML", v)) +} + +// tomlTable writes the keys of m, in this order, as an inline table. +func tomlTable(m map[string]any, keys []string) string { + if len(keys) == 0 { + return "{}" + } + fields := make([]string, len(keys)) + for i, k := range keys { + fields[i] = tomlKey(k) + " = " + tomlValue(m[k]) + } + return "{ " + strings.Join(fields, ", ") + " }" +} + +var bareKey = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + +func tomlKey(k string) string { + if bareKey.MatchString(k) { + return k + } + return tomlString(k) +} + +// tomlString writes s as a basic string, on one line. Bytes that aren't +// UTF-8, which TOML can't have, become U+FFFD. +func tomlString(s string) string { + var b strings.Builder + b.WriteByte('"') + for _, r := range strings.ToValidUTF8(s, string(utf8.RuneError)) { + switch { + case r == '"' || r == '\\': + b.WriteByte('\\') + b.WriteRune(r) + case r == '\n': + b.WriteString(`\n`) + case r == '\t': + b.WriteString(`\t`) + case r == '\r': + b.WriteString(`\r`) + case r < 0x20 || r == 0x7f: + fmt.Fprintf(&b, `\u%04X`, r) + default: + b.WriteRune(r) + } + } + b.WriteByte('"') + return b.String() +} diff --git a/internal/checks/record_test.go b/internal/checks/record_test.go new file mode 100644 index 0000000..43ec433 --- /dev/null +++ b/internal/checks/record_test.go @@ -0,0 +1,74 @@ +package checks + +import ( + "reflect" + "strings" + "testing" + "testing/fstest" + + "github.com/nrepl/proof/internal/check" + "github.com/nrepl/proof/nrepl" +) + +// A recording is a client profile proof can read, whose checks send the +// same requests, in sessions of their own. +func TestRecordingsAreClientProfiles(t *testing.T) { + sent := func(m nrepl.Message) nrepl.Event { return nrepl.Event{Dir: nrepl.Sent, Msg: m} } + got := func(m nrepl.Message) nrepl.Event { return nrepl.Event{Dir: nrepl.Received, Msg: m} } + done := []any{"done"} + eval := nrepl.Message{"id": "4", "op": "eval", "session": "4f1c", "code": "value", + "file": "a \"file\"\nwith\ta \x01 and \xff", "line": int64(3), "\xfe": "", "nrepl.middleware.print/stream?": []any{}, + "nrepl.middleware.print/options": map[string]any{"right-margin": int64(70), "": []any{"x", int64(1)}}} + traffic := []check.Traffic{ + {Label: "connection 1", Events: []nrepl.Event{ + sent(nrepl.Message{"id": "1", "op": "clone", "client-name": "Test"}), + got(nrepl.Message{"id": "1", "new-session": "4f1c", "status": done}), + // The same session again is a new one for proof. + sent(nrepl.Message{"id": "2", "op": "clone"}), + got(nrepl.Message{"id": "2", "new-session": "4f1c", "status": done}), + sent(nrepl.Message{"id": "3", "op": "describe", "session": "4f1c"}), + sent(eval), + got(nrepl.Message{"id": "3", "ops": map[string]any{}, "status": done}), + got(nrepl.Message{"id": "4", "value": "3", "session": "4f1c"}), + {Dir: nrepl.Sent, Closed: true}, + }}, + // Frames that aren't dicts aren't requests. + {Label: "connection 2", Events: []nrepl.Event{{Dir: nrepl.Sent, Data: []any{}}}}, + // Sessions outlive connections, and only sessions stand for them. + {Label: "connection 3", Events: []nrepl.Event{ + sent(nrepl.Message{"id": "1", "op": "eval", "session": "4f1c", "code": "$s1"}), + }}, + } + recorded := RecordedProfile(traffic) + p, err := readClientProfile(fstest.MapFS{"test.toml": {Data: []byte(recorded)}}, "test.toml") + if err != nil { + t.Fatalf("%v\n%s", err, recorded) + } + if !strings.Contains(recorded, "\nsend = { op = \"clone\", client-name = \"Test\" }\n") { + t.Errorf("the clone isn't on one line, starting with the op:\n%s", recorded) + } + if p.Name != "Test" || len(p.Checks) != 2 || p.Checks[0].ID != "connection-1" || p.Checks[1].ID != "connection-3" { + t.Fatalf("got %+v from\n%s", p, recorded) + } + wantEval := map[string]any{"op": "eval", "session": "$s2", "code": "value", "file": "a \"file\"\nwith\ta \x01 and \ufffd", + "line": int64(3), "\ufffd": "", "nrepl.middleware.print/stream?": []any{}, + "nrepl.middleware.print/options": map[string]any{"right-margin": int64(70), "": []any{"x", int64(1)}}} + want := [][]clientStep{{ + {Send: request{"op": "clone", "client-name": "Test"}, NewSession: "s1"}, + {Send: request{"op": "clone"}, NewSession: "s2"}, + {Send: request{"op": "describe", "session": "$s2"}}, + {Send: request(wantEval)}, + }, { + {Send: request{"op": "eval", "session": "$s2", "code": "$s1"}}, + }} + for i, c := range p.Checks { + if !reflect.DeepEqual(c.Steps, want[i]) { + t.Errorf("%s: got steps\n%#v\nwant\n%#v\nfrom\n%s", c.ID, c.Steps, want[i], recorded) + } + } + c := p.Checks[0] + r := runFakeChecks(t, quirks{}, []*check.Check{{ID: c.ID, Title: c.Title, Run: c.replay}})[c.ID] + if r.Verdict != check.Pass { + t.Errorf("replaying it got %s %v", r.Verdict, r.Details) + } +}