A profile's operation may say where the vendor's record is, and which read names the resource (kiff/tafs#498)

CHANGELOG.md

@@ -7,6 +7,13 @@ version headings refer to releases, not merely to merged pull requests.
 
 ### Added
 
+- Two optional fields on a profile's operation, for KIFF's "Did this happen?"
+  ask: `record_url`, where the owner looks at the vendor's record of the
+  resource (an https address with the resource's id path-escaped into
+  `{resource}`, reviewed catalog only), and `label_read`, a read of the same
+  profile that gives the resource's own name (for an operation with a
+  `resource`). A profile that names neither is unchanged.
+
 - A call whose outcome comes later (the slow-tools design): a write's
   profile may say `completes: {event, key, event_key, timeout_s, words}`.
   The relay then refuses the call while the taf doesn't listen for that

docs/connections.md

@@ -377,7 +377,12 @@ revision so a replay cannot adopt a later profile or send the write again.
 
 Profiles can require arguments before any vendor call. A marker must name a
 string field in the vendor schema; planque fills that field from the permit's
-operation id and refuses a caller-supplied value there. Agent proposals
+operation id and refuses a caller-supplied value there. An operation may say
+where the owner looks at the vendor's own record of its resource
+(`record_url`, an https address with the resource's id path-escaped into
+`{resource}`; reviewed catalog only, as `no_effect` is) and which read of the
+profile gives the resource's own name (`label_read`, for an operation with a
+`resource`): KIFF shows both when it asks the owner "Did this happen?". Agent proposals
 cannot claim reviewed catalog status or successful test calls. Profiles needing a nominal-currency lookup definition are refused until
 that definition is supported. Version-header pins use the reviewed
 configuration described below.

internal/profile/profile.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/url"
 	"regexp"
 	"slices"
 	"strings"
@@ -49,6 +50,9 @@ type Profile struct {
 	Operations []Operation `json:"operations"`
 }
 
+// RecordPlaceholder is what a record_url carries where the resource's id goes.
+const RecordPlaceholder = "{resource}"
+
 // Pin is how the profile is pinned to a vendor version.
 type Pin struct {
 	// Kind is "header" (a version header, such as Stripe-Version),
@@ -81,6 +85,17 @@ type Operation struct {
 	NoEffect   *NoEffect   `json:"no_effect,omitempty"`
 	// RequiredArguments are arguments planque requires before sending.
 	RequiredArguments []string `json:"required_arguments,omitempty"`
+	// RecordURL is where the owner looks at the vendor's record of this
+	// operation's resource ("https://dashboard.stripe.com/payments/{resource}"):
+	// an https address at the vendor, with the resource's id path-escaped
+	// into {resource}. Reviewed catalog only: a drafted profile carries none,
+	// so no link reaches the owner that no reviewer saw. KIFF shows it
+	// when it asks "Did this happen?".
+	RecordURL string `json:"record_url,omitempty"`
+	// LabelRead is a read of the same profile that gives the resource's own
+	// name (a customer's name, an invoice's number), for the words KIFF
+	// asks with; it needs a resource.
+	LabelRead *Read `json:"label_read,omitempty"`
 	// Completes says the write's outcome comes later, as one of the app's
 	// events: the run that made the call waits for it (kiff/planque#86's
 	// slow tools). Only a write.
@@ -394,6 +409,25 @@ func (p Profile) validateOp(o Operation, byTool map[string]Operation) error {
 			return fmt.Errorf("required_arguments: %q isn't an argument name", a)
 		}
 	}
+	if u := o.RecordURL; u != "" {
+		if p.Source != "catalog" {
+			return errors.New("record_url comes only from a catalog entry merged by review (reviewed catalog only)")
+		}
+		if err := checkRecordURL(u); err != nil {
+			return fmt.Errorf("record_url: %w", err)
+		}
+	}
+	if l := o.LabelRead; l != nil {
+		if o.Resource == nil {
+			return errors.New("label_read names the resource's own name, so the operation needs a resource")
+		}
+		if err := readOf(l.Tool, l.Args, byTool); err != nil {
+			return fmt.Errorf("label_read: %w", err)
+		}
+		if !pathExpr.MatchString(l.Path) {
+			return errors.New("label_read.path is a dotted path")
+		}
+	}
 	if c := o.Completes; c != nil {
 		switch {
 		case !eventSlug.MatchString(c.Event):
@@ -411,6 +445,46 @@ func (p Profile) validateOp(o Operation, byTool map[string]Operation) error {
 	return nil
 }
 
+// checkRecordURL accepts an https address at a host, with no credentials in
+// it and at most one {resource} placeholder, which is the only brace in it
+// and sits in the path: never in the query or the fragment, where an id's
+// "&" or "=" would add a parameter the vendor's page may act on.
+func checkRecordURL(u string) error {
+	if len(u) > 512 || !strings.HasPrefix(u, "https://") {
+		return errors.New("an https address at the vendor, at most 512 characters")
+	}
+	if strings.Count(u, RecordPlaceholder) > 1 || strings.Count(u, "{") != strings.Count(u, RecordPlaceholder) || strings.Count(u, "}") != strings.Count(u, RecordPlaceholder) {
+		return errors.New("{resource} is the only placeholder, at most once")
+	}
+	parsed, err := url.Parse(strings.ReplaceAll(u, RecordPlaceholder, "x"))
+	if err != nil || parsed.Scheme != "https" || parsed.Host == "" || parsed.User != nil {
+		return errors.New("an https address at a host, with no credentials in it")
+	}
+	// Where the placeholder sits, by position: after the host, before any "?" or "#".
+	if at := strings.Index(u, RecordPlaceholder); at >= 0 {
+		host := len("https://") + len(parsed.Host)
+		stop := strings.IndexAny(u, "?#")
+		if at < host || (stop >= 0 && at > stop) {
+			return errors.New("{resource} goes in the address's path, never its host, query or fragment")
+		}
+	}
+	return nil
+}
+
+// RecordURL fills template with the resource's id, path-escaped, so an id
+// holding "/", "?", "#" or "@" stays inside the vendor's host and path. An
+// empty id, or "." or "..", which a browser reads as a step up the path,
+// gives no link: KIFF then shows the vendor's name alone.
+func RecordURL(template, resource string) string {
+	if !strings.Contains(template, RecordPlaceholder) {
+		return template
+	}
+	if resource == "" || resource == "." || resource == ".." {
+		return ""
+	}
+	return strings.ReplaceAll(template, RecordPlaceholder, url.PathEscape(resource))
+}
+
 func validateAmount(a Amount, byTool map[string]Operation) error {
 	switch a.From {
 	case "argument":

internal/profile/profile_test.go

@@ -54,3 +54,89 @@ func TestProfileRefusesUnknownAndTrailingFields(t *testing.T) {
 		}
 	}
 }
+
+// An operation may say where the owner looks at the vendor's record of its
+// resource (record_url, reviewed catalog only) and which read gives the
+// resource's own name (label_read); the id is path-escaped into the address.
+func TestRecordURLAndLabelRead(t *testing.T) {
+	raw, _ := os.ReadFile("testdata/stripe-refunds.json")
+	with := func(changes map[string]any, source string) ([]byte, error) {
+		var p map[string]any
+		if err := json.Unmarshal(raw, &p); err != nil {
+			t.Fatal(err)
+		}
+		if source != "" {
+			p["source"] = source
+			delete(p, "catalog_entry")
+		}
+		ops := p["operations"].([]any)
+		for _, o := range ops {
+			op := o.(map[string]any)
+			if op["tool"] == "create_refund" {
+				for k, v := range changes {
+					op[k] = v
+				}
+			}
+		}
+		out, _ := json.Marshal(p)
+		_, err := Parse(out)
+		return out, err
+	}
+	good := map[string]any{"record_url": "https://dashboard.stripe.com/payments/{resource}", "label_read": map[string]any{"tool": "get_payment_intent", "args": map[string]string{"payment_intent": "$args.payment_intent"}, "path": "description"}}
+	if _, err := with(good, ""); err != nil {
+		t.Fatalf("a record_url and a label_read on a catalog profile: %v", err)
+	}
+	for name, c := range map[string]struct {
+		changes map[string]any
+		source  string
+		want    string
+	}{
+		"a drafted profile's record_url":  {map[string]any{"record_url": "https://dashboard.stripe.com/payments/{resource}"}, "drafted", "reviewed catalog only"},
+		"http":                            {map[string]any{"record_url": "http://dashboard.stripe.com/payments/{resource}"}, "", "https"},
+		"credentials in it":               {map[string]any{"record_url": "https://user:pw@dashboard.stripe.com/{resource}"}, "", "no credentials"},
+		"two placeholders":                {map[string]any{"record_url": "https://dashboard.stripe.com/{resource}/{resource}"}, "", "at most once"},
+		"another brace":                   {map[string]any{"record_url": "https://dashboard.stripe.com/{account}/payments"}, "", "only placeholder"},
+		"the placeholder as the host":     {map[string]any{"record_url": "https://{resource}/payments"}, "", "path"},
+		"the placeholder in the query":    {map[string]any{"record_url": "https://dash.example.com/resources/view?id={resource}"}, "", "path"},
+		"the placeholder in the fragment": {map[string]any{"record_url": "https://dash.example.com/resource#{resource}"}, "", "path"},
+		"a label_read that is a write":    {map[string]any{"label_read": map[string]any{"tool": "create_refund", "path": "id"}}, "", "is a write"},
+		"a label_read's bad path":         {map[string]any{"label_read": map[string]any{"tool": "get_payment_intent", "path": "../x"}}, "", "dotted path"},
+	} {
+		if _, err := with(c.changes, c.source); err == nil || !strings.Contains(err.Error(), c.want) {
+			t.Errorf("%s: %v, want %q", name, err, c.want)
+		}
+	}
+	// A label_read needs a resource: list_contacts in another vector has none.
+	raw2, _ := os.ReadFile("testdata/holded-invoices.json")
+	var p map[string]any
+	_ = json.Unmarshal(raw2, &p)
+	for _, o := range p["operations"].([]any) {
+		if op := o.(map[string]any); op["tool"] == "list_contacts" {
+			op["label_read"] = map[string]any{"tool": "list_contacts", "path": "name"}
+		}
+	}
+	out, _ := json.Marshal(p)
+	if _, err := Parse(out); err == nil || !strings.Contains(err.Error(), "needs a resource") {
+		t.Errorf("a label_read on an operation without a resource: %v", err)
+	}
+	// The id is path-escaped into the address: it can't leave the vendor's host or path.
+	for id, want := range map[string]string{
+		"pi_3Abc":          "https://dashboard.stripe.com/payments/pi_3Abc",
+		"../../admin":      "https://dashboard.stripe.com/payments/..%2F..%2Fadmin",
+		"x?y=1#z":          "https://dashboard.stripe.com/payments/x%3Fy=1%23z",
+		"a@evil.example/b": "https://dashboard.stripe.com/payments/a@evil.example%2Fb",
+	} {
+		if got := RecordURL("https://dashboard.stripe.com/payments/{resource}", id); got != want {
+			t.Errorf("%q: %s, want %s", id, got, want)
+		}
+	}
+	if got := RecordURL("https://dashboard.stripe.com/payments", "pi_1"); got != "https://dashboard.stripe.com/payments" {
+		t.Errorf("no placeholder: %s", got)
+	}
+	// An id a browser would read as a step up the path, or none, gives no link.
+	for _, id := range []string{"", ".", ".."} {
+		if got := RecordURL("https://dashboard.stripe.com/payments/{resource}", id); got != "" {
+			t.Errorf("%q: %s, want no link", id, got)
+		}
+	}
+}