Agent: sharing tools that ask first, and a hard delete for a misplaced plop #131
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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,191 @@ 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 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.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.
|
||||
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)
|
||||
}
|
||||
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"`
|
||||
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))
|
||||
}
|
||||
_, err := a.svc.AddShare(ctx, a.actor, args.GardenID, email, role)
|
||||
switch {
|
||||
case errors.Is(err, domain.ErrShareUserNotFound):
|
||||
|
gitea-actions
commented
🟡 shareView is built inline five times; a toShareView helper would remove the copy-paste maintainability · flagged by 1 model
🪰 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>
|
||||
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": toShareView(existing), "note": "already shared with them at that role; nothing changed"}, nil
|
||||
}
|
||||
if _, err := a.svc.UpdateShareRole(ctx, a.actor, args.GardenID, existing.UserID, role); err != nil {
|
||||
return nil, err
|
||||
|
gitea-actions
commented
🟡 share_garden new-share success path omits DisplayName, inconsistent with its other return paths and list_shares/remove_share maintainability · flagged by 1 model
🪰 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>
|
||||
}
|
||||
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
|
||||
}
|
||||
// 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
|
||||
// — 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": toShareView(share)}, nil
|
||||
|
gitea-actions
commented
🟠 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
🪰 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>
|
||||
}
|
||||
|
||||
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"
|
||||
}
|
||||
// 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
|
||||
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)
|
||||
default: // disable
|
||||
if err = a.svc.DisablePublicShareLink(ctx, a.actor, args.GardenID); err == nil {
|
||||
link = &service.PublicShareLink{Enabled: false}
|
||||
}
|
||||
}
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return a.linkOf(link), nil
|
||||
}
|
||||
|
||||
@@ -1026,3 +1026,143 @@ 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]" || 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") {
|
||||
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 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 {
|
||||
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")
|
||||
// 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})
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -87,6 +87,20 @@ 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 != "" {
|
||||
|
gitea-actions
commented
🟡 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>
|
||||
// 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
|
||||
}
|
||||
|
||||
// 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 {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user
🟡 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—linkViewreturnsmap[string]anywhileshareViewis a typed struct. Both are "the view the tools report," andlistSharesreturns them side by side in the same map (line 1003). The link gets an untyped map (so callers/tests reach in withpublicLink["enabled"], see tools_test.go:1085–1088), the share gets a typed struct. Inconsistent for conceptually sibling output types. A smalllinkViewstruct (Enabled bool/URL string \json:"url,omitempty"\…🪰 Gadfly · advisory