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
359 lines
13 KiB
Go
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)
|
|
}
|
|
}
|