internal/agent/tools.go

@@ -993,26 +993,35 @@ func (a *adapter) listShares(ctx context.Context, args struct {
 		return nil, err
 	}
 	out := make([]shareView, 0, len(shares))
-	for _, sh := range shares {
-		out = append(out, shareView{UserID: sh.UserID, Email: sh.Email, DisplayName: sh.DisplayName, Role: sh.Role})
+	for i := range shares {
+		out = append(out, toShareView(&shares[i]))
 	}
 	link, err := a.svc.GetPublicShareLink(ctx, a.actor, args.GardenID)
 	if err != nil {
 		return nil, err
 	}
-	return map[string]any{"shares": out, "publicLink": a.linkView(link)}, nil
+	return map[string]any{"shares": out, "publicLink": a.linkOf(link)}, nil
 }
 
 // linkView is a public link as the tools report it: on/off, and the address
 // when on. The token itself is only ever shown as part of that address.
-func (a *adapter) linkView(link *service.PublicShareLink) map[string]any {
-	v := map[string]any{"enabled": link.Enabled}
+type linkView struct {
+	Enabled bool   `json:"enabled"`
+	URL     string `json:"url,omitempty"`
+}
+
+func (a *adapter) linkOf(link *service.PublicShareLink) linkView {
+	v := linkView{Enabled: link.Enabled}
 	if link.Enabled {
-		v["url"] = a.svc.PublicShareURL(link.Token)
+		v.URL = a.svc.PublicShareURL(link.Token)
 	}
 	return v
 }
 
+func toShareView(sh *domain.ShareWithUser) shareView {
+	return shareView{UserID: sh.UserID, Email: sh.Email, DisplayName: sh.DisplayName, Role: sh.Role}
+}
+
 func (a *adapter) shareGarden(ctx context.Context, args struct {
 	GardenID  int64  `json:"gardenId" description:"garden to share (the user must own it)"`
 	Email     string `json:"email" description:"the other person's pansy account email, exactly as the user gave it"`
@@ -1027,7 +1036,7 @@ func (a *adapter) shareGarden(ctx context.Context, args struct {
 	if !args.Confirmed {
 		return nil, errUnconfirmed(fmt.Sprintf("sharing this garden with %s as %s", email, role))
 	}
-	share, err := a.svc.AddShare(ctx, a.actor, args.GardenID, email, role)
+	_, err := a.svc.AddShare(ctx, a.actor, args.GardenID, email, role)
 	switch {
 	case errors.Is(err, domain.ErrShareUserNotFound):
 		return nil, fmt.Errorf("%w — they need to sign in to pansy once before a garden can be shared with them", err)
@@ -1038,19 +1047,22 @@ func (a *adapter) shareGarden(ctx context.Context, args struct {
 			return nil, ferr
 		}
 		if existing.Role == role {
-			return map[string]any{"share": shareView{UserID: existing.UserID, Email: existing.Email, DisplayName: existing.DisplayName, Role: existing.Role},
-				"note": "already shared with them at that role; nothing changed"}, nil
+			return map[string]any{"share": toShareView(existing), "note": "already shared with them at that role; nothing changed"}, nil
 		}
-		updated, uerr := a.svc.UpdateShareRole(ctx, a.actor, args.GardenID, existing.UserID, role)
-		if uerr != nil {
-			return nil, uerr
+		if _, err := a.svc.UpdateShareRole(ctx, a.actor, args.GardenID, existing.UserID, role); err != nil {
+			return nil, err
 		}
-		return map[string]any{"share": shareView{UserID: updated.UserID, Email: existing.Email, DisplayName: existing.DisplayName, Role: updated.Role},
-			"note": "they already had access; their role is now " + updated.Role}, nil
+		existing.Role = role
+		return map[string]any{"share": toShareView(existing), "note": "they already had access; their role is now " + role}, nil
 	case err != nil:
 		return nil, err
 	}
-	return map[string]any{"share": shareView{UserID: share.UserID, Email: email, Role: share.Role}}, nil
+	// Read the share back so every path reports the same shape, name included.
+	created, err := a.findShare(ctx, args.GardenID, email)
+	if err != nil {
+		return nil, err
+	}
+	return map[string]any{"share": toShareView(created)}, nil
 }
 
 // findShare resolves an email to the garden's share for it, case-insensitively
@@ -1087,7 +1099,7 @@ func (a *adapter) removeShare(ctx context.Context, args struct {
 	if err := a.svc.RemoveShare(ctx, a.actor, args.GardenID, share.UserID); err != nil {
 		return nil, err
 	}
-	return map[string]any{"removed": shareView{UserID: share.UserID, Email: share.Email, DisplayName: share.DisplayName, Role: share.Role}}, nil
+	return map[string]any{"removed": toShareView(share)}, nil
 }
 
 func (a *adapter) publicLink(ctx context.Context, args struct {
@@ -1099,12 +1111,21 @@ func (a *adapter) publicLink(ctx context.Context, args struct {
 	if action == "" {
 		action = "get"
 	}
-	if action != "get" && !args.Confirmed {
-		return nil, errUnconfirmed(map[string]string{
-			"enable":  "turning the public link on, so anyone with the link can see this garden",
-			"rotate":  "rotating the public link, so the old link stops working",
-			"disable": "turning the public link off, so the link stops working",
-		}[action])
+	// What each outward-facing action does, in the words the model should ask
+	// with. Checked before the confirmation gate, so an unknown action is told
+	// it is unknown rather than asked to confirm nothing in particular.
+	asks := map[string]string{
+		"get":     "",
+		"enable":  "turning the public link on, so anyone with the link can see this garden",
+		"rotate":  "rotating the public link, so the old link stops working",
+		"disable": "turning the public link off, so the link stops working",
+	}
+	ask, known := asks[action]
+	if !known {
+		return nil, fmt.Errorf("%w: action must be get, enable, rotate or disable", domain.ErrInvalidInput)
+	}
+	if ask != "" && !args.Confirmed {
+		return nil, errUnconfirmed(ask)
 	}
 	var (
 		link *service.PublicShareLink
@@ -1117,15 +1138,13 @@ func (a *adapter) publicLink(ctx context.Context, args struct {
 		link, err = a.svc.EnablePublicShareLink(ctx, a.actor, args.GardenID, false)
 	case "rotate":
 		link, err = a.svc.EnablePublicShareLink(ctx, a.actor, args.GardenID, true)
-	case "disable":
+	default: // disable
 		if err = a.svc.DisablePublicShareLink(ctx, a.actor, args.GardenID); err == nil {
 			link = &service.PublicShareLink{Enabled: false}
 		}
-	default:
-		return nil, fmt.Errorf("%w: action must be get, enable, rotate or disable", domain.ErrInvalidInput)
 	}
 	if err != nil {
 		return nil, err
 	}
-	return a.linkView(link), nil
+	return a.linkOf(link), nil
 }

internal/agent/tools_test.go

@@ -1066,8 +1066,8 @@ func TestSharingToolsAskFirst(t *testing.T) {
 		Note  string    `json:"note"`
 	}
 	mustCall("share_garden", map[string]any{"gardenId": g.ID, "email": "[email protected]", "role": "editor", "confirmed": true}, &shared)
-	if shared.Share.Role != domain.RoleEditor || shared.Share.Email != "[email protected]" {
-		t.Errorf("share = %+v, want sam as editor", shared)
+	if shared.Share.Role != domain.RoleEditor || shared.Share.Email != "[email protected]" || shared.Share.DisplayName != "Sam" {
+		t.Errorf("share = %+v, want Sam as editor, named", shared)
 	}
 	mustCall("share_garden", map[string]any{"gardenId": g.ID, "email": "[email protected]", "role": "viewer", "confirmed": true}, &shared)
 	if shared.Share.Role != domain.RoleViewer || !strings.Contains(shared.Note, "now viewer") {
@@ -1081,11 +1081,11 @@ func TestSharingToolsAskFirst(t *testing.T) {
 	refused("share_garden", map[string]any{"gardenId": g.ID, "email": "[email protected]", "role": "owner", "confirmed": true})
 
 	var listed struct {
-		Shares     []shareView    `json:"shares"`
-		PublicLink map[string]any `json:"publicLink"`
+		Shares     []shareView `json:"shares"`
+		PublicLink linkView    `json:"publicLink"`
 	}
 	mustCall("list_shares", map[string]any{"gardenId": g.ID}, &listed)
-	if len(listed.Shares) != 1 || listed.Shares[0].Email != "[email protected]" || listed.Shares[0].Role != domain.RoleViewer || listed.PublicLink["enabled"] != false {
+	if len(listed.Shares) != 1 || listed.Shares[0].Email != "[email protected]" || listed.Shares[0].Role != domain.RoleViewer || listed.PublicLink.Enabled {
 		t.Errorf("list_shares = %+v", listed)
 	}
 	// Not the owner: Sam can see the garden but can't manage its sharing.
@@ -1135,6 +1135,9 @@ func TestSharingToolsAskFirst(t *testing.T) {
 		t.Error("the link is still on after disable")
 	}
 	refused("public_link", map[string]any{"gardenId": g.ID, "action": "share", "confirmed": true}, "get, enable, rotate or disable")
+	// An unknown action is unknown whether or not it was confirmed — not a
+	// request to confirm nothing in particular.
+	refused("public_link", map[string]any{"gardenId": g.ID, "action": "share"}, "get, enable, rotate or disable")
 
 	// --- delete_planting: gone from every view, but in the history — undoable.
 	bed, err := svc.CreateObject(ctx, owner, g.ID, service.ObjectInput{Kind: domain.KindBed, Name: "Bed", XCM: 500, YCM: 500, WidthCM: 200, HeightCM: 200})

internal/service/public.go

@@ -94,7 +94,9 @@ func (s *Service) EnablePublicShareLink(ctx context.Context, actorID, gardenID i
 func (s *Service) PublicShareURL(token string) string {
 	path := "/g/" + token
 	if s.cfg != nil && s.cfg.BaseURL != "" {
-		return s.cfg.BaseURL + path
+		// config trims the trailing slash already; a Config built by hand
+		// (tests, an embedder) may not have.
+		return strings.TrimRight(s.cfg.BaseURL, "/") + path
 	}
 	return path
 }