Address Gadfly review on the garden copy

* api: detect an absent copy body by io.EOF from the decoder rather than
  Content-Length. A chunked request reports -1, not 0, so the length test
  read an empty chunked body as malformed and 400'd instead of taking the
  default-name path. Covered by a new unknown-length test.

* store: hoist the garden_objects/plantings INSERTs into shared
  objectInsert/plantingInsert statements with matching *InsertArgs
  helpers. CopyGarden had its own copies of both column lists, so a
  column added to the table could be wired into Create* and silently
  dropped from a copy.

* store: move the queryer interface to sqlite.go, next to the other
  shared query plumbing, instead of users.go.

* web: derive a copy's prefilled name via defaultCopyName, mirroring the
  server's copyName including its 200-BYTE cap. The inline
  `${name} (copy)` both duplicated the suffix and, for a garden whose
  name was already at the cap, prefilled an over-long name that the
  server rejected with a 400. Unit-tested, including multi-byte
  truncation on a code-point boundary.

* test: drop a throwaway `_ = kept`.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

internal/api/api.go

@@ -81,7 +81,7 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
 	gardens.GET("/:id", h.getGarden)
 	gardens.PATCH("/:id", h.updateGarden)
 	gardens.DELETE("/:id", h.deleteGarden)
-	gardens.POST("/:id/copy", h.copyGarden) // duplicate a garden the actor owns
+	gardens.POST("/:id/copy", h.copyGarden)   // duplicate a garden the actor owns
 	gardens.GET("/:id/full", h.getGardenFull) // one-shot editor load
 	gardens.POST("/:id/objects", h.createObject)
 

internal/api/gardens.go

@@ -2,6 +2,7 @@ package api
 
 import (
 	"errors"
+	"io"
 	"net/http"
 
 	"github.com/gin-gonic/gin"
@@ -115,13 +116,13 @@ func (h *handlers) copyGarden(c *gin.Context) {
 		return
 	}
 	// The body is optional — POST with no body means "copy under the default
-	// name" — so only decode when one was actually sent.
+	// name". An absent body surfaces as io.EOF from the decoder, which is the
+	// reliable signal: Content-Length is -1 (not 0) for a chunked request, so
+	// testing the length would misread an empty chunked body as malformed.
 	var req gardenCopyRequest
-	if c.Request.ContentLength != 0 {
-		if err := c.ShouldBindJSON(&req); err != nil {
-			writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "name must be a string")
-			return
-		}
+	if err := c.ShouldBindJSON(&req); err != nil && !errors.Is(err, io.EOF) {
+		writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "name must be a string")
+		return
 	}
 	g, err := h.svc.CopyGarden(c.Request.Context(), mustActor(c).ID, id, req.Name)
 	if err != nil {

internal/api/gardens_test.go

@@ -2,8 +2,11 @@ package api
 
 import (
 	"encoding/json"
+	"io"
 	"net/http"
+	"net/http/httptest"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/gin-gonic/gin"
@@ -247,3 +250,32 @@ func TestCopyGardenEndpoint(t *testing.T) {
 		t.Errorf("bad body status = %d, want 400", w.Code)
 	}
 }
+
+// A body of unknown length (chunked transfer, Content-Length -1) that turns out
+// to be empty must take the default-name path, not fail as malformed.
+func TestCopyGardenEmptyChunkedBody(t *testing.T) {
+	r := authEngine(t, localCfg())
+	cookie := registerAndCookie(t, r, "[email protected]")
+
+	w := doJSON(t, r, http.MethodPost, "/api/v1/gardens", map[string]any{"name": "Home"}, cookie)
+	id := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
+
+	// A plain io.Reader (not a *strings.Reader/*bytes.Buffer) leaves
+	// ContentLength at -1, which is what a chunked request looks like server-side.
+	req := httptest.NewRequest(http.MethodPost,
+		"/api/v1/gardens/"+strconv.FormatInt(id, 10)+"/copy", io.NopCloser(strings.NewReader("")))
+	req.Header.Set("Content-Type", "application/json")
+	req.AddCookie(cookie)
+	if req.ContentLength != -1 {
+		t.Fatalf("test setup: ContentLength = %d, want -1 (unknown)", req.ContentLength)
+	}
+	rec := httptest.NewRecorder()
+	r.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusCreated {
+		t.Fatalf("status = %d, want 201; body %s", rec.Code, rec.Body.String())
+	}
+	if got := decodeMap(t, rec.Body.Bytes())["name"]; got != "Home (copy)" {
+		t.Errorf("name = %v, want the derived 'Home (copy)'", got)
+	}
+}

internal/service/gardens_test.go

@@ -345,9 +345,8 @@ func TestCopyGardenSkipsRemovedPlantings(t *testing.T) {
 	bed := seedBed(t, s, owner, src.ID)
 	plant := seedOwnPlant(t, s, owner, 20)
 
-	kept, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{PlantID: plant.ID, RadiusCM: 10})
-	if err != nil {
-		t.Fatalf("CreatePlanting(kept): %v", err)
+	if _, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{PlantID: plant.ID, RadiusCM: 10}); err != nil {
+		t.Fatalf("CreatePlanting(first): %v", err)
 	}
 	if _, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{PlantID: plant.ID, XCM: 50, RadiusCM: 10}); err != nil {
 		t.Fatalf("CreatePlanting(cleared): %v", err)
@@ -361,7 +360,6 @@ func TestCopyGardenSkipsRemovedPlantings(t *testing.T) {
 	if err != nil {
 		t.Fatalf("CreatePlanting(replanted): %v", err)
 	}
-	_ = kept
 
 	dup, err := s.CopyGarden(ctx, owner, src.ID, "")
 	if err != nil {

internal/store/gardens.go

@@ -213,16 +213,7 @@ func (d *DB) CopyGarden(ctx context.Context, srcID, ownerID int64, name string)
 	// re-parented onto the right new object.
 	objectIDs := make(map[int64]int64, len(srcObjects))
 	for _, o := range srcObjects {
-		copied, err := scanObject(tx.QueryRowContext(ctx,
-			`INSERT INTO garden_objects
-			   (garden_id, kind, name, shape, points, x_cm, y_cm, width_cm, height_cm,
-			    rotation_deg, z_index, plantable, color, props, grid_size_cm, snap_to_grid, notes)
-			 VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
-			 RETURNING `+objectColumns,
-			created.ID, o.Kind, o.Name, o.Shape, o.Points, o.XCM, o.YCM, o.WidthCM, o.HeightCM,
-			o.RotationDeg, o.ZIndex, boolToInt(o.Plantable), o.Color, o.Props,
-			o.GridSizeCM, boolToInt(o.SnapToGrid), o.Notes,
-		))
+		copied, err := scanObject(tx.QueryRowContext(ctx, objectInsert, objectInsertArgs(created.ID, &o)...))
 		if err != nil {
 			return nil, fmt.Errorf("store: copy object: %w", err)
 		}
@@ -243,11 +234,7 @@ func (d *DB) CopyGarden(ctx context.Context, srcID, ownerID int64, name string)
 			// Unreachable: every active planting joins an object we just copied.
 			return nil, fmt.Errorf("store: copy planting %d: source object %d not copied", p.ID, p.ObjectID)
 		}
-		if _, err := tx.ExecContext(ctx,
-			`INSERT INTO plantings (object_id, plant_id, x_cm, y_cm, radius_cm, count, label, planted_at)
-			 VALUES (?, ?, ?, ?, ?, ?, ?, ?)`,
-			objectID, p.PlantID, p.XCM, p.YCM, p.RadiusCM, p.Count, p.Label, p.PlantedAt,
-		); err != nil {
+		if _, err := scanPlanting(tx.QueryRowContext(ctx, plantingInsert, plantingInsertArgs(objectID, &p)...)); err != nil {
 			return nil, fmt.Errorf("store: copy planting: %w", err)
 		}
 	}

internal/store/objects.go

@@ -31,19 +31,30 @@ func scanObject(s scanner) (*domain.GardenObject, error) {
 	return &o, nil
 }
 
+// objectInsert inserts one garden_objects row and returns it. It is shared by
+// CreateObject and CopyGarden's in-transaction copy, with objectInsertArgs
+// supplying its parameters, so a column added to the table can't be wired into
+// one path and silently dropped from the other.
+const objectInsert = `INSERT INTO garden_objects
+	   (garden_id, kind, name, shape, points, x_cm, y_cm, width_cm, height_cm,
+	    rotation_deg, z_index, plantable, color, props, grid_size_cm, snap_to_grid, notes)
+	 VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+	 RETURNING ` + objectColumns
+
+// objectInsertArgs builds objectInsert's parameters, placing o in gardenID (which
+// is o's own garden on create, and the new garden when copying).
+func objectInsertArgs(gardenID int64, o *domain.GardenObject) []any {
+	return []any{
+		gardenID, o.Kind, o.Name, o.Shape, o.Points, o.XCM, o.YCM, o.WidthCM, o.HeightCM,
+		o.RotationDeg, o.ZIndex, boolToInt(o.Plantable), o.Color, o.Props,
+		o.GridSizeCM, boolToInt(o.SnapToGrid), o.Notes,
+	}
+}
+
 // CreateObject inserts a garden object (fields already validated by the service)
 // and returns the stored row.
 func (d *DB) CreateObject(ctx context.Context, o *domain.GardenObject) (*domain.GardenObject, error) {
-	created, err := scanObject(d.sql.QueryRowContext(ctx,
-		`INSERT INTO garden_objects
-		   (garden_id, kind, name, shape, points, x_cm, y_cm, width_cm, height_cm,
-		    rotation_deg, z_index, plantable, color, props, grid_size_cm, snap_to_grid, notes)
-		 VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
-		 RETURNING `+objectColumns,
-		o.GardenID, o.Kind, o.Name, o.Shape, o.Points, o.XCM, o.YCM, o.WidthCM, o.HeightCM,
-		o.RotationDeg, o.ZIndex, boolToInt(o.Plantable), o.Color, o.Props,
-		o.GridSizeCM, boolToInt(o.SnapToGrid), o.Notes,
-	))
+	created, err := scanObject(d.sql.QueryRowContext(ctx, objectInsert, objectInsertArgs(o.GardenID, o)...))
 	if err != nil {
 		return nil, fmt.Errorf("store: insert object: %w", err)
 	}

internal/store/plantings.go

@@ -104,16 +104,25 @@ func (d *DB) GetPlanting(ctx context.Context, id int64) (*domain.Planting, error
 	return p, nil
 }
 
+// plantingInsert inserts one plantings row and returns it. Shared by
+// CreatePlanting, the CreatePlantings batch and CopyGarden's in-transaction copy
+// — with plantingInsertArgs supplying its parameters — so all three stay in step
+// with the table. removed_at is never set here: a new plop is active, and "clear
+// bed" sets removed_at later via UpdatePlanting.
+const plantingInsert = `INSERT INTO plantings (object_id, plant_id, x_cm, y_cm, radius_cm, count, label, planted_at)
+	 VALUES (?, ?, ?, ?, ?, ?, ?, ?)
+	 RETURNING ` + plantingColumns
+
+// plantingInsertArgs builds plantingInsert's parameters, parenting p to objectID
+// (p's own object normally, and the copied object when copying a garden).
+func plantingInsertArgs(objectID int64, p *domain.Planting) []any {
+	return []any{objectID, p.PlantID, p.XCM, p.YCM, p.RadiusCM, p.Count, p.Label, p.PlantedAt}
+}
+
 // CreatePlanting inserts a plop (fields already validated by the service) and
-// returns the stored row. removed_at is always NULL on create — a new plop is
-// active; "clear bed" sets removed_at later via UpdatePlanting.
+// returns the stored row.
 func (d *DB) CreatePlanting(ctx context.Context, p *domain.Planting) (*domain.Planting, error) {
-	created, err := scanPlanting(d.sql.QueryRowContext(ctx,
-		`INSERT INTO plantings (object_id, plant_id, x_cm, y_cm, radius_cm, count, label, planted_at)
-		 VALUES (?, ?, ?, ?, ?, ?, ?, ?)
-		 RETURNING `+plantingColumns,
-		p.ObjectID, p.PlantID, p.XCM, p.YCM, p.RadiusCM, p.Count, p.Label, p.PlantedAt,
-	))
+	created, err := scanPlanting(d.sql.QueryRowContext(ctx, plantingInsert, plantingInsertArgs(p.ObjectID, p)...))
 	if err != nil {
 		return nil, fmt.Errorf("store: insert planting: %w", err)
 	}
@@ -132,13 +141,9 @@ func (d *DB) CreatePlantings(ctx context.Context, plantings []*domain.Planting)
 	}
 	defer tx.Rollback() //nolint:errcheck // no-op after a successful commit
 
-	const stmt = `INSERT INTO plantings (object_id, plant_id, x_cm, y_cm, radius_cm, count, label, planted_at)
-		 VALUES (?, ?, ?, ?, ?, ?, ?, ?)
-		 RETURNING ` + plantingColumns
 	out := make([]domain.Planting, 0, len(plantings))
 	for _, p := range plantings {
-		created, err := scanPlanting(tx.QueryRowContext(ctx, stmt,
-			p.ObjectID, p.PlantID, p.XCM, p.YCM, p.RadiusCM, p.Count, p.Label, p.PlantedAt))
+		created, err := scanPlanting(tx.QueryRowContext(ctx, plantingInsert, plantingInsertArgs(p.ObjectID, p)...))
 		if err != nil {
 			return nil, fmt.Errorf("store: insert planting (batch): %w", err)
 		}

internal/store/sqlite.go

@@ -5,6 +5,7 @@
 package store
 
 import (
+	"context"
 	"database/sql"
 	"fmt"
 	"net/url"
@@ -99,6 +100,13 @@ func pragmaName(p string) string {
 	return strings.ToLower(strings.TrimSpace(p))
 }
 
+// queryer is the read surface shared by *sql.DB and *sql.Tx, so a list helper can
+// serve both a standalone read and one inside a transaction. (Its single-row
+// counterpart is `scanner`, in users.go.)
+type queryer interface {
+	QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error)
+}
+
 // qualifyColumns prefixes each comma-separated column in cols with "alias." so a
 // shared column list can be used in a JOIN — e.g. qualifyColumns("pl", "id, x")
 // → "pl.id, pl.x". Whitespace/newlines in the list are trimmed.

internal/store/users.go

@@ -19,12 +19,6 @@ type scanner interface {
 	Scan(dest ...any) error
 }
 
-// queryer is the read surface shared by *sql.DB and *sql.Tx, so a list helper can
-// serve both a standalone read and one inside a transaction.
-type queryer interface {
-	QueryContext(ctx context.Context, query string, args ...any) (*sql.Rows, error)
-}
-
 // scanUser reads one users row. is_admin is stored as INTEGER 0/1 (the driver
 // returns it as int64, which does not convert straight to bool), so it is read
 // into an int64 and mapped.

web/src/components/gardens/CopyGardenModal.tsx

@@ -6,7 +6,7 @@ import { Modal } from '@/components/ui/Modal'
 import { TextField } from '@/components/ui/TextField'
 import { toast } from '@/components/ui/toast'
 import { errorMessage } from '@/lib/api'
-import { useCopyGarden, type Garden } from '@/lib/gardens'
+import { defaultCopyName, useCopyGarden, type Garden } from '@/lib/gardens'
 
 /**
  * Duplicate a garden under a new name. The name is prefilled with the server's
@@ -17,7 +17,7 @@ import { useCopyGarden, type Garden } from '@/lib/gardens'
 export function CopyGardenModal({ garden, onClose }: { garden: Garden; onClose: () => void }) {
   const copy = useCopyGarden()
   const navigate = useNavigate()
-  const [name, setName] = useState(`${garden.name} (copy)`)
+  const [name, setName] = useState(() => defaultCopyName(garden.name))
   const [error, setError] = useState<string | null>(null)
 
   async function onSubmit(e: React.FormEvent) {

web/src/lib/gardens.test.ts

@@ -0,0 +1,31 @@
+import { describe, expect, it } from 'vitest'
+import { defaultCopyName } from './gardens'
+
+// The server caps a garden name at 200 BYTES and derives a copy's name the same
+// way (copyName in internal/service/gardens.go). These cases pin the mirror: an
+// over-long prefill would come back as a 400 rather than a copy.
+describe('defaultCopyName', () => {
+  it('appends the suffix to a short name', () => {
+    expect(defaultCopyName('Home')).toBe('Home (copy)')
+  })
+
+  it('keeps a derived name within the 200-byte cap', () => {
+    const name = 'a'.repeat(200)
+    const derived = defaultCopyName(name)
+    expect(new TextEncoder().encode(derived).length).toBeLessThanOrEqual(200)
+    expect(derived.endsWith(' (copy)')).toBe(true)
+  })
+
+  it('truncates multi-byte names on a code-point boundary', () => {
+    const name = 'é'.repeat(100) // 200 bytes, exactly at the cap
+    const derived = defaultCopyName(name)
+    expect(new TextEncoder().encode(derived).length).toBeLessThanOrEqual(200)
+    // No replacement character: nothing was cut mid-code-point.
+    expect(derived).not.toContain('�')
+  })
+
+  it('does not leave a dangling space before the suffix', () => {
+    const name = 'x'.repeat(192) + ' y' // truncation lands on the space
+    expect(defaultCopyName(name)).toBe('x'.repeat(192) + ' (copy)')
+  })
+})

web/src/lib/gardens.ts

@@ -86,6 +86,36 @@ export function useUpdateGarden() {
   })
 }
 
+// Mirrors the server's garden-name cap (maxGardenNameLen in
+// internal/service/gardens.go). It is a BYTE cap, not a character count.
+const MAX_GARDEN_NAME_BYTES = 200
+const COPY_SUFFIX = ' (copy)'
+
+/** Trim s to at most maxBytes of UTF-8, never splitting a code point. */
+function truncateUtf8(s: string, maxBytes: number): string {
+  const enc = new TextEncoder()
+  if (enc.encode(s).length <= maxBytes) return s
+  let out = ''
+  let used = 0
+  for (const ch of s) {
+    const size = enc.encode(ch).length
+    if (used + size > maxBytes) break
+    out += ch
+    used += size
+  }
+  return out
+}
+
+/**
+ * The name a copy gets by default. Mirrors the server's copyName (same file as
+ * the cap above) so the prefilled value is one the server will accept — without
+ * the byte-capped truncation, copying a garden whose name is already at the cap
+ * would prefill an over-long name and fail with a 400.
+ */
+export function defaultCopyName(name: string): string {
+  return truncateUtf8(name, MAX_GARDEN_NAME_BYTES - COPY_SUFFIX.length).trim() + COPY_SUFFIX
+}
+
 /**
  * Duplicate a garden the actor owns, including its objects and current plantings.
  * An omitted/blank name lets the server derive "<source> (copy)". The copy does