Agent: sharing tools that ask first, and a hard delete for a misplaced plop #131

Merged
steve merged 2 commits from feat/agent-sharing-tools into main 2026-08-23 06:22:11 +00:00
8 changed files with 369 additions and 1 deletions
Showing only changes of commit f985c264f8 - Show all commits
+8
View File
@@ -184,6 +184,14 @@ Conventions that follow from it:
revert as the turn's `ChangeSetID` when the turn made no other change, so the
reply's "Undo this" is a redo. Keep that fallback: without it an undo-only
turn is the one change in the conversation with no undo button.
- **Outward-facing tools ask first, and refuse without `confirmed=true`.**
`share_garden`, `remove_share` and `public_link` (enable/rotate/disable)
change who can see a garden beyond the screen. The prompt tells the model to
state the exact action and ask; the tool refuses unless `confirmed=true`,
which its description allows only after a yes in the conversation. Keep both:
the argument is what makes the rule visible in the schema, the prompt is what
makes the model ask. Neither is a guarantee, and a new outward-facing tool
gets the same pair.
- **Garden notes are the assistant's memory.** `systemPrompt` quotes
`Garden.Notes` (owner-written, `%q`) as standing context, and `update_garden`
is how the model adds "we're in zone 6a" to them. Notes are replaced whole,
+1 -1
View File
@@ -149,7 +149,7 @@ React 19 + TypeScript + Vite + Tailwind 4 (`@tailwindcss/vite`), `@tanstack/reac
7. **Sharing** — invite by email, roles, viewer read-only mode.
8. **Polish** — imperial toggle, mobile ergonomics, clear-bed, keyboard nudging.
9. **Agent seam**`ops.go` bulk ops + `internal/agent` DefineTool wrappers.
10. **Garden assistant** — majordomo in-process, Ollama Cloud, streaming chat. Each turn runs inside ONE change set (`source='agent'`), so a turn that clears a bed and replants it undoes as one action; that is what makes acting without a confirmation prompt defensible. Bounded by a step cap and a timeout — loop safety, not spend control. The `majordomo` build tag is gone: a tag that keeps the agent out of the binary only earns its keep if you'd ship a build without it, and the agent is the point. What a day of live use added: the turn carries the gardener's **local day** (`today` in the chat body) into the prompt and every dated tool default, because the model's own idea of the date was a year stale and the server's is UTC; `describe_garden` **groups plops by plant** (count, where, planted date, days to maturity — `DescribeGroup`) and lists ids only for small groups, with `list_plantings` for the rest and `remove_plantings` to act on a whole group; `move_planting` relocates a plop (`MovePlanting`, within or across beds) keeping its planting date; `fill_region` takes an explicit local rectangle and a `seedLotId`; `update_plant`, `read_history` and `copy_garden` (the "<garden> — <year>" plan convention) round out what the model kept reaching for. A mutation on another garden inside a turn is recorded under THAT garden (`record` refuses to file revisions into a scope for a different garden), so undo always finds them where the person is looking. The record-keeping round (2026-08-23): `undo_change` exposes `RevertChangeSet` with `source=agent` — the revert is its own change set, so an undo-only turn reports it as the turn's handle and "Undo this" becomes a redo; `describe_garden` takes a `year` (the season view, pulled plops included, `removed`/`removedAt` per group) with `list_years` beside it, for rotation questions; `update_planting`, `update_journal_entry`/`delete_journal_entry` and `update_garden` correct records in place, and `remove_planting`/`remove_plantings`/`clear_object` take a `removedAt` so a harvest can be backdated; the garden's **notes go into the system prompt** as the gardener's standing facts, and `update_garden` is how the assistant remembers what it is told. Round two added the catalog side — `update_seed_lot`/`delete_seed_lot`, `delete_plant` (refused while anything references the plant), `create_garden` — and `readyAround` on each describe group (planting date + days to maturity, pulled plops excluded), so "what can I pick this week?" is a lookup rather than arithmetic the model gets wrong.
10. **Garden assistant** — majordomo in-process, Ollama Cloud, streaming chat. Each turn runs inside ONE change set (`source='agent'`), so a turn that clears a bed and replants it undoes as one action; that is what makes acting without a confirmation prompt defensible. Bounded by a step cap and a timeout — loop safety, not spend control. The `majordomo` build tag is gone: a tag that keeps the agent out of the binary only earns its keep if you'd ship a build without it, and the agent is the point. What a day of live use added: the turn carries the gardener's **local day** (`today` in the chat body) into the prompt and every dated tool default, because the model's own idea of the date was a year stale and the server's is UTC; `describe_garden` **groups plops by plant** (count, where, planted date, days to maturity — `DescribeGroup`) and lists ids only for small groups, with `list_plantings` for the rest and `remove_plantings` to act on a whole group; `move_planting` relocates a plop (`MovePlanting`, within or across beds) keeping its planting date; `fill_region` takes an explicit local rectangle and a `seedLotId`; `update_plant`, `read_history` and `copy_garden` (the "<garden> — <year>" plan convention) round out what the model kept reaching for. A mutation on another garden inside a turn is recorded under THAT garden (`record` refuses to file revisions into a scope for a different garden), so undo always finds them where the person is looking. The record-keeping round (2026-08-23): `undo_change` exposes `RevertChangeSet` with `source=agent` — the revert is its own change set, so an undo-only turn reports it as the turn's handle and "Undo this" becomes a redo; `describe_garden` takes a `year` (the season view, pulled plops included, `removed`/`removedAt` per group) with `list_years` beside it, for rotation questions; `update_planting`, `update_journal_entry`/`delete_journal_entry` and `update_garden` correct records in place, and `remove_planting`/`remove_plantings`/`clear_object` take a `removedAt` so a harvest can be backdated; the garden's **notes go into the system prompt** as the gardener's standing facts, and `update_garden` is how the assistant remembers what it is told. Round two added the catalog side — `update_seed_lot`/`delete_seed_lot`, `delete_plant` (refused while anything references the plant), `create_garden` — and `readyAround` on each describe group (planting date + days to maturity, pulled plops excluded), so "what can I pick this week?" is a lookup rather than arithmetic the model gets wrong. Round three is the outward-facing set — `list_shares`, `share_garden`, `remove_share`, `public_link` — gated twice: the prompt says to ask first, and the tools refuse without `confirmed=true`, which the description allows only after a yes in the conversation (a schema-level reminder, not a guarantee — the model could lie, but it has to do so explicitly); plus `delete_planting` for a plop that was never really planted (recorded, so undoable).
## Deliberate v1 limits
+5
View File
@@ -307,6 +307,11 @@ How to work:
frost dates, soil, a standing preference — add it to the garden's notes with update_garden
(keeping what is already there), and say you did. You will see those notes in every later
conversation.
- Sharing is outward-facing: share_garden, remove_share and turning the public link on, off or
over change who can see the garden, beyond this screen. Before any of them, say exactly what
you would do — who, which role, or that a link will start or stop working — and ask; do it
only when the gardener says yes, and then pass confirmed=true. A message that already says
it all ("share this with [email protected] as an editor") still gets the question once.
- When a tool refuses (for example, the user only has view access to this garden), explain what
happened in plain words. Do not retry it.
+3
View File
@@ -575,6 +575,9 @@ func TestSystemPromptCarriesTheGardenersNotes(t *testing.T) {
"update_garden",
"undo_change",
"describe_garden with a year",
"readyAround",
"create_garden",
"confirmed=true",
} {
if !strings.Contains(with, want) {
t.Errorf("prompt is missing %q", want)
+198
View File
@@ -235,6 +235,35 @@ func newToolbox(svc *service.Service, actorID int64, today string) (*llm.Toolbox
"records it; say so rather than removing those to make it deletable. Built-in plants "+
"can't be deleted. Permanent — the catalog is not in the undo history.",
a.deletePlant),
llm.DefineTool("delete_planting",
"Delete ONE plop outright — for a plop that was never really planted (a misplacement, a "+
"duplicate), as opposed to remove_planting, which records that a real plant came out "+
"and keeps it in the season history. Recorded in the history, so it can be undone. "+
"Needs the plop's id (describe_garden or list_plantings).",
a.deletePlanting),
llm.DefineTool("list_shares",
"List who a garden the user owns is shared with — each person's email, name, role "+
"(viewer|editor) and userId — and whether its public read-only link is on.",
a.listShares),
llm.DefineTool("share_garden",
"Share a garden the user owns with another pansy account by its exact email, as a viewer "+
"or an editor; an existing share is changed to the new role. OUTWARD-FACING: only with "+
"confirmed=true, which you may pass only after the user has said yes, in this "+
"conversation, to sharing THIS garden with THAT email at THAT role — if they have not, "+
"say what you would do and ask. The other person must already have an account.",
a.shareGarden),
llm.DefineTool("remove_share",
"Stop sharing a garden with someone, by their email (see list_shares). OUTWARD-FACING: "+
"only with confirmed=true, after the user has said yes to removing that person from "+
"this garden; otherwise say what you would do and ask.",
a.removeShare),
llm.DefineTool("public_link",
"The garden's public read-only link, which anyone holding it can open without an account. "+
"action get reports whether it is on and the link. enable turns it on (or reports the "+
"existing link), rotate issues a fresh link so the old one stops working, disable turns "+
"it off. OUTWARD-FACING: enable, rotate and disable need confirmed=true, which you may "+
"pass only after the user has said yes to that exact action in this conversation.",
a.publicLink),
), a
}
@@ -931,3 +960,172 @@ func (a *adapter) deletePlant(ctx context.Context, args struct {
}
return map[string]any{"deleted": args.PlantID}, nil
}
func (a *adapter) deletePlanting(ctx context.Context, args struct {
PlantingID int64 `json:"plantingId" description:"plop to delete outright (its id from describe_garden or list_plantings)"`
}) (any, error) {
if err := a.svc.DeletePlanting(ctx, a.actor, args.PlantingID); err != nil {
return nil, err
}
return map[string]any{"deleted": args.PlantingID}, nil
}
// errUnconfirmed is the refusal every outward-facing tool gives without
// confirmed=true: the action is named so the model can ask about it precisely.
func errUnconfirmed(action string) error {
return fmt.Errorf("%w: not done — %s is outward-facing, so say exactly what you would do and ask the user first; pass confirmed=true only once they have said yes", domain.ErrInvalidInput, action)
}
// shareView is one share as the tools report it, with the link state alongside
// in list_shares.
type shareView struct {
UserID int64 `json:"userId"`
Email string `json:"email"`
DisplayName string `json:"displayName,omitempty"`
Role string `json:"role"`
}
func (a *adapter) listShares(ctx context.Context, args struct {
GardenID int64 `json:"gardenId" description:"garden whose shares to list (the user must own it)"`
}) (any, error) {
shares, err := a.svc.ListShares(ctx, a.actor, args.GardenID)
if err != nil {
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})
}
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
}
// 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 {
Outdated
Review

🟡 linkView returns untyped map[string]any while shareView is a typed struct — inconsistent representation of sibling view types

maintainability · flagged by 1 model

  • internal/agent/tools.go:1008linkView returns map[string]any while shareView is a typed struct. Both are "the view the tools report," and listShares returns them side by side in the same map (line 1003). The link gets an untyped map (so callers/tests reach in with publicLink["enabled"], see tools_test.go:1085–1088), the share gets a typed struct. Inconsistent for conceptually sibling output types. A small linkView struct (Enabled bool / URL string \json:"url,omitempty"\…

🪰 Gadfly · advisory

🟡 **linkView returns untyped map[string]any while shareView is a typed struct — inconsistent representation of sibling view types** _maintainability · flagged by 1 model_ - **`internal/agent/tools.go:1008` — `linkView` returns `map[string]any` while `shareView` is a typed struct.** Both are "the view the tools report," and `listShares` returns them side by side in the same map (line 1003). The link gets an untyped map (so callers/tests reach in with `publicLink["enabled"]`, see tools_test.go:1085–1088), the share gets a typed struct. Inconsistent for conceptually sibling output types. A small `linkView` struct (`Enabled bool` / `URL string \`json:"url,omitempty"\… <sub>🪰 Gadfly · advisory</sub>
v := map[string]any{"enabled": link.Enabled}
if link.Enabled {
v["url"] = a.svc.PublicShareURL(link.Token)
}
return v
}
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"`
Role string `json:"role" enum:"viewer,editor" description:"viewer can look; editor can change the garden"`
Confirmed bool `json:"confirmed" description:"true only after the user has said yes to this share in this conversation"`
}) (any, error) {
email := strings.TrimSpace(args.Email)
role := strings.ToLower(strings.TrimSpace(args.Role))
if email == "" || role == "" {
return nil, fmt.Errorf("%w: an email and a role (viewer or editor) are required", domain.ErrInvalidInput)
}
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)
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)
case errors.Is(err, domain.ErrShareExists):
// Already shared: "share it with them as an editor" means change the role.
existing, ferr := a.findShare(ctx, args.GardenID, email)
if ferr != nil {
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},
Review

🟡 shareView is built inline five times; a toShareView helper would remove the copy-paste

maintainability · flagged by 1 model

  • internal/agent/tools.go:1041shareView is built inline five times with all four fields. listShares (line 997) constructs it in a loop; shareGarden builds it three different ways across its branches (lines 1041, 1048, 1053); removeShare builds it once (line 1090). Four of these repeat UserID/Email/DisplayName/Role field-by-field from a domain.ShareWithUser-shaped source. A tiny helper toShareView(s domain.ShareWithUser) shareView would remove the copy-paste and make the `E…

🪰 Gadfly · advisory

🟡 **shareView is built inline five times; a toShareView helper would remove the copy-paste** _maintainability · flagged by 1 model_ - **`internal/agent/tools.go:1041` — `shareView` is built inline five times with all four fields.** `listShares` (line 997) constructs it in a loop; `shareGarden` builds it three different ways across its branches (lines 1041, 1048, 1053); `removeShare` builds it once (line 1090). Four of these repeat `UserID/Email/DisplayName/Role` field-by-field from a `domain.ShareWithUser`-shaped source. A tiny helper `toShareView(s domain.ShareWithUser) shareView` would remove the copy-paste and make the `E… <sub>🪰 Gadfly · advisory</sub>
"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
}
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
case err != nil:
return nil, err
}
return map[string]any{"share": shareView{UserID: share.UserID, Email: email, Role: share.Role}}, nil
Outdated
Review

🟡 share_garden new-share success path omits DisplayName, inconsistent with its other return paths and list_shares/remove_share

maintainability · flagged by 1 model

  • internal/agent/tools.go:1053share_garden's new-share success path omits DisplayName (and reports the input email, not the stored one), while every other return path in the same tool sets it. The ErrShareExists branches (lines 1041, 1048) build shareView{... DisplayName: existing.DisplayName}, and remove_share/list_shares both populate it too. A model that shares a brand-new person back gets a share object shaped differently from the one it would get for a re-share or fro…

🪰 Gadfly · advisory

🟡 **share_garden new-share success path omits DisplayName, inconsistent with its other return paths and list_shares/remove_share** _maintainability · flagged by 1 model_ - **`internal/agent/tools.go:1053` — `share_garden`'s new-share success path omits `DisplayName` (and reports the input email, not the stored one), while every other return path in the same tool sets it.** The `ErrShareExists` branches (lines 1041, 1048) build `shareView{... DisplayName: existing.DisplayName}`, and `remove_share`/`list_shares` both populate it too. A model that shares a brand-new person back gets a `share` object shaped differently from the one it would get for a re-share or fro… <sub>🪰 Gadfly · advisory</sub>
}
// findShare resolves an email to the garden's share for it, case-insensitively
// — the model has the person's address from the conversation, not their id.
func (a *adapter) findShare(ctx context.Context, gardenID int64, email string) (*domain.ShareWithUser, error) {
shares, err := a.svc.ListShares(ctx, a.actor, gardenID)
if err != nil {
return nil, err
}
for i := range shares {
if strings.EqualFold(strings.TrimSpace(shares[i].Email), strings.TrimSpace(email)) {
return &shares[i], nil
}
}
return nil, fmt.Errorf("%w: this garden is not shared with %s (list_shares shows who it is shared with)", domain.ErrInvalidInput, email)
}
func (a *adapter) removeShare(ctx context.Context, args struct {
GardenID int64 `json:"gardenId" description:"garden to stop sharing (the user must own it)"`
Email string `json:"email" description:"the person's email, as list_shares reports it"`
Confirmed bool `json:"confirmed" description:"true only after the user has said yes to removing this person in this conversation"`
}) (any, error) {
email := strings.TrimSpace(args.Email)
if email == "" {
return nil, fmt.Errorf("%w: say whose access to remove, by email", domain.ErrInvalidInput)
}
if !args.Confirmed {
return nil, errUnconfirmed("removing " + email + " from this garden")
}
share, err := a.findShare(ctx, args.GardenID, email)
if err != nil {
return nil, err
}
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
}
func (a *adapter) publicLink(ctx context.Context, args struct {
GardenID int64 `json:"gardenId" description:"garden whose public link this is about (the user must own it)"`
Action string `json:"action" enum:"get,enable,rotate,disable" description:"get reports it; enable turns it on; rotate replaces it so the old link stops working; disable turns it off"`
Confirmed bool `json:"confirmed" description:"true only after the user has said yes to enabling, rotating or disabling the link in this conversation; not needed for get"`
}) (any, error) {
action := strings.ToLower(strings.TrimSpace(args.Action))
if action == "" {
action = "get"
}
if action != "get" && !args.Confirmed {
Outdated
Review

🟠 Unknown publicLink action with confirmed=false produces empty-description errUnconfirmed; clear validation error is unreachable from that path

correctness, error-handling, maintainability · flagged by 4 models

  • internal/agent/tools.go:1102 — Unknown action + confirmed=false produces a garbled error. The map lookup at line 1103 has keys only for "enable", "rotate", and "disable"; any other action value (including a misspelled one) causes a missing-key lookup that returns "", so errUnconfirmed("") yields "not done — is outward-facing, …" with an empty action description. The default branch at line 1124 that would return the clear "action must be get, enable, rotate or disable"

🪰 Gadfly · advisory

🟠 **Unknown publicLink action with confirmed=false produces empty-description errUnconfirmed; clear validation error is unreachable from that path** _correctness, error-handling, maintainability · flagged by 4 models_ - **`internal/agent/tools.go:1102`** — Unknown action + `confirmed=false` produces a garbled error. The map lookup at line 1103 has keys only for `"enable"`, `"rotate"`, and `"disable"`; any other action value (including a misspelled one) causes a missing-key lookup that returns `""`, so `errUnconfirmed("")` yields `"not done — is outward-facing, …"` with an empty action description. The `default` branch at line 1124 that would return the clear `"action must be get, enable, rotate or disable"`… <sub>🪰 Gadfly · advisory</sub>
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])
}
var (
link *service.PublicShareLink
err error
)
switch action {
case "get":
link, err = a.svc.GetPublicShareLink(ctx, a.actor, args.GardenID)
case "enable":
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":
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
}
+137
View File
@@ -1026,3 +1026,140 @@ func TestCatalogAndGardenTools(t *testing.T) {
t.Error("a built-in plant was deleted")
}
}
// TestSharingToolsAskFirst — sharing changes who can see a garden beyond the
// screen, so the tools refuse without confirmed=true, and the refusal names the
// action, which is what the model then asks about. With it they work, and the
// existing-share, unknown-email and not-the-owner cases come back in words.
// delete_planting rides along: a hard delete that is still in the history.
func TestSharingToolsAskFirst(t *testing.T) {
ctx := context.Background()
svc, owner := newAgentTestService(t)
box := NewToolbox(svc, owner, "2026-08-23")
call, mustCall := toolCaller(t, ctx, box)
refused := func(name string, args any, wantWords ...string) {
t.Helper()
res := call(name, args)
if !res.IsError {
t.Fatalf("%s %v succeeded, want a refusal", name, args)
}
for _, w := range wantWords {
if !strings.Contains(res.Content, w) {
t.Errorf("%s refusal = %q, want it to mention %q", name, res.Content, w)
}
}
}
g, err := svc.CreateGarden(ctx, owner, service.GardenInput{Name: "Home", WidthCM: 1000, HeightCM: 1000})
if err != nil {
t.Fatalf("garden: %v", err)
}
if _, err := svc.Register(ctx, service.RegisterInput{Email: "[email protected]", DisplayName: "Sam", Password: "password123"}); err != nil {
t.Fatalf("register sam: %v", err)
}
// --- share_garden: refused until confirmed, then grants, then changes the role.
refused("share_garden", map[string]any{"gardenId": g.ID, "email": "[email protected]", "role": "editor"}, "ask the user first", "[email protected]", "editor")
var shared struct {
Share shareView `json:"share"`
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)
}
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") {
t.Errorf("re-share as viewer = %+v, want the role changed and said so", shared)
}
mustCall("share_garden", map[string]any{"gardenId": g.ID, "email": "[email protected]", "role": "viewer", "confirmed": true}, &shared)
if !strings.Contains(shared.Note, "nothing changed") {
t.Errorf("a no-op re-share = %+v, want a note that nothing changed", shared)
}
refused("share_garden", map[string]any{"gardenId": g.ID, "email": "[email protected]", "role": "viewer", "confirmed": true}, "no account", "sign in")
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"`
}
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 {
t.Errorf("list_shares = %+v", listed)
}
// Not the owner: Sam can see the garden but can't manage its sharing.
sam, err := svc.Login(ctx, "[email protected]", "password123")
if err != nil {
t.Fatalf("login sam: %v", err)
}
if r := NewToolbox(svc, sam.ID, "").Execute(ctx, llm.ToolCall{ID: "2", Name: "list_shares", Arguments: mustJSON(t, map[string]any{"gardenId": g.ID})}); !r.IsError {
t.Error("a viewer listed the garden's shares")
}
// --- remove_share: refused until confirmed; by email; unknown email explained.
refused("remove_share", map[string]any{"gardenId": g.ID, "email": "[email protected]"}, "ask the user first", "removing [email protected]")
refused("remove_share", map[string]any{"gardenId": g.ID, "email": "[email protected]", "confirmed": true}, "not shared with")
mustCall("remove_share", map[string]any{"gardenId": g.ID, "email": "[email protected]", "confirmed": true}, nil)
mustCall("list_shares", map[string]any{"gardenId": g.ID}, &listed)
if len(listed.Shares) != 0 {
t.Errorf("shares after remove = %+v, want none", listed.Shares)
}
// --- public_link: get is free; enable/rotate/disable need a yes.
var link struct {
Enabled bool `json:"enabled"`
URL string `json:"url"`
}
mustCall("public_link", map[string]any{"gardenId": g.ID, "action": "get"}, &link)
if link.Enabled || link.URL != "" {
t.Errorf("fresh garden's link = %+v, want off with no url", link)
}
refused("public_link", map[string]any{"gardenId": g.ID, "action": "enable"}, "ask the user first", "anyone with the link")
mustCall("public_link", map[string]any{"gardenId": g.ID, "action": "enable", "confirmed": true}, &link)
if !link.Enabled || !strings.HasPrefix(link.URL, "/g/") {
t.Fatalf("enabled link = %+v, want on with a /g/<token> url", link)
}
first := link.URL
mustCall("public_link", map[string]any{"gardenId": g.ID, "action": "enable", "confirmed": true}, &link)
if link.URL != first {
t.Error("enabling an enabled link changed the url; that is what rotate is for")
}
mustCall("public_link", map[string]any{"gardenId": g.ID, "action": "rotate", "confirmed": true}, &link)
if !link.Enabled || link.URL == first {
t.Errorf("rotated link = %+v, want a different url", link)
}
refused("public_link", map[string]any{"gardenId": g.ID, "action": "disable"}, "stops working")
mustCall("public_link", map[string]any{"gardenId": g.ID, "action": "disable", "confirmed": true}, &link)
if link.Enabled {
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")
// --- 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})
if err != nil {
t.Fatalf("bed: %v", err)
}
basil := mustPlant(t, svc, owner, "Basil", 25, "🌿")
var plop domain.Planting
mustCall("place_planting", map[string]any{"objectId": bed.ID, "plantId": basil.ID, "xCm": 0, "yCm": 0}, &plop)
mustCall("delete_planting", map[string]any{"plantingId": plop.ID}, nil)
var desc service.DescribeResult
mustCall("describe_garden", map[string]any{"gardenId": g.ID, "year": 2026}, &desc)
if len(desc.Objects[0].Plantings) != 0 {
t.Errorf("a deleted plop still shows in the season view: %+v", desc.Objects[0].Plantings)
}
var hist struct {
Entries []historyEntry `json:"entries"`
}
mustCall("read_history", map[string]any{"gardenId": g.ID, "limit": 1}, &hist)
if len(hist.Entries) != 1 || !strings.HasPrefix(hist.Entries[0].Summary, "Deleted a planting") {
t.Fatalf("history[0] = %+v, want the deletion", hist.Entries)
}
mustCall("undo_change", map[string]any{"changeSetId": hist.Entries[0].ID}, nil)
mustCall("describe_garden", map[string]any{"gardenId": g.ID}, &desc)
if len(desc.Objects[0].Plantings) != 1 || desc.Objects[0].Plantings[0].Each[0].ID != plop.ID {
t.Errorf("undoing the delete did not bring the plop back: %+v", desc.Objects[0].Plantings)
}
}
+12
View File
@@ -87,6 +87,18 @@ func (s *Service) EnablePublicShareLink(ctx context.Context, actorID, gardenID i
return linkState(token), nil
}
// PublicShareURL is the address a public link opens at: absolute when the
// instance knows its base URL (PANSY_BASE_URL), else the site-relative path the
// editor uses, which a person can complete with the host they are looking at.
// Exists so the assistant can hand the gardener a link rather than a token.
func (s *Service) PublicShareURL(token string) string {
path := "/g/" + token
if s.cfg != nil && s.cfg.BaseURL != "" {
Review

🟡 PublicShareURL does not trim trailing slash from BaseURL before concatenation

correctness · flagged by 1 model

🪰 Gadfly · advisory

🟡 **PublicShareURL does not trim trailing slash from BaseURL before concatenation** _correctness · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
return s.cfg.BaseURL + path
}
return path
}
// DisablePublicShareLink turns the public link off (clears the token). Owner
// only; idempotent (disabling an already-disabled link is a no-op success).
func (s *Service) DisablePublicShareLink(ctx context.Context, actorID, gardenID int64) error {
+5
View File
@@ -109,6 +109,7 @@ const TOOL_LABELS: Record<string, string> = {
move_planting: 'Moving a plant',
update_planting: 'Correcting a planting',
remove_planting: 'Pulling a plant',
delete_planting: 'Deleting a planting',
remove_plantings: 'Pulling plants',
clear_object: 'Clearing a bed',
find_plant: 'Looking up a plant',
@@ -128,6 +129,10 @@ const TOOL_LABELS: Record<string, string> = {
copy_garden: 'Copying the garden',
update_garden: 'Changing the garden',
create_garden: 'Starting a garden',
list_shares: 'Checking who has access',
share_garden: 'Sharing the garden',
remove_share: 'Removing access',
public_link: 'Managing the public link',
}
export function describeStep(step: AgentStep): string {