Files
pansy/internal/agent/runtime_test.go
T
steveandClaude Opus 4.8 7c1faa1515
Build image / build-and-push (push) Successful in 6s
Gadfly review (reusable) / review (pull_request) Successful in 12m51s
Adversarial Review (Gadfly) / review (pull_request) Successful in 12m51s
Fill mode: a "grid" layout that plants individual plants in rows (#77)
Steve chose option 3: keep clumps as the default primitive, add a grid/rows
fill mode, so sketching and planning are different operations with different
outputs rather than one model forced to be both.

A plop is a CLUMP, not a plant — great for "a few plops of garlic in a corner",
useless for drawing a plantable 8-rows-of-garlic bed (that came out as ~15
blobs, #77). FillLayout selects what a fill packs:
- clump (default, unchanged): radius 1.5×spacing, ~7 plants per plop.
- grid: radius spacing/2, pitch = spacing, ONE plant per plop — rows you could
  actually plant from.

The geometry is the SAME hexCenters lattice and the SAME #75 half-spacing edge
rule; only the radius→spacing relationship differs (plopRadiusFor). Grid keeps
no 15cm floor — its whole point is true spacing — while clump keeps it so a
tiny-spacing plant doesn't make invisible clumps.

Threaded through FillRegion/FillNamedRegion (empty layout = clump, so existing
callers are unchanged; unknown layout = ErrInvalidInput), the REST /fill
endpoint (`layout`), and the agent's fill_region tool (`mode`, enum clump|grid),
so "plant the bed in rows" works.

Tests: grid produces many more, single-plant plops than clump on the same bed
(radius spacing/2, derived count 1); unknown layout is refused at both the
service and the API. maxFillPlops still caps a grid fill of a huge bed.

No frontend fill affordance exists yet (fill is agent-only in the UI; the fill
UI was deferred in #82), so the mode toggle rides along when that's built —
noted. Docs: DESIGN placement-model decision.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
2026-07-22 00:01:27 -04:00

359 lines
13 KiB
Go

package agent
import (
"context"
"encoding/json"
"errors"
"fmt"
"strings"
"testing"
"time"
"unicode/utf8"
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
"gitea.stevedudenhoeffer.com/steve/majordomo/provider/fake"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/service"
)
// scriptedRunner wires a Runner to a fake provider so the whole loop — tools,
// change-set scoping, loop guards — is exercised with no live model.
func scriptedRunner(t *testing.T, svc *service.Service, steps ...fake.Step) *Runner {
t.Helper()
p := fake.New("fake")
for _, s := range steps {
p.Enqueue("m", s)
}
model, err := p.Model("m")
if err != nil {
t.Fatalf("fake model: %v", err)
}
return &Runner{svc: svc, model: model}
}
// toolCall scripts one model turn that calls a tool.
func toolCall(name string, args any) fake.Step {
raw, _ := json.Marshal(args)
return fake.ReplyWith(llm.Response{
FinishReason: llm.FinishToolCalls,
ToolCalls: []llm.ToolCall{{ID: "c1", Name: name, Arguments: raw}},
})
}
// TestTurnIsOneChangeSet is the acceptance criterion the whole "act freely"
// posture rests on: a turn that clears a bed and replants it — one object edit
// and many planting inserts — has to undo as ONE action, not thirteen.
func TestTurnIsOneChangeSet(t *testing.T) {
ctx := context.Background()
svc, owner := newAgentTestService(t)
g, err := svc.CreateGarden(ctx, owner, service.GardenInput{Name: "Plot", WidthCM: 2000, HeightCM: 2000})
if err != nil {
t.Fatalf("garden: %v", err)
}
garlic := mustPlant(t, svc, owner, "Garlic", 15, "🧄")
cucumber := mustPlant(t, svc, owner, "Cucumber", 45, "🥒")
bed, err := svc.CreateObject(ctx, owner, g.ID, service.ObjectInput{
Kind: domain.KindBed, Name: "Garlic bed", XCM: 1000, YCM: 1000, WidthCM: 400, HeightCM: 400,
})
if err != nil {
t.Fatalf("bed: %v", err)
}
if _, err := svc.FillNamedRegion(ctx, owner, bed.ID, "all", garlic.ID, nil, service.FillClump); err != nil {
t.Fatalf("seed garlic: %v", err)
}
before, _, err := svc.GardenHistory(ctx, owner, g.ID, 0, 0)
if err != nil {
t.Fatalf("history: %v", err)
}
r := scriptedRunner(t, svc,
toolCall("clear_object", map[string]any{"objectId": bed.ID}),
toolCall("fill_region", map[string]any{"objectId": bed.ID, "region": "all", "plantId": cucumber.ID}),
fake.Reply("Cleared the garlic and replanted the bed with cucumbers."),
)
turn, err := r.Run(ctx, owner, g.ID, "change the garlic bed to cucumbers this year", nil, nil)
if err != nil {
t.Fatalf("Run: %v", err)
}
if turn.ChangeSetID == nil {
t.Fatal("a turn that changed things produced no change set")
}
if !strings.Contains(turn.Reply, "cucumbers") {
t.Errorf("reply = %q", turn.Reply)
}
// Exactly ONE new change set, whatever the model did inside the turn.
after, _, err := svc.GardenHistory(ctx, owner, g.ID, 0, 0)
if err != nil {
t.Fatalf("history: %v", err)
}
if len(after)-len(before) != 1 {
t.Fatalf("turn produced %d change sets, want exactly 1", len(after)-len(before))
}
cs := after[0]
if cs.Source != domain.SourceAgent {
t.Errorf("source = %q, want agent", cs.Source)
}
if cs.Summary != "change the garlic bed to cucumbers this year" {
t.Errorf("summary = %q, want the user's own words", cs.Summary)
}
// The bed really is cucumbers now.
full, _ := svc.GardenFull(ctx, owner, g.ID, nil)
if len(full.Plantings) == 0 {
t.Fatal("bed ended up empty")
}
for _, p := range full.Plantings {
if p.PlantID != cucumber.ID {
t.Errorf("unexpected plant %d still in the bed", p.PlantID)
}
}
// And one undo puts it back.
if _, conflicts, err := svc.RevertChangeSet(ctx, owner, *turn.ChangeSetID, domain.SourceUI); err != nil || len(conflicts) != 0 {
t.Fatalf("undo: err=%v conflicts=%+v", err, conflicts)
}
full, _ = svc.GardenFull(ctx, owner, g.ID, nil)
for _, p := range full.Plantings {
if p.PlantID != garlic.ID {
t.Errorf("after undo the bed holds plant %d, want the garlic back", p.PlantID)
}
}
}
// TestViewerGetsAnExplainableRefusal — the ACL story only works if the model can
// narrate the refusal, so a tool denial has to reach it as a tool RESULT it can
// read, not as a 500 that ends the run.
func TestViewerGetsAnExplainableRefusal(t *testing.T) {
ctx := context.Background()
svc, owner := newAgentTestService(t)
viewer, err := svc.Register(ctx, service.RegisterInput{Email: "[email protected]", DisplayName: "V", Password: "password123"})
if err != nil {
t.Fatalf("register: %v", err)
}
g, err := svc.CreateGarden(ctx, owner, service.GardenInput{Name: "Plot", WidthCM: 2000, HeightCM: 2000})
if err != nil {
t.Fatalf("garden: %v", err)
}
bed, err := svc.CreateObject(ctx, owner, g.ID, service.ObjectInput{
Kind: domain.KindBed, XCM: 1000, YCM: 1000, WidthCM: 400, HeightCM: 400,
})
if err != nil {
t.Fatalf("bed: %v", err)
}
if _, err := svc.AddShare(ctx, owner, g.ID, "[email protected]", domain.RoleViewer); err != nil {
t.Fatalf("share: %v", err)
}
// A viewer can't open a change set at all, so the turn is refused up front —
// before any model call — and the API turns that into a plain explanation.
r := scriptedRunner(t, svc, fake.Reply("unused"))
_, err = r.Run(ctx, viewer.ID, g.ID, "plant garlic in that bed", nil, nil)
if !errors.Is(err, domain.ErrForbidden) {
t.Fatalf("viewer turn err = %v, want ErrForbidden", err)
}
// And at the tool layer, a refusal comes back as a readable tool result
// rather than killing the run.
box := NewToolbox(svc, viewer.ID)
raw, _ := json.Marshal(map[string]any{"objectId": bed.ID})
res := box.Execute(ctx, llm.ToolCall{ID: "1", Name: "clear_object", Arguments: raw})
if !res.IsError {
t.Fatal("a viewer's clear_object succeeded")
}
if res.Content == "" {
t.Error("the refusal carried no text for the model to explain")
}
}
// TestRunStopsAtTheStepCap — a model that gets wedged should stop on its own,
// and what it managed to do must still be recorded and undoable.
func TestRunStopsAtTheStepCap(t *testing.T) {
ctx := context.Background()
svc, owner := newAgentTestService(t)
g, err := svc.CreateGarden(ctx, owner, service.GardenInput{Name: "Plot", WidthCM: 2000, HeightCM: 2000})
if err != nil {
t.Fatalf("garden: %v", err)
}
// More describe_garden calls than the cap allows, forever.
steps := make([]fake.Step, 0, maxSteps+4)
for i := 0; i < maxSteps+4; i++ {
steps = append(steps, toolCall("describe_garden", map[string]any{"gardenId": g.ID}))
}
r := scriptedRunner(t, svc, steps...)
turn, err := r.Run(ctx, owner, g.ID, "look at the garden", nil, nil)
if err != nil {
t.Fatalf("a capped run should end cleanly, got %v", err)
}
if !turn.Truncated {
t.Error("turn.Truncated = false; the run hit the cap and should say so")
}
if turn.Reply == "" {
t.Error("a capped run said nothing; silence reads as a hang")
}
// It only read, so there is nothing to undo — and no empty change set either.
if turn.ChangeSetID != nil {
t.Errorf("a read-only turn produced change set %d", *turn.ChangeSetID)
}
}
// TestReadOnlyTurnWritesNoChangeSet — asking a question must not litter the
// history with empty entries.
func TestReadOnlyTurnWritesNoChangeSet(t *testing.T) {
ctx := context.Background()
svc, owner := newAgentTestService(t)
g, err := svc.CreateGarden(ctx, owner, service.GardenInput{Name: "Plot", WidthCM: 2000, HeightCM: 2000})
if err != nil {
t.Fatalf("garden: %v", err)
}
before, _, _ := svc.GardenHistory(ctx, owner, g.ID, 0, 0)
r := scriptedRunner(t, svc,
toolCall("describe_garden", map[string]any{"gardenId": g.ID}),
fake.Reply("It's empty — nothing planted yet."),
)
turn, err := r.Run(ctx, owner, g.ID, "what's in the garden?", nil, nil)
if err != nil {
t.Fatalf("Run: %v", err)
}
if turn.ChangeSetID != nil {
t.Errorf("a question produced change set %d", *turn.ChangeSetID)
}
after, _, _ := svc.GardenHistory(ctx, owner, g.ID, 0, 0)
if len(after) != len(before) {
t.Errorf("history grew by %d for a read-only turn", len(after)-len(before))
}
}
// TestNewRunnerNeedsConfiguration — an instance with no key or no model must not
// get a half-built runner; the caller treats the error as "no assistant" and
// carries on. (Whether the assistant is ENABLED is resolved before NewRunner is
// reached, so it isn't NewRunner's concern any more.)
func TestNewRunnerNeedsConfiguration(t *testing.T) {
svc, _ := newAgentTestService(t)
for _, tc := range []struct{ key, model string }{
{"", "ollama-cloud/x"},
{"k", ""},
{"k", " "},
} {
if _, err := NewRunner(svc, tc.key, tc.model); err == nil {
t.Errorf("NewRunner accepted key=%q model=%q", tc.key, tc.model)
}
}
// A model spec naming a provider that doesn't exist is a configuration
// error, not a panic at first use.
if _, err := NewRunner(svc, "k", "nonesuch/model"); err == nil {
t.Error("NewRunner accepted an unresolvable model spec")
}
}
// TestTurnSummaryFitsAHistoryRow — the user's own words are the most useful
// label for a change set, but a paragraph would wreck the list.
func TestTurnSummaryFitsAHistoryRow(t *testing.T) {
if got := turnSummary(" change the garlic bed\n to cucumbers "); got != "change the garlic bed to cucumbers" {
t.Errorf("turnSummary = %q", got)
}
long := turnSummary(strings.Repeat("plant garlic ", 40))
if len(long) > 130 || !strings.HasSuffix(long, "…") {
t.Errorf("long summary = %q (%d chars)", long, len(long))
}
}
// TestSystemPromptStatesTheCompassConvention — -y being north is not guessable,
// and a model that assumes otherwise plants the wrong end of the bed.
func TestSystemPromptStatesTheCompassConvention(t *testing.T) {
p := systemPrompt(&domain.Garden{ID: 1, Name: "Plot", WidthCM: 500, HeightCM: 400, UnitPref: domain.UnitImperial})
for _, want := range []string{"NORTH", "-y", "centimeters", "Plot", "version"} {
if !strings.Contains(p, want) {
t.Errorf("system prompt is missing %q:\n%s", want, p)
}
}
if !strings.Contains(p, fmt.Sprintf("%.0f", 500.0)) {
t.Error("system prompt doesn't state the garden's size")
}
}
// TestPartialWorkSurvivesATimeout is the finding that mattered most on this PR.
//
// The run context carries a timeout. When it fires, WithChangeSet's recovery
// path has to record what already committed — and doing that with the SAME
// dead context would fail, losing the history for changes that really happened.
// The user-facing message says "anything I'd already changed is in History", so
// this isn't just a gap, it's a promise the code has to keep.
func TestPartialWorkSurvivesATimeout(t *testing.T) {
ctx := context.Background()
svc, owner := newAgentTestService(t)
g, err := svc.CreateGarden(ctx, owner, service.GardenInput{Name: "Plot", WidthCM: 2000, HeightCM: 2000})
if err != nil {
t.Fatalf("garden: %v", err)
}
bed, err := svc.CreateObject(ctx, owner, g.ID, service.ObjectInput{
Kind: domain.KindBed, Name: "Bed", XCM: 1000, YCM: 1000, WidthCM: 400, HeightCM: 400,
})
if err != nil {
t.Fatalf("bed: %v", err)
}
before, _, _ := svc.GardenHistory(ctx, owner, g.ID, 0, 0)
// A turn that renames the bed, then dies with the context already cancelled.
cancelled, cancel := context.WithCancel(ctx)
r := scriptedRunner(t, svc,
toolCall("move_object", map[string]any{
"objectId": bed.ID, "xCm": 600.0, "yCm": 600.0, "version": bed.Version,
}),
fake.Step{Err: context.DeadlineExceeded},
)
// Cancel once the first tool call has landed, so the failure path runs with a
// dead context — exactly the timeout case.
go func() {
time.Sleep(50 * time.Millisecond)
cancel()
}()
_, err = r.Run(cancelled, owner, g.ID, "move the bed", nil, nil)
if err == nil {
t.Fatal("expected the turn to fail")
}
// The move committed, so it must be in history and undoable.
after, _, herr := svc.GardenHistory(ctx, owner, g.ID, 0, 0)
if herr != nil {
t.Fatalf("history: %v", herr)
}
if len(after) != len(before)+1 {
t.Fatalf("the failed turn recorded %d change sets, want 1 — its work is otherwise un-undoable",
len(after)-len(before))
}
if !strings.Contains(after[0].Summary, "failed partway") {
t.Errorf("summary = %q, want it marked as partial", after[0].Summary)
}
if _, conflicts, rerr := svc.RevertChangeSet(ctx, owner, after[0].ID, domain.SourceUI); rerr != nil || len(conflicts) != 0 {
t.Fatalf("the partial turn should be undoable: err=%v conflicts=%+v", rerr, conflicts)
}
o, _ := svc.DescribeGarden(ctx, owner, g.ID)
if len(o.Objects) > 0 && o.Objects[0].XCM != bed.XCM {
t.Errorf("undo left the bed at %v, want %v", o.Objects[0].XCM, bed.XCM)
}
}
// TestTurnSummaryTrimsByRunes — slicing a byte offset would cut a multibyte
// character in half and store invalid UTF-8 in the history summary.
func TestTurnSummaryTrimsByRunes(t *testing.T) {
// 200 multibyte runes: a byte slice at 120 would land mid-character.
got := turnSummary(strings.Repeat("🌱", 200))
if !utf8.ValidString(got) {
t.Errorf("turnSummary produced invalid UTF-8: %q", got)
}
if !strings.HasSuffix(got, "…") {
t.Errorf("long summary should be elided, got %q", got)
}
if n := utf8.RuneCountInString(got); n > 121 {
t.Errorf("summary is %d runes, want it trimmed", n)
}
}