diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c20d302765..119f258b11 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,12 +35,14 @@ on: branches: - dev - santos/dev2 + - workstreams/main # Also on the push, because a merge of two individually-green branches can # still be red, and the merge is exactly the moment nobody is watching. push: branches: - dev - santos/dev2 + - workstreams/main workflow_dispatch: permissions: @@ -323,7 +325,12 @@ jobs: # -p 2 permits concurrent compilation/testing in rest on the public # 4-vCPU / 16-GB runner, leaving headroom for the linker. tui3 and # session use four shards; codeaf runs its suite without sharding. - ./scripts/touched-verdict.sh run --base "$BASE" --shards 4 --report "$TMPDIR/$LEG.json" $PACKAGES + # CODEAF'S ONE TEST BINARY GETS 25 MINUTES, AND ONLY IT. Sharding cmd/codeaf + # was tried and its sharded runs failed (#1734), and its serial suite measured + # 1065 s (17.8 min) on this runner on 2026-11-03, over the Makefile's 15m, so + # every test after the cut went unchecked. The other legs pass no --timeout and + # keep the Makefile's 15m. Drop this once the suite is back under 15 minutes. + ./scripts/touched-verdict.sh run --base "$BASE" --shards 4 ${{ matrix.leg == 'codeaf' && '--timeout 25m' || '' }} --report "$TMPDIR/$LEG.json" $PACKAGES - uses: actions/upload-artifact@v4 if: always() with: diff --git a/cmd/codeaf/app.go b/cmd/codeaf/app.go index 482b654a2e..cfa473addf 100644 --- a/cmd/codeaf/app.go +++ b/cmd/codeaf/app.go @@ -202,6 +202,7 @@ func runApp(args []string) error { // ONE SUPERVISOR FOR EVERY CONVERSATION THE APP BUILDS (appservices.go). services := openAppServices() appServiceDoor.Store(services) + appClientToolsBackend.Store(backend) backend.attachServices(services) guard.Go("app/crews-start", func() { backend.crews() }) // THE APP IS AN OPEN WINDOW for as long as it runs, so the clock that runs @@ -552,6 +553,7 @@ func appConnOf(client *remote.Client, release func(), assembled bool) *appConn { // appBackend is the [appwire.Engine] over this process's conversations. type appBackend struct { + clientTools appClientTools pluginPauses map[string]appPluginPause pluginOllama appPluginOllama pluginAuth map[string]*appPluginAttempt @@ -746,6 +748,7 @@ type appBackend struct { collections *workspace.Store collectionsClosed bool collectionsPath string + collectionFactory collectionFactory // services is the app's one supervisor of kept-running processes // (appservices.go); nil is the capability absent. services *appServices @@ -1251,10 +1254,12 @@ func appRowOnDisk(id string) (session.SessionRow, bool) { // A folder minted and never spoken in is still a conversation the app // may have created; its meta names the workspace. meta, err := session.LoadMeta(dir) - if err != nil || strings.TrimSpace(meta.ID) == "" { + if err != nil || meta.ID != id { continue } - return session.SessionRow{ID: id, AgentDID: meta.AgentDID(), Dir: dir, Transcript: transcript, Title: meta.Title, Workspace: meta.Workspace, Created: meta.Created}, true + return session.SessionRow{ID: id, AgentDID: meta.AgentDID(), Dir: dir, Transcript: transcript, + Title: meta.Title, Workspace: meta.Workspace, Owned: meta.Owned, Model: meta.Model, + Archived: meta.Archived, Spend: meta.SpentUSD, At: meta.Created, Created: meta.Created}, true } return session.SessionRow{}, false } @@ -4494,6 +4499,8 @@ func (b *appBackend) Close() error { if b.leaving != nil { b.leaving() } + b.clientTools.disconnect() + appClientToolsBackend.CompareAndSwap(b, nil) already := false var crews *appCrewRun var agents []*appAgent diff --git a/cmd/codeaf/appblank.go b/cmd/codeaf/appblank.go index 7dd6e31fb9..90ab9cbff9 100644 --- a/cmd/codeaf/appblank.go +++ b/cmd/codeaf/appblank.go @@ -139,7 +139,7 @@ func (b *appBackend) reapBlank(a *appAgent) bool { } gone := false _ = teamstore.InspectConversation(b.crewProfile(), transcript, func(referenced bool) error { - if referenced || session.InUse(transcript) || !v3EmptyUnassignedSession(dir) { + if referenced || session.InUse(transcript) || !v3EmptyUnassignedSession(dir) || collectionKeepsConversation(dir) { return nil } gone = os.RemoveAll(dir) == nil diff --git a/cmd/codeaf/appclienttools.go b/cmd/codeaf/appclienttools.go new file mode 100644 index 0000000000..fe6b55a0bd --- /dev/null +++ b/cmd/codeaf/appclienttools.go @@ -0,0 +1,507 @@ +package main + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "sort" + "sync" + "sync/atomic" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/guard" + "github.com/Agent-Field/codeaf/internal/remote" + "github.com/Agent-Field/codeaf/internal/session" +) + +const clientToolSettledTTL = 10 * time.Minute +const clientToolSettledMax = 1024 + +var appClientToolsBackend atomic.Pointer[appBackend] +var clientToolFreshID atomic.Uint64 + +type clientToolAgentSets struct { + version uint64 + sets []session.ClientToolSet + builtins map[string]bool +} + +type clientToolPending struct { + ctx context.Context + done chan struct{} + call session.ClientToolCall + deadline time.Time + answer session.ClientToolAnswer + event *appwire.AgentsToolCall +} + +type clientToolSettled struct { + answer session.ClientToolAnswer + at time.Time +} + +// The registry belongs to one stdio connection, including its replay memory. +type appClientTools struct { + mu sync.Mutex + agents map[string]clientToolAgentSets + pending map[string]*clientToolPending + settled map[string]clientToolSettled + order []string + closed bool + senderWake chan struct{} + senderDone chan struct{} +} + +type appClientToolDoor struct { + backend *appBackend + agent string +} + +var _ appwire.ClientTooler = (*appBackend)(nil) +var _ session.ClientToolDoor = (*appClientToolDoor)(nil) + +func (b *appBackend) clientToolEventCategory(a *appAgent, event session.Event) session.ActionCategory { + if event.Category != "" { + return event.Category + } + var id string + var conv appConversation + b.locked(func() { + id = a.view.ID + if a.live != nil { + conv = a.live.conn.Conv + } + }) + if local, ok := conv.(interface { + ActionCategoryForTool(string) session.ActionCategory + }); ok { + return local.ActionCategoryForTool(event.Tool) + } + r := &b.clientTools + r.mu.Lock() + defer r.mu.Unlock() + if r.agents[id].builtins[event.Tool] { + return session.ActionCategoryForTool(event.Tool) + } + for _, set := range r.agents[id].sets { + for _, tool := range set.Tools { + if tool.Name == event.Tool { + return tool.Category + } + } + } + return session.ActionCategoryForTool(event.Tool) +} + +func appClientToolsDoor(sessionFile string) session.ClientToolDoor { + b := appClientToolsBackend.Load() + if b == nil { + return nil + } + return &appClientToolDoor{backend: b, agent: appAgentID(sessionFile)} +} + +// Root builders share launch settings, but each opening needs its own conversation door. +func appClientToolsOpener(open func(session.Config) (*session.Agent, error)) func(session.Config) (*session.Agent, error) { + return func(cfg session.Config) (*session.Agent, error) { + cfg.ClientTools = appClientToolsDoor(cfg.SessionFile) + return open(cfg) + } +} + +func (b *appBackend) SetClientTools(p appwire.ClientToolsSetParams) (appwire.ClientToolsSetResult, error) { + result := appwire.ClientToolsSetResult{Agent: p.Agent, Set: p.Set, Tools: []string{}} + if err := p.Validate(); err != nil { + return result, err + } + a, err := b.find(p.Agent) + if err != nil { + return result, err + } + a.gate.Lock() + defer a.gate.Unlock() + if err := b.clientToolScope(a); err != nil { + return result, err + } + held := b.clientToolBuiltinNames(a) + r := &b.clientTools + r.mu.Lock() + defer r.mu.Unlock() + if r.closed { + return result, appwire.Errorf(appwire.CodeClosed, "the app connection is closed") + } + state := r.agents[p.Agent] + for name := range state.builtins { + held[name] = true + } + if err := clientToolReplacementValid(p, state.sets, held); err != nil { + return result, err + } + sets := make([]session.ClientToolSet, 0, len(state.sets)+1) + for _, set := range state.sets { + if set.ID != p.Set { + sets = append(sets, set) + } + } + if len(p.Tools) > 0 { + set := clientToolSnapshot(p) + sets = append(sets, set) + for _, tool := range set.Tools { + result.Tools = append(result.Tools, tool.Name) + } + } + sort.Slice(sets, func(i, j int) bool { return sets[i].ID < sets[j].ID }) + if r.agents == nil { + r.agents = make(map[string]clientToolAgentSets) + } + r.agents[p.Agent] = clientToolAgentSets{version: state.version + 1, sets: sets, builtins: state.builtins} + return result, nil +} + +func (b *appBackend) clientToolScope(a *appAgent) error { + remoteHeld := false + b.locked(func() { + if a.live != nil { + _, remoteHeld = a.live.conn.Conv.(*remote.Agent) + remoteHeld = remoteHeld && !a.live.conn.Assembled && !a.live.conn.Lent + } + remoteHeld = remoteHeld || a.view.Location != nil && !a.view.Location.Here + }) + if remoteHeld { + return appwire.Errorf(appwire.CodeBadScope, "client tools need a conversation this engine runs") + } + return nil +} + +// Local app conversations also use a transport proxy. Their session reports its +// built-in inventory here, so validation still sees the actual engine belt. +func (d *appClientToolDoor) ClientToolBuiltins(names []string) { + r := &d.backend.clientTools + r.mu.Lock() + defer r.mu.Unlock() + if r.closed { + return + } + state := r.agents[d.agent] + state.builtins = make(map[string]bool, len(names)) + for _, name := range names { + state.builtins[name] = true + } + if r.agents == nil { + r.agents = make(map[string]clientToolAgentSets) + } + r.agents[d.agent] = state +} + +func (b *appBackend) clientToolBuiltinNames(a *appAgent) map[string]bool { + var conv appConversation + b.locked(func() { + if a.live != nil { + conv = a.live.conn.Conv + } + }) + held := make(map[string]bool) + if live, ok := conv.(interface{ ClientToolBuiltinNames() []string }); ok { + for _, name := range live.ClientToolBuiltinNames() { + held[name] = true + } + } + return held +} + +func clientToolReplacementValid(p appwire.ClientToolsSetParams, sets []session.ClientToolSet, builtins map[string]bool) error { + names := make(map[string]bool) + count, setCount := len(p.Tools), 0 + for _, set := range sets { + if set.ID == p.Set { + continue + } + setCount++ + count += len(set.Tools) + for _, tool := range set.Tools { + names[tool.Name] = true + } + } + if len(p.Tools) > 0 { + setCount++ + } + if setCount > session.ClientToolMaxSets || count > session.ClientToolMaxPerAgent { + return appwire.Errorf(appwire.CodeBadParams, "an agent allows at most %d sets and %d client tools", session.ClientToolMaxSets, session.ClientToolMaxPerAgent) + } + for _, tool := range p.Tools { + if builtins[tool.Name] || session.ActionCategoryForTool(tool.Name) != session.ActionWork { + return appwire.Errorf(appwire.CodeBadParams, "%s is a built-in engine tool", tool.Name) + } + if names[tool.Name] { + return appwire.Errorf(appwire.CodeBadParams, "%s is already in another set", tool.Name) + } + } + return nil +} + +func clientToolSnapshot(p appwire.ClientToolsSetParams) session.ClientToolSet { + set := session.ClientToolSet{ID: p.Set, Instructions: p.Instructions, DeadlineMS: p.DeadlineMS, + Tools: append([]session.ClientTool(nil), p.Tools...)} + if set.DeadlineMS == 0 { + set.DeadlineMS = session.ClientToolDefaultDeadlineMS + } + for i := range set.Tools { + set.Tools[i].InputSchema = bytes.Clone(set.Tools[i].InputSchema) + } + return set +} + +func (d *appClientToolDoor) ClientToolSets() (uint64, []session.ClientToolSet) { + r := &d.backend.clientTools + r.mu.Lock() + defer r.mu.Unlock() + state := r.agents[d.agent] + return state.version, state.sets +} + +func clientToolCallID(agent, provider string) string { + if provider == "" { + return fmt.Sprintf("ct_fresh_%x", clientToolFreshID.Add(1)) + } + sum := sha256.Sum256([]byte(agent + "\x00" + provider)) + return "ct_" + hex.EncodeToString(sum[:]) +} + +func (d *appClientToolDoor) CallClientTool(ctx context.Context, call session.ClientToolCall) (session.ClientToolAnswer, error) { + if err := ctx.Err(); err != nil { + return clientToolTurnEnded(call.Name), nil + } + var object map[string]json.RawMessage + if json.Unmarshal(call.Args, &object) != nil || object == nil { + return session.ClientToolAnswer{Text: "Invalid arguments: expected a JSON object", IsError: true}, nil + } + id := clientToolCallID(d.agent, call.TurnCall) + r := &d.backend.clientTools + pending, answer, emit := r.begin(ctx, d.agent, id, call) + if pending == nil { + return answer, nil + } + if emit { + var turn uint64 + d.backend.locked(func() { + if a := d.backend.agents[d.agent]; a != nil && a.live != nil { + turn = a.live.turn + } + }) + r.queueEmission(d.backend.emit, pending, appwire.AgentsToolCall{Agent: d.agent, Turn: turn, + Call: id, TurnCall: call.TurnCall, SourceMessageID: call.SourceMessage, Set: call.Set, Name: call.Name, Args: bytes.Clone(call.Args), DeadlineMS: pending.call.DeadlineMS}) + } + return r.wait(ctx, id, pending), nil +} + +// One sender owns all writer waits. Queued events live only in pending calls, +// so calls that end behind a blocked writer leave no stale queue to grow. +func (r *appClientTools) queueEmission(emit appwire.Emitter, pending *clientToolPending, event appwire.AgentsToolCall) { + r.mu.Lock() + defer r.mu.Unlock() + if r.closed || r.pending[event.Call] != pending { + return + } + pending.event = &event + if r.senderWake == nil { + r.senderWake = make(chan struct{}, 1) + r.senderDone = make(chan struct{}) + guard.Go("app/client-tools", func() { r.sendEvents(emit) }) + } + select { + case r.senderWake <- struct{}{}: + default: + } +} + +func (r *appClientTools) sendEvents(emit appwire.Emitter) { + defer close(r.senderDone) + for range r.senderWake { + for { + event := r.nextEmission() + if event == nil { + break + } + emit.Emit(appwire.EventAgentsToolCall, *event) + } + } +} + +func (r *appClientTools) nextEmission() *appwire.AgentsToolCall { + r.mu.Lock() + defer r.mu.Unlock() + if r.closed { + return nil + } + for _, pending := range r.pending { + event := pending.event + pending.event = nil + if event != nil && pending.ctx.Err() == nil && time.Now().Before(pending.deadline) { + return event + } + } + return nil +} + +// begin takes the registry lock for [appClientTools.beginLocked] alone, so a +// panic inside it cannot leave the lock held. +func (r *appClientTools) begin(ctx context.Context, agent, id string, call session.ClientToolCall) (*clientToolPending, session.ClientToolAnswer, bool) { + r.mu.Lock() + defer r.mu.Unlock() + return r.beginLocked(ctx, agent, id, call) +} + +// Register before emitting, so even a synchronous client answer finds its waiter. +func (r *appClientTools) beginLocked(ctx context.Context, agent, id string, call session.ClientToolCall) (*clientToolPending, session.ClientToolAnswer, bool) { + r.pruneLocked(time.Now()) + if settled, ok := r.settled[id]; ok { + return nil, settled.answer, false + } + if pending := r.pending[id]; pending != nil { + return pending, session.ClientToolAnswer{}, false + } + deadline, attached := r.attachedLocked(agent, call.Set, call.Name) + if r.closed || !attached { + return nil, session.ClientToolUnavailable(call.Name), false + } + if call.DeadlineMS >= session.ClientToolMinDeadlineMS && call.DeadlineMS <= session.ClientToolMaxDeadlineMS { + deadline = call.DeadlineMS + } + call.DeadlineMS = deadline + pending := &clientToolPending{ctx: ctx, done: make(chan struct{}), call: call, deadline: time.Now().Add(time.Duration(deadline) * time.Millisecond)} + if r.pending == nil { + r.pending = make(map[string]*clientToolPending) + } + r.pending[id] = pending + return pending, session.ClientToolAnswer{}, true +} + +func (r *appClientTools) attachedLocked(agent, setID, name string) (int, bool) { + for _, set := range r.agents[agent].sets { + if set.ID != setID { + continue + } + for _, tool := range set.Tools { + if tool.Name == name { + return set.DeadlineMS, true + } + } + } + return 0, false +} + +func (r *appClientTools) wait(ctx context.Context, id string, pending *clientToolPending) session.ClientToolAnswer { + timer := time.NewTimer(time.Until(pending.deadline)) + defer timer.Stop() + var failed session.ClientToolAnswer + select { + case <-pending.done: + return pending.answer + case <-timer.C: + failed = clientToolTimeout(pending.call) + case <-ctx.Done(): + failed = clientToolTurnEnded(pending.call.Name) + } + r.mu.Lock() + defer r.mu.Unlock() + r.settleLocked(id, pending, failed) + return pending.answer +} + +func clientToolTimeout(call session.ClientToolCall) session.ClientToolAnswer { + return session.ClientToolAnswer{Text: fmt.Sprintf("client_timeout: the app that provides %s did not answer within %gs; the outcome is unknown, so check before trying again.", call.Name, float64(call.DeadlineMS)/1000), IsError: true} +} + +func clientToolTurnEnded(name string) session.ClientToolAnswer { + return session.ClientToolAnswer{Text: fmt.Sprintf("the turn ended before the app answered %s; the outcome is unknown, so check before trying again.", name), IsError: true} +} + +func (r *appClientTools) settleLocked(id string, pending *clientToolPending, answer session.ClientToolAnswer) bool { + if r.pending[id] != pending { + return false + } + delete(r.pending, id) + pending.answer = answer + if r.settled == nil { + r.settled = make(map[string]clientToolSettled) + } + r.pruneLocked(time.Now()) + if len(r.order) == clientToolSettledMax { + delete(r.settled, r.order[0]) + r.order = r.order[1:] + } + r.settled[id] = clientToolSettled{answer: answer, at: time.Now()} + r.order = append(r.order, id) + close(pending.done) + return true +} + +func (r *appClientTools) pruneLocked(now time.Time) { + for len(r.order) > 0 { + id := r.order[0] + if now.Sub(r.settled[id].at) < clientToolSettledTTL { + break + } + delete(r.settled, id) + r.order = r.order[1:] + } +} + +func (b *appBackend) ClientToolResult(p appwire.ClientToolResultParams) (appwire.ClientToolResultAck, error) { + if err := p.Validate(); err != nil { + return appwire.ClientToolResultAck{}, err + } + var result string + if p.Error == nil { + var compact bytes.Buffer + _ = json.Compact(&compact, p.Result) + result = session.BoundClientToolOutput(compact.String()) + } + r := &b.clientTools + r.mu.Lock() + defer r.mu.Unlock() + pending := r.pending[p.Call] + if pending == nil { + return appwire.ClientToolResultAck{}, nil + } + if pending.ctx.Err() != nil { + r.settleLocked(p.Call, pending, clientToolTurnEnded(pending.call.Name)) + return appwire.ClientToolResultAck{}, nil + } + if !time.Now().Before(pending.deadline) { + r.settleLocked(p.Call, pending, clientToolTimeout(pending.call)) + return appwire.ClientToolResultAck{}, nil + } + answer := session.ClientToolAnswer{} + if p.Error != nil { + answer.Text = fmt.Sprintf("%s could not do %s: %s", pending.call.Set, pending.call.Name, *p.Error) + answer.IsError = true + } else { + answer.Text = result + } + return appwire.ClientToolResultAck{Accepted: r.settleLocked(p.Call, pending, answer)}, nil +} + +// Disconnect precedes conversation shutdown, so pending calls receive the owner-loss answer. +func (r *appClientTools) disconnect() { + r.mu.Lock() + defer r.mu.Unlock() + if r.closed { + return + } + r.closed = true + if r.senderWake != nil { + close(r.senderWake) + } + for agent, state := range r.agents { + r.agents[agent] = clientToolAgentSets{version: state.version + 1} + } + for id, pending := range r.pending { + answer := session.ClientToolAnswer{Text: fmt.Sprintf("client_unavailable: the app that provides %s disconnected before answering; the outcome is unknown, so check before trying again.", pending.call.Name), IsError: true} + r.settleLocked(id, pending, answer) + } +} diff --git a/cmd/codeaf/appclienttools_test.go b/cmd/codeaf/appclienttools_test.go new file mode 100644 index 0000000000..d2a101c37a --- /dev/null +++ b/cmd/codeaf/appclienttools_test.go @@ -0,0 +1,890 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "reflect" + "strings" + "sync" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/remote" + "github.com/Agent-Field/codeaf/internal/session" +) + +type clientToolEmitter struct { + t *testing.T + mu sync.Mutex + calls []appwire.AgentsToolCall + seen chan appwire.AgentsToolCall + answer func(appwire.AgentsToolCall) +} + +// Source contract (a), (b): real provider calls name the entry returned by the +// app read projection, across a second send, a reopened journal and takeover resume. +func TestClientToolsSourceMessageWireAndRestart(t *testing.T) { + for _, stopped := range []bool{false, true} { + t.Run(fmt.Sprintf("stopped=%t", stopped), func(t *testing.T) { + b, door, emitter := clientToolBackend(t) + clientToolAttach(t, b, door) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + entered := make(chan struct{}) + var mu sync.Mutex + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request struct { + Messages []struct { + Role string `json:"role"` + } `json:"messages"` + Tools []json.RawMessage `json:"tools"` + } + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + tool := len(request.Tools) > 0 && len(request.Messages) > 0 && request.Messages[len(request.Messages)-1].Role == "user" + mu.Lock() + if tool { + calls++ + } + call := calls + mu.Unlock() + if tool && stopped && call == 1 { + close(entered) + select { + case <-r.Context().Done(): + case <-ctx.Done(): + } + return + } + w.Header().Set("Content-Type", "text/event-stream") + if tool { + fmt.Fprintf(w, "data: "+`{"choices":[{"index":0,"delta":{"role":"assistant","tool_calls":[{"index":0,"id":"provider-source-%d","type":"function","function":{"name":"app_note","arguments":"{}"}}]},"finish_reason":null}]}`+"\n\n", call) + fmt.Fprint(w, "data: "+`{"choices":[{"index":0,"delta":{},"finish_reason":"tool_calls"}]}`+"\n\n") + } else { + fmt.Fprint(w, "data: "+`{"choices":[{"index":0,"delta":{"role":"assistant","content":"done"},"finish_reason":"stop"}]}`+"\n\n") + } + fmt.Fprint(w, "data: [DONE]\n\n") + })) + t.Cleanup(server.Close) + answer := func(backend *appBackend, emit *clientToolEmitter) { + emit.answer = func(event appwire.AgentsToolCall) { + ack, err := backend.ClientToolResult(appwire.ClientToolResultParams{Call: event.Call, Result: json.RawMessage(`true`)}) + if err != nil || !ack.Accepted { + t.Errorf("tool result: %+v %v", ack, err) + } + } + } + answer(b, emitter) + path := filepath.Join(t.TempDir(), "session.jsonl") + cfg := session.Config{Workspace: t.TempDir(), SessionFile: path, Model: "test/model", APIKey: "fixture", BaseURL: server.URL, + OneModel: true, DeterministicNames: true, PromptProfile: "full", ClientTools: door} + agent, err := session.New(cfg) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = agent.Close() }) + b.locked(func() { b.agents[door.agent].live.conn.Conv = agent }) + drain := func(events <-chan session.Event) { + t.Helper() + for { + select { + case _, open := <-events: + if !open { + return + } + case <-ctx.Done(): + t.Fatal("turn did not finish") + } + } + } + events, err := agent.Submit(ctx, "first question") + if err != nil { + t.Fatal(err) + } + if stopped { + clientToolAwait(t, entered) + agent.InterruptFor(session.StopByTakeover) + } + drain(events) + messageID := func(conv *session.Agent, text string) string { + t.Helper() + for _, entry := range appEntriesOf(door.agent, conv.Transcript(), time.Time{}, nil) { + if entry.Kind == "user" && entry.Text == text && entry.MessageID != "" { + return entry.MessageID + } + } + t.Fatalf("read omitted message id for %q", text) + return "" + } + firstID := messageID(agent, "first question") + if !stopped { + first := clientToolAwait(t, emitter.seen) + if first.SourceMessageID != firstID { + t.Fatalf("first call=%+v read message=%q", first, firstID) + } + next, err := agent.SubmitMessage(ctx, session.Submission{Text: "second question", ClientID: "second-window"}) + if err != nil { + t.Fatal(err) + } + drain(next) + second := clientToolAwait(t, emitter.seen) + secondID := messageID(agent, "second question") + if second.SourceMessageID != secondID || secondID == firstID { + t.Fatalf("second call=%+v read ids=%q/%q", second, firstID, secondID) + } + } + if err := agent.Close(); err != nil { + t.Fatal(err) + } + fresh, freshDoor, freshEmitter := clientToolBackend(t) + clientToolAttach(t, fresh, freshDoor) + answer(fresh, freshEmitter) + cfg.ClientTools = freshDoor + reopened, err := session.New(cfg) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = reopened.Close() }) + fresh.locked(func() { fresh.agents[freshDoor.agent].live.conn.Conv = reopened }) + if got := messageID(reopened, "first question"); got != firstID { + t.Fatalf("reopened read changed message id: %q want %q", got, firstID) + } + if stopped { + resumed, ok := reopened.ResumeStoppedTurn(ctx) + if !ok { + t.Fatal("reopened stopped turn did not resume") + } + drain(resumed) + call := clientToolAwait(t, freshEmitter.seen) + if call.SourceMessageID != firstID { + t.Fatalf("resumed call=%+v read message=%q", call, firstID) + } + } + }) + } +} + +// Contract (a), (b), (e): a real session provider request crosses the app door, +// then a turn started after disconnect loses the tools and their instructions. +func TestClientToolsModelWireRoundTripAndDisconnectBoundary(t *testing.T) { + b, door, emitter := clientToolBackend(t) + clientToolAttach(t, b, door) + var mu sync.Mutex + var requests []struct { + Messages []struct { + Role string `json:"role"` + Content json.RawMessage `json:"content"` + Call string `json:"tool_call_id"` + } `json:"messages"` + Tools []struct { + Function struct { + Name string `json:"name"` + } `json:"function"` + } `json:"tools"` + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request struct { + Messages []struct { + Role string `json:"role"` + Content json.RawMessage `json:"content"` + Call string `json:"tool_call_id"` + } `json:"messages"` + Tools []struct { + Function struct { + Name string `json:"name"` + } `json:"function"` + } `json:"tools"` + } + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + mu.Lock() + foreground := len(request.Tools) > 0 + first := foreground && len(requests) == 0 + if foreground { + requests = append(requests, request) + } + mu.Unlock() + w.Header().Set("Content-Type", "text/event-stream") + if first { + fmt.Fprint(w, "data: "+`{"choices":[{"index":0,"delta":{"role":"assistant","tool_calls":[{"index":0,"id":"provider-client-1","type":"function","function":{"name":"app_note","arguments":"{\"id\":3}"}}]},"finish_reason":null}]}`+"\n\n") + fmt.Fprint(w, "data: "+`{"choices":[{"index":0,"delta":{},"finish_reason":"tool_calls"}]}`+"\n\n") + } else { + fmt.Fprint(w, "data: "+`{"choices":[{"index":0,"delta":{"role":"assistant","content":"done"},"finish_reason":"stop"}]}`+"\n\n") + } + fmt.Fprint(w, "data: [DONE]\n\n") + })) + t.Cleanup(server.Close) + disconnected := make(chan struct{}) + emitter.answer = func(event appwire.AgentsToolCall) { + ack, err := b.ClientToolResult(appwire.ClientToolResultParams{Call: event.Call, Result: json.RawMessage(`{ "note": "found" }`)}) + if err != nil || !ack.Accepted { + t.Errorf("wire answer: %+v %v", ack, err) + } + b.clientTools.disconnect() + close(disconnected) + } + agent, err := session.New(session.Config{Workspace: t.TempDir(), Model: "test/model", APIKey: "fixture", BaseURL: server.URL, + OneModel: true, DeterministicNames: true, PromptProfile: "full", ClientTools: door}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = agent.Close() }) + b.locked(func() { b.agents[door.agent].live.conn.Conv = agent }) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + events, err := agent.Submit(ctx, "read the app note") + if err != nil { + t.Fatal(err) + } + turnCall := "" + for { + select { + case event, open := <-events: + if !open { + goto finished + } + if event.Kind == session.EventToolBegin && event.Tool == "app_note" { + turnCall = event.CallID + } + case <-ctx.Done(): + t.Fatal("model round trip did not finish") + } + } +finished: + clientToolAwait(t, disconnected) + // Start this turn after disconnect has completed, independently of sender scheduling. + next, err := agent.Submit(ctx, "what tools remain?") + if err != nil { + t.Fatal(err) + } + for { + select { + case _, open := <-next: + if !open { + goto nextFinished + } + case <-ctx.Done(): + t.Fatal("post-disconnect turn did not finish") + } + } +nextFinished: + event := clientToolAwait(t, emitter.seen) + if event.TurnCall != turnCall || turnCall != "provider-client-1" { + t.Fatalf("turn.tool correlation: %+v / %q", event, turnCall) + } + mu.Lock() + defer mu.Unlock() + if len(requests) != 3 { + t.Fatalf("provider requests=%d", len(requests)) + } + first, second, afterDisconnect := requests[0], requests[1], requests[2] + if first.Tools[len(first.Tools)-1].Function.Name != "app_note" || !strings.Contains(string(first.Messages[0].Content), "Tools from the app") { + t.Fatal("initial request lost app definitions or instructions") + } + for _, tool := range afterDisconnect.Tools { + if tool.Function.Name == "app_note" { + t.Fatal("disconnected tool remained on the next request") + } + } + if strings.Contains(string(afterDisconnect.Messages[0].Content), "Tools from the app") { + t.Fatal("disconnected instructions remained on the next request") + } + found := false + for _, message := range second.Messages { + if message.Role != "tool" || message.Call != turnCall { + continue + } + if text := clientToolWireText(message.Content); text != `{"note":"found"}` { + t.Fatalf("model result: %s", message.Content) + } + found = true + } + if !found { + t.Fatal("model did not receive the wire result") + } +} + +func clientToolWireText(raw json.RawMessage) string { + var text string + if json.Unmarshal(raw, &text) == nil { + return text + } + var parts []struct { + Text string `json:"text"` + } + _ = json.Unmarshal(raw, &parts) + for _, part := range parts { + text += part.Text + } + return text +} + +func (e *clientToolEmitter) Emit(event string, data any) { + if event != appwire.EventAgentsToolCall { + return + } + call := data.(appwire.AgentsToolCall) + e.mu.Lock() + e.calls = append(e.calls, call) + e.mu.Unlock() + select { + case e.seen <- call: + case <-time.After(2 * time.Second): + e.t.Error("client tool event could not be delivered within two seconds") + return + } + if e.answer != nil { + e.answer(call) + } +} + +func clientToolBackend(t *testing.T) (*appBackend, *appClientToolDoor, *clientToolEmitter) { + t.Helper() + t.Setenv(home.EnvVar, t.TempDir()) + e := &clientToolEmitter{t: t, seen: make(chan appwire.AgentsToolCall, 20)} + b := newFakeBackend(e, nil) + id := "client-agent" + transcript := filepath.Join(t.TempDir(), id, "transcript.jsonl") + b.locked(func() { + b.agents[id] = &appAgent{view: appwire.Agent{ID: id}, transcript: transcript, + live: &appLive{turn: 7, conn: &appConn{Conv: &fakeConversation{}}}} + }) + t.Cleanup(func() { + b.clientTools.disconnect() + b.locked(func() { b.agents[id].live = nil }) + _ = b.Close() + }) + return b, &appClientToolDoor{backend: b, agent: id}, e +} + +func clientToolParams(agent, set, name string) appwire.ClientToolsSetParams { + return appwire.ClientToolsSetParams{Agent: agent, Set: set, Instructions: "Use this set.", Tools: []session.ClientTool{ + {Name: name, Description: "An app action", InputSchema: json.RawMessage(`{"type":"object"}`), Category: session.ActionEdit}, + }} +} + +func clientToolAttach(t *testing.T, b *appBackend, d *appClientToolDoor) session.ClientToolCall { + t.Helper() + if _, err := b.SetClientTools(clientToolParams(d.agent, "notes", "app_note")); err != nil { + t.Fatal(err) + } + return session.ClientToolCall{Set: "notes", Name: "app_note", TurnCall: "provider-1", Args: json.RawMessage(`{"id":3}`)} +} + +// Contract (a), (h): the Fresh/Open builder binds the final conversation, even when its config carried another door. +func TestClientToolsFreshAndOpenBindTheirConversation(t *testing.T) { + b, door, _ := clientToolBackend(t) + clientToolAttach(t, b, door) + previous := appClientToolsBackend.Swap(b) + t.Cleanup(func() { appClientToolsBackend.Store(previous) }) + root := t.TempDir() + dir := writeV3Session(t, root, "client-b", "", time.Time{}) + file := (session.Place{Dir: dir}).Transcript() + b.locked(func() { b.agents["client-b"] = &appAgent{view: appwire.Agent{ID: "client-b"}, transcript: file} }) + p := clientToolParams("client-b", "other", "app_other") + p.Instructions = "Only conversation B's instructions." + p.Tools[0].Category = session.ActionRead + if _, err := b.SetClientTools(p); err != nil { + t.Fatal(err) + } + cfg := session.Config{Workspace: root, ClientTools: door} + b.locked(func() { cfg.SessionFile = b.agents[door.agent].transcript }) + cfg.Place = session.Place{Dir: session.DirOf(cfg.SessionFile), Workspace: root} + for _, path := range []string{"fresh", "open"} { + t.Run(path, func(t *testing.T) { + open := appClientToolsOpener(func(opened session.Config) (*session.Agent, error) { + bound, ok := opened.ClientTools.(*appClientToolDoor) + if !ok || bound.agent != "client-b" { + t.Fatalf("replacement door: %+v", opened.ClientTools) + } + _, sets := bound.ClientToolSets() + if len(sets) != 1 || sets[0].ID != "other" || sets[0].Instructions != p.Instructions || + len(sets[0].Tools) != 1 || sets[0].Tools[0].Name != "app_other" || sets[0].Tools[0].Category != session.ActionRead { + t.Fatalf("replacement inherited the wrong sets: %+v", sets) + } + return nil, nil + }) + var next session.Config + var err error + if path == "fresh" { + next, err = v3PointAt(cfg, session.Place{Dir: dir, Workspace: root}) + } else { + next, err = v3Reopen(cfg, file, root) + } + if err != nil { + t.Fatal(err) + } + if _, err := open(next); err != nil { + t.Fatal(err) + } + }) + } + if cfg.ClientTools != door { + t.Fatal("opening a replacement changed the original config") + } + appClientToolsBackend.Store(nil) + open := appClientToolsOpener(func(opened session.Config) (*session.Agent, error) { + if opened.ClientTools != nil { + t.Fatal("opening without an app retained a stale door") + } + return nil, nil + }) + if _, err := open(cfg); err != nil { + t.Fatal(err) + } +} + +func clientToolStart(ctx context.Context, d *appClientToolDoor, call session.ClientToolCall) <-chan session.ClientToolAnswer { + done := make(chan session.ClientToolAnswer, 1) + go func() { + answer, err := d.CallClientTool(ctx, call) + if err != nil { + answer = session.ClientToolAnswer{Text: err.Error(), IsError: true} + } + done <- answer + }() + return done +} + +func clientToolAwait[T any](t *testing.T, done <-chan T) T { + t.Helper() + select { + case answer := <-done: + return answer + case <-time.After(2 * time.Second): + t.Fatal("client tool did not settle within two seconds") + } + var zero T + return zero +} + +// Contract (b), (d), source contract (b): redispatch preserves the source and emits once. +func TestClientToolsEventResultAndReplay(t *testing.T) { + b, door, emitter := clientToolBackend(t) + call := clientToolAttach(t, b, door) + call.SourceMessage = "msg-recorded" + first := clientToolStart(context.Background(), door, call) + event := clientToolAwait(t, emitter.seen) + if event.Agent != door.agent || event.Turn != 7 || event.TurnCall != call.TurnCall || event.Set != call.Set || event.Name != call.Name || + string(event.Args) != string(call.Args) || event.DeadlineMS != session.ClientToolDefaultDeadlineMS || event.Call != clientToolCallID(door.agent, call.TurnCall) || event.SourceMessageID != call.SourceMessage { + t.Fatalf("event shape: %+v", event) + } + second := clientToolStart(context.Background(), door, call) + ack, err := b.ClientToolResult(appwire.ClientToolResultParams{Call: event.Call, Result: json.RawMessage(`{ "ok": true }`)}) + if err != nil || !ack.Accepted { + t.Fatalf("answer: %+v %v", ack, err) + } + for _, done := range []<-chan session.ClientToolAnswer{first, second, clientToolStart(context.Background(), door, call)} { + if answer := clientToolAwait(t, done); answer.IsError || answer.Text != `{"ok":true}` { + t.Fatalf("result: %+v", answer) + } + } + for _, id := range []string{event.Call, "unknown"} { + ack, err := b.ClientToolResult(appwire.ClientToolResultParams{Call: id, Result: json.RawMessage(`null`)}) + if err != nil || ack.Accepted { + t.Fatalf("duplicate/unknown: %+v %v", ack, err) + } + } + emitter.mu.Lock() + defer emitter.mu.Unlock() + if len(emitter.calls) != 1 { + t.Fatalf("%d events for one call", len(emitter.calls)) + } + // A new registry represents a new process, with a different local counter. + fresh, freshDoor, freshEmitter := clientToolBackend(t) + clientToolAttach(t, fresh, freshDoor) + fresh.locked(func() { fresh.agents[freshDoor.agent].live.turn = 1 }) + replayed := clientToolStart(context.Background(), freshDoor, call) + again := clientToolAwait(t, freshEmitter.seen) + if again.Call != event.Call || again.SourceMessageID != event.SourceMessageID || again.Turn != 1 { + t.Fatalf("fresh process changed correlation: %+v / %+v", event, again) + } + if _, err := fresh.ClientToolResult(appwire.ClientToolResultParams{Call: again.Call, Result: json.RawMessage(`true`)}); err != nil { + t.Fatal(err) + } + clientToolAwait(t, replayed) +} + +// Contract (b): the pending entry exists before an emitter can answer synchronously. +func TestClientToolsFastErrorAnswer(t *testing.T) { + b, door, emitter := clientToolBackend(t) + call := clientToolAttach(t, b, door) + emitter.answer = func(event appwire.AgentsToolCall) { + text := "permission denied" + ack, err := b.ClientToolResult(appwire.ClientToolResultParams{Call: event.Call, Error: &text}) + if err != nil || !ack.Accepted { + t.Errorf("fast answer: %+v %v", ack, err) + } + } + answer := clientToolAwait(t, clientToolStart(context.Background(), door, call)) + if !answer.IsError || answer.Text != "notes could not do app_note: permission denied" { + t.Fatalf("error result: %+v", answer) + } +} + +// Contract (c), (d), (e): each ending is bounded and rejects every late answer. +func TestClientToolsDisconnectDeadlineAndTurnEnd(t *testing.T) { + for _, ending := range []string{"disconnect", "deadline", "turn end"} { + t.Run(ending, func(t *testing.T) { + b, door, emitter := clientToolBackend(t) + call := clientToolAttach(t, b, door) + if ending == "deadline" { + // Only this in-process fixture bypasses the wire's one-second minimum. + b.clientTools.mu.Lock() + state := b.clientTools.agents[door.agent] + state.sets[0].DeadlineMS = 15 + b.clientTools.agents[door.agent] = state + b.clientTools.mu.Unlock() + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := clientToolStart(ctx, door, call) + event := clientToolAwait(t, emitter.seen) + want := "client_timeout: the app that provides app_note did not answer within 0.015s; the outcome is unknown, so check before trying again." + switch ending { + case "disconnect": + b.clientTools.disconnect() + b.clientTools.disconnect() + want = "client_unavailable: the app that provides app_note disconnected before answering; the outcome is unknown, so check before trying again." + _, sets := door.ClientToolSets() + if len(sets) != 0 { + t.Fatal("disconnect kept sets") + } + case "turn end": + cancel() + want = clientToolTurnEnded(call.Name).Text + } + if answer := clientToolAwait(t, done); !answer.IsError || answer.Text != want { + t.Fatalf("ending: %+v", answer) + } + ack, err := b.ClientToolResult(appwire.ClientToolResultParams{Call: event.Call, Result: json.RawMessage(`1`)}) + if err != nil || ack.Accepted { + t.Fatalf("late answer: %+v %v", ack, err) + } + }) + } +} + +// Contract (c), (d): a full writer cannot hold the deadline, turn end or disconnect hostage. +func TestClientToolsBlockedEmitterDoesNotBlockEndings(t *testing.T) { + for _, ending := range []string{"deadline", "turn end", "disconnect"} { + t.Run(ending, func(t *testing.T) { + b, door, emitter := clientToolBackend(t) + call := clientToolAttach(t, b, door) + if ending == "deadline" { + // Only this fixture bypasses the wire's one-second minimum. + b.clientTools.mu.Lock() + b.clientTools.agents[door.agent].sets[0].DeadlineMS = 100 + b.clientTools.mu.Unlock() + } + entered, release, returned := make(chan struct{}), make(chan struct{}), make(chan struct{}) + var once sync.Once + unblock := func() { once.Do(func() { close(release) }) } + t.Cleanup(unblock) + emitter.answer = func(appwire.AgentsToolCall) { + close(entered) + <-release + close(returned) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := clientToolStart(ctx, door, call) + event := clientToolAwait(t, emitter.seen) + clientToolAwait(t, entered) + want := "client_timeout: the app that provides app_note did not answer within 0.1s; the outcome is unknown, so check before trying again." + switch ending { + case "turn end": + cancel() + want = clientToolTurnEnded(call.Name).Text + case "disconnect": + b.clientTools.disconnect() + want = "client_unavailable: the app that provides app_note disconnected before answering; the outcome is unknown, so check before trying again." + } + if answer := clientToolAwait(t, done); !answer.IsError || answer.Text != want { + t.Fatalf("blocked %s: %+v", ending, answer) + } + select { + case <-returned: + t.Fatal("emitter returned before the call ended") + default: + } + ack, err := b.ClientToolResult(appwire.ClientToolResultParams{Call: event.Call, Result: json.RawMessage(`null`)}) + if err != nil || ack.Accepted { + t.Fatalf("late answer: %+v %v", ack, err) + } + unblock() + clientToolAwait(t, returned) + b.clientTools.disconnect() + clientToolAwait(t, b.clientTools.senderDone) + }) + } +} + +// Contract (c), (d): calls ending behind a stalled writer are dropped before delivery. +func TestClientToolsEndedQueuedCallsAreNotEmitted(t *testing.T) { + for _, ending := range []string{"deadline", "turn end", "disconnect", "result"} { + t.Run(ending, func(t *testing.T) { + b, door, emitter := clientToolBackend(t) + call := clientToolAttach(t, b, door) + release := make(chan struct{}) + var once sync.Once + unblock := func() { once.Do(func() { close(release) }) } + t.Cleanup(unblock) + emitter.answer = func(event appwire.AgentsToolCall) { + if event.TurnCall == "provider-1" { + <-release + } + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + first := clientToolStart(ctx, door, call) + clientToolAwait(t, emitter.seen) + cancel() + clientToolAwait(t, first) + + queuedCtx, cancelQueued := context.WithCancel(context.Background()) + defer cancelQueued() + call.TurnCall = "queued-call" + id := clientToolCallID(door.agent, call.TurnCall) + r := &b.clientTools + r.mu.Lock() + pending, _, emit := r.beginLocked(queuedCtx, door.agent, id, call) + r.mu.Unlock() + if pending == nil || !emit { + t.Fatal("queued call was not registered") + } + r.queueEmission(b.emit, pending, appwire.AgentsToolCall{Agent: door.agent, Call: id, TurnCall: call.TurnCall}) + switch ending { + case "deadline": + r.mu.Lock() + pending.deadline = time.Now().Add(-time.Second) + r.mu.Unlock() + case "turn end": + cancelQueued() + case "disconnect": + r.disconnect() + case "result": + ack, err := b.ClientToolResult(appwire.ClientToolResultParams{Call: id, Result: json.RawMessage(`true`)}) + if err != nil || !ack.Accepted { + t.Fatalf("queued answer: %+v %v", ack, err) + } + } + done := make(chan session.ClientToolAnswer, 1) + go func() { done <- r.wait(queuedCtx, id, pending) }() + answer := clientToolAwait(t, done) + if answer.IsError != (ending != "result") { + t.Fatalf("queued ending: %+v", answer) + } + unblock() + if ending != "disconnect" { + call.TurnCall = "after-queue" + after := clientToolStart(context.Background(), door, call) + event := clientToolAwait(t, emitter.seen) + if event.TurnCall != "after-queue" { + t.Fatalf("ended call left the queue: %+v", event) + } + r.disconnect() + clientToolAwait(t, after) + } + clientToolAwait(t, r.senderDone) + select { + case event := <-emitter.seen: + t.Fatalf("extra event after queue drain: %+v", event) + default: + } + }) + } +} + +// Contract (e), (h): replacement is atomic and removal affects only its own set. +func TestClientToolsSetReplacementRemovalAndValidation(t *testing.T) { + b, door, _ := clientToolBackend(t) + clientToolAttach(t, b, door) + if _, err := b.SetClientTools(clientToolParams(door.agent, "second", "app_other")); err != nil { + t.Fatal(err) + } + beforeVersion, before := door.ClientToolSets() + for _, p := range []appwire.ClientToolsSetParams{ + clientToolParams(door.agent, "notes", "factory_add"), + clientToolParams(door.agent, "notes", "app_other"), + clientToolParams(door.agent, "notes", "change_setting"), + } { + if _, err := b.SetClientTools(p); err == nil || !strings.Contains(err.Error(), p.Tools[0].Name) { + t.Fatalf("collision accepted: %v", err) + } + version, sets := door.ClientToolSets() + if version != beforeVersion || !reflect.DeepEqual(sets, before) { + t.Fatal("refusal changed the registry") + } + } + replacement := clientToolParams(door.agent, "notes", "app_replaced") + if _, err := b.SetClientTools(replacement); err != nil { + t.Fatal(err) + } + replacement.Tools[0].InputSchema[0] = '[' + _, snapshots := door.ClientToolSets() + if !json.Valid(snapshots[0].Tools[0].InputSchema) { + t.Fatal("registry kept the caller's mutable schema") + } + removed, err := b.SetClientTools(appwire.ClientToolsSetParams{Agent: door.agent, Set: "notes"}) + if err != nil || len(removed.Tools) != 0 { + t.Fatalf("removal: %+v %v", removed, err) + } + _, sets := door.ClientToolSets() + if len(sets) != 1 || sets[0].ID != "second" { + t.Fatal("removal changed another set") + } + answer := clientToolAwait(t, clientToolStart(context.Background(), door, session.ClientToolCall{Set: "notes", Name: "app_replaced", Args: json.RawMessage(`{}`)})) + if !answer.IsError || answer.Text != "client_unavailable: the app that provides app_replaced is not connected; nothing was done." { + t.Fatalf("retired closure: %+v", answer) + } +} + +// Contract (h): aggregate limits and remote scope do not open a conversation. +func TestClientToolsAgentLimitsAndScope(t *testing.T) { + b, door, _ := clientToolBackend(t) + if _, err := b.SetClientTools(clientToolParams("missing", "notes", "app_note")); err == nil { + t.Fatal("unknown agent accepted") + } + for i := 0; i < session.ClientToolMaxSets; i++ { + if _, err := b.SetClientTools(clientToolParams(door.agent, fmt.Sprintf("set%d", i), fmt.Sprintf("app_%d", i))); err != nil { + t.Fatal(err) + } + } + if _, err := b.SetClientTools(clientToolParams(door.agent, "fifth", "app_fifth")); err == nil { + t.Fatal("fifth set accepted") + } + b.clientTools.mu.Lock() + b.clientTools.agents[door.agent] = clientToolAgentSets{} + b.clientTools.mu.Unlock() + for i := 0; i < 2; i++ { + p := clientToolParams(door.agent, fmt.Sprintf("set%d", i), "unused") + p.Tools = nil + for j := 0; j < session.ClientToolMaxPerSet; j++ { + p.Tools = append(p.Tools, clientToolParams(door.agent, p.Set, fmt.Sprintf("app_%d_%d", i, j)).Tools[0]) + } + if _, err := b.SetClientTools(p); err != nil { + t.Fatal(err) + } + } + if _, err := b.SetClientTools(clientToolParams(door.agent, "overflow", "app_extra")); err == nil { + t.Fatal("65th tool accepted") + } + b.locked(func() { b.agents[door.agent].live.conn.Conv = &remote.Agent{} }) + if _, err := b.SetClientTools(clientToolParams(door.agent, "remote", "app_remote")); err == nil || !strings.Contains(err.Error(), "client tools need a conversation this engine runs") { + t.Fatal(err) + } + b.locked(func() { b.agents[door.agent].live.conn.Assembled = true }) + if _, err := b.SetClientTools(appwire.ClientToolsSetParams{Agent: door.agent, Set: "set0"}); err != nil { + t.Fatalf("local transport proxy refused: %v", err) + } + b.locked(func() { b.agents[door.agent].live = nil }) + if _, err := b.SetClientTools(appwire.ClientToolsSetParams{Agent: door.agent, Set: "set0"}); err != nil { + t.Fatalf("cold attachment opened a conversation: %v", err) + } +} + +// Contract (d), source contract (b): pending joins retain the source without emitting. +func TestClientToolsPendingJoinAndCancelledAnswer(t *testing.T) { + b, door, emitter := clientToolBackend(t) + call := clientToolAttach(t, b, door) + call.SourceMessage = "msg-pending" + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := clientToolStart(ctx, door, call) + event := clientToolAwait(t, emitter.seen) + if event.SourceMessageID != call.SourceMessage { + t.Fatalf("pending event lost source: %+v", event) + } + r := &b.clientTools + r.mu.Lock() + first := r.pending[event.Call] + joined, _, emit := r.beginLocked(ctx, door.agent, event.Call, call) + r.mu.Unlock() + if first != joined || emit { + t.Fatal("duplicate dispatch did not join") + } + cancel() + ack, err := b.ClientToolResult(appwire.ClientToolResultParams{Call: event.Call, Result: json.RawMessage(`true`)}) + if err != nil || ack.Accepted { + t.Fatalf("cancelled turn accepted an answer: %+v %v", ack, err) + } + if answer := clientToolAwait(t, done); !answer.IsError || !strings.Contains(answer.Text, "turn ended") { + t.Fatal(answer) + } +} + +// Contract (h): local proxy inventories include dynamically armed engine names. +func TestClientToolsBuiltinInventoryFromLocalEngine(t *testing.T) { + b, door, _ := clientToolBackend(t) + clientToolAttach(t, b, door) + door.ClientToolBuiltins([]string{"app_reserved"}) + if _, err := b.SetClientTools(clientToolParams(door.agent, "other", "app_reserved")); err == nil || !strings.Contains(err.Error(), "app_reserved") { + t.Fatal(err) + } +} + +// Contract (d): replay memory expires and never exceeds its entry budget. +func TestClientToolsSettledMemoryBoundAndExpiry(t *testing.T) { + r := &appClientTools{pending: make(map[string]*clientToolPending)} + for i := 0; i < clientToolSettledMax+5; i++ { + id := fmt.Sprint(i) + pending := &clientToolPending{done: make(chan struct{})} + r.pending[id] = pending + r.settleLocked(id, pending, session.ClientToolAnswer{Text: id}) + } + if len(r.settled) != clientToolSettledMax || len(r.order) != clientToolSettledMax { + t.Fatal("unbounded replay memory") + } + r.pruneLocked(time.Now().Add(clientToolSettledTTL)) + if len(r.settled) != 0 { + t.Fatal("expired answers remain") + } + if clientToolCallID("a", "p") == clientToolCallID("b", "p") || clientToolCallID("a", "") == clientToolCallID("a", "") { + t.Fatal("call identity collision") + } +} + +// Contract (b): compact JSON is bounded with the exact number of dropped bytes. +func TestClientToolsBoundedOutputAndKinds(t *testing.T) { + data := []byte(`"` + strings.Repeat("é", session.ClientToolOutputMax) + `"`) + text := session.BoundClientToolOutput(string(data)) + if !strings.HasSuffix(text, fmt.Sprintf("[cut: %d bytes dropped]", len(data)-(session.ClientToolOutputMax-1))) { + t.Fatalf("cut marker: %.80s", text[len(text)-80:]) + } + b, door, _ := clientToolBackend(t) + clientToolAttach(t, b, door) + if category := b.clientToolEventCategory(b.agents[door.agent], session.Event{Tool: "app_note"}); category != session.ActionEdit { + t.Fatal(category) + } + if kind := appToolKind("app_note", "", session.ActionEdit); kind != appwire.ToolEdit { + t.Fatal(kind) + } +} + +// Contract (i): the feature remains usable by any app, with no domain vocabulary. +func TestClientToolsSourcesStayGeneric(t *testing.T) { + for _, path := range []string{"../../internal/appwire/client_tools.go", "../../internal/appwire/client_tools_test.go", + "../../internal/session/client_tools.go", "../../internal/session/client_tools_test.go", "appclienttools.go", "appclienttools_test.go", + "../../internal/manual/chat/client-tools.md"} { + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if strings.Contains(strings.ToLower(string(raw)), "work"+"stream") { + t.Errorf("%s contains domain vocabulary", path) + } + } +} diff --git a/cmd/codeaf/appcollectionkeep.go b/cmd/codeaf/appcollectionkeep.go new file mode 100644 index 0000000000..b64f4bfb65 --- /dev/null +++ b/cmd/codeaf/appcollectionkeep.go @@ -0,0 +1,39 @@ +package main + +import ( + "context" + "database/sql" + "errors" + "os" + "path/filepath" + "time" + + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/sqlitefile" + "github.com/Agent-Field/codeaf/internal/workspace" +) + +// A collection's reference makes a bare conversation intentional. Read-only mode +// also protects against the store disappearing between Stat and the SQLite open. +func collectionKeepsConversation(dir string) bool { + path := home.Join("v3", "collections.db") + if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { + return false + } else if err != nil { + return true + } + db, err := sql.Open("sqlite", sqlitefile.URI(path, "mode=ro&_pragma=busy_timeout(100)")) + if err != nil { + return true + } + defer db.Close() + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + var kept bool + // These membership columns exist in every collections schema. EXISTS avoids + // loading collection metadata or interpreting any newer collection features. + err = db.QueryRowContext(ctx, `SELECT EXISTS(SELECT 1 FROM memberships m + JOIN collections c ON c.id=m.collection_id WHERE m.kind=? AND m.ref_id=? AND m.session_id='')`, + workspace.ConversationKind, filepath.Base(filepath.Clean(dir))).Scan(&kept) + return kept || err != nil +} diff --git a/cmd/codeaf/appcollectionkeep_test.go b/cmd/codeaf/appcollectionkeep_test.go new file mode 100644 index 0000000000..41e282c152 --- /dev/null +++ b/cmd/codeaf/appcollectionkeep_test.go @@ -0,0 +1,745 @@ +package main + +import ( + "bytes" + "context" + "os" + "path/filepath" + "reflect" + "sync/atomic" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/agentdid" + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/teams" + "github.com/Agent-Field/codeaf/internal/workspace" +) + +func collectionKeepEnvironment(t *testing.T) (string, string) { + t.Helper() + root := t.TempDir() + t.Setenv(home.EnvVar, filepath.Join(root, "state")) + t.Setenv(config.ProfileDirEnv, filepath.Join(root, "profile")) + bucket, err := v3ProjectDir(root) + if err != nil { + t.Fatal(err) + } + return root, bucket +} + +// STAB2 E-B1 +func TestACollectionHeldBlankIsNeverTheConversationALaunchResumes(t *testing.T) { + for _, test := range []struct { + name string + spoken bool + held bool + unreadable bool + team bool + }{ + {name: "a held blank beside an older spoken conversation", spoken: true, held: true}, + {name: "only a held blank", held: true}, + {name: "an unreadable store beside an older spoken conversation", spoken: true, unreadable: true}, + {name: "only a blank with an unreadable store", unreadable: true}, + {name: "a blank held by both a team and a collection", held: true, team: true}, + } { + t.Run(test.name, func(t *testing.T) { + root, bucket := collectionKeepEnvironment(t) + blank := writeV3Session(t, bucket, "blank-agent", "", time.Time{}) + older := time.Date(2026, 10, 28, 9, 0, 0, 0, time.UTC) + newer := older.Add(24 * time.Hour) + if err := os.Chtimes(blank, newer, newer); err != nil { + t.Fatal(err) + } + var conversation string + if test.spoken { + conversation = writeV3Session(t, bucket, "spoken-agent", "keep these notes", older) + } + path := home.Join("v3", "collections.db") + if test.held { + store, err := workspace.Open(path) + if err != nil { + t.Fatal(err) + } + collection, err := store.Create(context.Background(), "Kept conversations") + if err != nil { + t.Fatal(err) + } + if err := store.Add(context.Background(), collection.ID, workspace.Ref{Kind: workspace.ConversationKind, ID: filepath.Base(blank)}); err != nil { + t.Fatal(err) + } + if err := store.Close(); err != nil { + t.Fatal(err) + } + } + if test.unreadable { + if err := os.WriteFile(path, []byte("unreadable database"), 0o600); err != nil { + t.Fatal(err) + } + } + if test.team { + transcript := session.Place{Dir: blank}.Transcript() + if err := teams.Save(config.ProfileDir(), []teams.Team{{ID: "aaaaaaaaaaaa", Name: "Notes", Manager: transcript, + Members: []teams.Member{{Key: transcript, File: transcript}}, + }}); err != nil { + t.Fatal(err) + } + } + + spoken, empty := v3ScanBucket(bucket) + if test.spoken { + if len(spoken) != 1 || spoken[0].dir != conversation { + t.Fatalf("resume candidates: %+v, want only %s", spoken, conversation) + } + } else if len(spoken) != 0 { + t.Fatalf("a blank became a resume candidate: %+v", spoken) + } + if len(empty) != 0 { + t.Fatalf("a kept blank was offered for reuse: %+v", empty) + } + folder, resumed, found := v3PickFolder(spoken, empty, true) + if test.spoken { + if !found || !resumed || folder.dir != conversation { + t.Fatalf("launch picked %+v, resumed=%t found=%t", folder, resumed, found) + } + } else if found || resumed { + t.Fatalf("launch did not ask for a fresh identity: %+v, resumed=%t found=%t", folder, resumed, found) + } + want := "" + if test.spoken { + want = session.Place{Dir: conversation}.Transcript() + } + if got := v3LatestTranscript(root); got != want { + t.Fatalf("host discovery found %q, want %q", got, want) + } + // Recheck a stale cleanup candidate as well as the scan's own pile. + v3ReapEmpty([]v3Folder{{dir: blank}}, "") + if _, err := os.Stat(blank); err != nil { + t.Fatalf("cleanup removed a kept blank: %v", err) + } + }) + } +} + +func TestABlankConversationCanBeReapedAfterItsLastCollectionReferenceIsRemoved(t *testing.T) { + for _, withSpoken := range []bool{false, true} { + name := "only a detached blank" + if withSpoken { + name = "a detached blank beside an older spoken conversation" + } + t.Run(name, func(t *testing.T) { + root, bucket := collectionKeepEnvironment(t) + dir := writeV3Session(t, bucket, "reserved-agent", "", time.Time{}) + older := time.Date(2026, 10, 28, 9, 0, 0, 0, time.UTC) + newer := older.Add(24 * time.Hour) + if err := os.Chtimes(dir, newer, newer); err != nil { + t.Fatal(err) + } + var conversation string + wantSpoken := 0 + if withSpoken { + conversation = writeV3Session(t, bucket, "spoken-agent", "keep these notes", older) + wantSpoken = 1 + } + store, err := workspace.Open(home.Join("v3", "collections.db")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + collection, err := store.Create(context.Background(), "Reserved") + if err != nil { + t.Fatal(err) + } + ref := workspace.Ref{Kind: workspace.ConversationKind, ID: filepath.Base(dir)} + if err := store.Add(context.Background(), collection.ID, ref); err != nil { + t.Fatal(err) + } + spoken, empty := v3ScanBucket(bucket) + if len(spoken) != wantSpoken || withSpoken && spoken[0].dir != conversation || len(empty) != 0 || v3EmptySession(dir) { + t.Fatalf("a held blank was offered for resume or cleanup: spoken=%+v empty=%+v", spoken, empty) + } + if err := store.Remove(context.Background(), collection.ID, ref); err != nil { + t.Fatal(err) + } + spoken, empty = v3ScanBucket(bucket) + if len(spoken) != wantSpoken || withSpoken && spoken[0].dir != conversation || len(empty) != 1 || empty[0].dir != dir || !v3EmptySession(dir) { + t.Fatalf("a detached blank was still kept: spoken=%+v empty=%+v", spoken, empty) + } + want := dir + if withSpoken { + want = conversation + } + folder, resumed, found := v3PickFolder(spoken, empty, false) + if !found || resumed != withSpoken || folder.dir != want { + t.Fatalf("host discovery picked %+v, resumed=%t found=%t", folder, resumed, found) + } + if got := v3LatestTranscript(root); got != (session.Place{Dir: want}).Transcript() { + t.Fatalf("host discovery found %q, want the transcript in %s", got, want) + } + folder, resumed, found = v3PickFolder(spoken, empty, true) + if withSpoken { + if !found || !resumed || folder.dir != conversation { + t.Fatalf("launch picked %+v, resumed=%t found=%t", folder, resumed, found) + } + } else if found || resumed { + t.Fatalf("launch did not ask for a fresh identity: %+v, resumed=%t found=%t", folder, resumed, found) + } + v3ReapEmpty(empty, "") + if _, err := os.Stat(dir); !os.IsNotExist(err) { + t.Fatalf("cleanup kept a detached blank: %v", err) + } + }) + } +} + +// Contract (g): collection-held blanks survive launch and the app's cleanup. +func TestCollectionHeldConversationSurvivesEveryBlankCleanup(t *testing.T) { + _, bucket := collectionKeepEnvironment(t) + held := writeV3Session(t, bucket, "collection-held", "", time.Time{}) + unheld := writeV3Session(t, bucket, "unheld", "", time.Time{}) + path := home.Join("v3", "collections.db") + store, err := workspace.Open(path) + if err != nil { + t.Fatal(err) + } + collection, err := store.Create(context.Background(), "Kept conversations") + if err != nil { + t.Fatal(err) + } + if err := store.Add(context.Background(), collection.ID, workspace.Ref{Kind: workspace.ConversationKind, ID: filepath.Base(held)}); err != nil { + t.Fatal(err) + } + if err := store.Close(); err != nil { + t.Fatal(err) + } + before, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + beforeInfo, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !v3EmptyUnassignedSession(held) { + t.Fatal("a collection reference changed whether the conversation is blank") + } + if v3EmptySession(held) { + t.Fatal("collection-held conversation is litter") + } + if !v3EmptyUnassignedSession(unheld) { + t.Fatal("unheld empty conversation was retained") + } + v3ReapEmpty([]v3Folder{{dir: held}, {dir: unheld}}, "") + if _, err := os.Stat(held); err != nil { + t.Fatal("launch reap removed collection-held conversation") + } + if _, err := os.Stat(unheld); !os.IsNotExist(err) { + t.Fatalf("launch reap kept unheld conversation: %v", err) + } + b := newFakeBackend(newAppRecorder(), nil) + b.profileDir = config.ProfileDir() + row, ok := appRowOnDisk(filepath.Base(held)) + if !ok { + t.Fatal("bare row missing") + } + b.remember(row) + a := b.agent(row.ID) + b.locked(func() { a.minted = true }) + if b.reapBlank(a) { + t.Fatal("app cleanup removed collection-held conversation") + } + if err := b.Close(); err != nil { + t.Fatal(err) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + afterInfo, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(before, after) || !beforeInfo.ModTime().Equal(afterInfo.ModTime()) { + t.Fatal("blank cleanup wrote collections.db") + } +} + +// Contract (g): the real launch-time sweep keeps collection-held litter and unreadable stores. +func TestCollectionHeldConversationSurvivesBackgroundSweep(t *testing.T) { + for _, unreadable := range []bool{false, true} { + name := "held and unheld" + if unreadable { + name = "unreadable store" + } + t.Run(name, func(t *testing.T) { + root, bucket := collectionKeepEnvironment(t) + held := writeV3Session(t, bucket, "collection-held", "", time.Time{}) + unheld := writeV3Session(t, bucket, "unheld", "", time.Time{}) + old := time.Now().Add(-8 * 24 * time.Hour) + for _, dir := range []string{held, unheld} { + meta := session.Meta{ID: filepath.Base(dir), Workspace: root, LaunchDir: root, Created: old} + if err := session.SaveMeta(dir, meta); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(dir, old, old); err != nil { + t.Fatal(err) + } + } + path := home.Join("v3", "collections.db") + store, err := workspace.Open(path) + if err != nil { + t.Fatal(err) + } + collection, err := store.Create(context.Background(), "Kept conversations") + if err != nil { + t.Fatal(err) + } + if err := store.Add(context.Background(), collection.ID, workspace.Ref{Kind: workspace.ConversationKind, ID: filepath.Base(held)}); err != nil { + t.Fatal(err) + } + if err := store.Close(); err != nil { + t.Fatal(err) + } + if unreadable { + if err := os.WriteFile(path, []byte("unreadable database"), 0o600); err != nil { + t.Fatal(err) + } + } + before, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + proc := &v3Process{} + proc.startPlaceSweep() + t.Cleanup(proc.stopPlaceSweep) + select { + case <-proc.sweepDone: + case <-time.After(3 * time.Second): + t.Fatal("background sweep did not finish") + } + if _, err := os.Stat(held); err != nil { + t.Fatalf("background sweep removed held conversation: %v", err) + } + _, err = os.Stat(unheld) + if unreadable && err != nil { + t.Fatalf("background sweep discarded an uncertain conversation: %v", err) + } + if !unreadable && !os.IsNotExist(err) { + t.Fatalf("background sweep kept unheld control: %v", err) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + afterInfo, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(before, after) || !info.ModTime().Equal(afterInfo.ModTime()) { + t.Fatal("background sweep wrote collections.db") + } + }) + } +} + +// Contract (g): missing means unheld; every unreadable database means keep. +func TestCollectionKeepMissingUnreadableAndNoWrites(t *testing.T) { + _, bucket := collectionKeepEnvironment(t) + dir := writeV3Session(t, bucket, "empty-chat", "", time.Time{}) + path := home.Join("v3", "collections.db") + if collectionKeepsConversation(dir) { + t.Fatal("missing store retained conversation") + } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatal("read created a store") + } + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + bad := []byte("not a SQLite database") + if err := os.WriteFile(path, bad, 0o600); err != nil { + t.Fatal(err) + } + if !collectionKeepsConversation(dir) || v3EmptySession(dir) { + t.Fatal("uncertainty discarded a conversation") + } + got, err := os.ReadFile(path) + if err != nil || !bytes.Equal(got, bad) { + t.Fatal("read repaired or migrated a corrupt store") + } + if err := os.Remove(path); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(path, 0o700); err != nil { + t.Fatal(err) + } + if !collectionKeepsConversation(dir) { + t.Fatal("unopenable store discarded a conversation") + } +} + +// STAB2 E-M1 +func TestACollectionHeldBlankAgentIsLiveAfterRestart(t *testing.T) { + for _, title := range []string{"Reserved notes", ""} { + name := "a saved title" + if title == "" { + name = "no saved title" + } + t.Run(name, func(t *testing.T) { + root, bucket := collectionKeepEnvironment(t) + const id = "reserved-agent" + dir := writeV3Session(t, bucket, id, "", time.Time{}) + if err := session.SaveMeta(dir, session.Meta{ID: id, Workspace: root, Title: title, + Created: time.Date(2026, 10, 29, 9, 0, 0, 0, time.UTC), + }); err != nil { + t.Fatal(err) + } + store, err := workspace.Open(home.Join("v3", "collections.db")) + if err != nil { + t.Fatal(err) + } + collection, err := store.Create(context.Background(), "Reserved") + if err != nil { + t.Fatal(err) + } + ref := workspace.Ref{Kind: workspace.ConversationKind, ID: id} + if err := store.Add(context.Background(), collection.ID, ref); err != nil { + t.Fatal(err) + } + if err := store.Close(); err != nil { + t.Fatal(err) + } + + opened := 0 + b := newFakeBackend(newAppRecorder(), func(ask appOpenAsk) (*appConn, error) { + opened++ + return refuseAppOpen(ask) + }) + t.Cleanup(func() { _ = b.Close() }) + b.profileDir = config.ProfileDir() + checkUnlisted := func() { + t.Helper() + list, err := b.ListAgents("") + if err != nil { + t.Fatal(err) + } + if _, found := listsAgent(list, id); found { + t.Fatal("agents.list included a blank conversation") + } + } + checkUnlisted() + open, err := b.OpenCollection(collection.ID, 1, 500, "") + if err != nil || len(open.Members) != 1 { + t.Fatalf("collections.open: %+v, %v", open, err) + } + member := open.Members[0] + if member.ID != id || member.Target.State != appwire.TargetLive || member.Target.Title != title { + t.Fatalf("collections.open member: %+v", member) + } + expanded, err := b.ExpandCollection(collection.ID, 3, 200) + if err != nil || expanded.Missing != 0 || expanded.Unavailable != 0 || len(expanded.Refs) != 1 { + t.Fatalf("collections.expand: %+v, %v", expanded, err) + } + live := expanded.Refs[0] + if live.ID != id || live.Target.State != appwire.TargetLive || live.Target.Title != title { + t.Fatalf("collections.expand member: %+v", live) + } + checkUnlisted() + if opened != 0 { + t.Fatalf("resolving collection members opened %d conversations", opened) + } + + missing := appwire.Ref{Kind: appwire.RefConversation, ID: "missing-agent"} + if _, err := b.AddToCollection(collection.ID, []appwire.Member{{Ref: missing}}); err != nil { + t.Fatal(err) + } + open, err = b.OpenCollection(collection.ID, 1, 500, "") + if err != nil || len(open.Members) != 2 { + t.Fatalf("collections.open with a missing agent: %+v, %v", open, err) + } + foundMissing := false + for _, member := range open.Members { + if member.ID == missing.ID { + foundMissing = true + if member.Target.State != appwire.TargetDeleted { + t.Fatalf("missing agent: %+v", member) + } + } + } + if !foundMissing { + t.Fatal("collections.open omitted the deleted member") + } + expanded, err = b.ExpandCollection(collection.ID, 3, 200) + if err != nil || expanded.Missing != 1 || expanded.Unavailable != 0 || len(expanded.Refs) != 1 || expanded.Refs[0].ID != id { + t.Fatalf("collections.expand with a missing agent: %+v, %v", expanded, err) + } + if opened != 0 { + t.Fatalf("resolving a missing member opened %d conversations", opened) + } + }) + } +} + +// STAB2-fix R4 +func TestACollectionReadNeverOpensAHeldBlankToResolveItsChildren(t *testing.T) { + for _, method := range []string{"collections.open", "collections.expand"} { + t.Run(method, func(t *testing.T) { + root, bucket := collectionKeepEnvironment(t) + const id = "reserved-agent" + dir := writeV3Session(t, bucket, id, "", time.Time{}) + if err := session.SaveMeta(dir, session.Meta{ID: id, Workspace: root, Title: "Reserved notes", + Created: time.Date(2026, 10, 29, 9, 0, 0, 0, time.UTC), + }); err != nil { + t.Fatal(err) + } + transcript := session.Place{Dir: dir}.Transcript() + if err := teams.Update(config.ProfileDir(), func(f *teams.File) error { + crew := teams.NewID() + f.Teams = append(f.Teams, teams.Team{ID: crew, Name: "Notes", Made: time.Now()}) + if err := f.AddMember(crew, teams.Member{Key: transcript, File: transcript, Where: root, Word: "notes", Handle: "notes"}); err != nil { + return err + } + return f.SetManager(crew, transcript) + }); err != nil { + t.Fatal(err) + } + store, err := workspace.Open(home.Join("v3", "collections.db")) + if err != nil { + t.Fatal(err) + } + collection, err := store.Create(context.Background(), "Reserved") + if err != nil { + t.Fatal(err) + } + refs := []workspace.Ref{ + {Kind: workspace.ConversationKind, ID: id}, + {Kind: workspace.TaskKind, ID: "1", SessionID: id}, + {Kind: workspace.ProcessKind, ID: id + ":p1", SessionID: id}, + } + // Reads visit newest members first, so resolve the conversation's + // fallback before either child asks for its owner. + if _, _, err := store.AddRefs(context.Background(), collection.ID, []workspace.Addition{ + {Ref: refs[0], AddedAt: 3}, + {Ref: refs[1], AddedAt: 2}, + {Ref: refs[2], AddedAt: 1}, + }); err != nil { + t.Fatal(err) + } + if err := store.Close(); err != nil { + t.Fatal(err) + } + + var opened atomic.Int32 + b := newFakeBackend(newAppRecorder(), func(ask appOpenAsk) (*appConn, error) { + opened.Add(1) + return refuseAppOpen(ask) + }) + t.Cleanup(func() { + _ = b.Close() + if got := opened.Load(); got != 0 { + t.Errorf("resolving collection members opened %d conversations", got) + } + }) + b.profileDir = config.ProfileDir() + list, err := b.ListAgents("") + if err != nil { + t.Fatal(err) + } + if _, found := listsAgent(list, id); found { + t.Fatal("agents.list included a blank conversation") + } + if method == "collections.open" { + open, err := b.OpenCollection(collection.ID, 1, 500, "") + if err != nil || len(open.Members) != len(refs) { + t.Fatalf("collections.open: %+v, %v", open, err) + } + for i, member := range open.Members { + if member.Kind != string(refs[i].Kind) || member.ID != refs[i].ID || member.SessionID != refs[i].SessionID { + t.Fatalf("collections.open member %d: %+v", i, member) + } + state := appwire.TargetDeleted + if i == 0 { + state = appwire.TargetLive + if member.Target.Title != "Reserved notes" || !reflect.DeepEqual(member.Target.In, []string{"Notes"}) { + t.Fatalf("held blank conversation: %+v", member) + } + } + if member.Target.State != state { + t.Fatalf("collections.open member: %+v, want %s", member, state) + } + } + } else { + expanded, err := b.ExpandCollection(collection.ID, 3, 200) + if err != nil || expanded.Missing != 2 || expanded.Unavailable != 0 || len(expanded.Refs) != 1 { + t.Fatalf("collections.expand: %+v, %v", expanded, err) + } + live := expanded.Refs[0] + if live.Kind != appwire.RefConversation || live.ID != id || live.Target.State != appwire.TargetLive || live.Target.Title != "Reserved notes" || !reflect.DeepEqual(live.Target.In, []string{"Notes"}) { + t.Fatalf("collections.expand member: %+v", live) + } + } + }) + } +} + +// STAB2-fix R1 +func TestAPermanentlyDeletedConversationStaysDeletedInItsCollection(t *testing.T) { + for _, pending := range []bool{false, true} { + name := "completed deletion" + if pending { + name = "pending deletion" + } + t.Run(name, func(t *testing.T) { + root, bucket := collectionKeepEnvironment(t) + const id = "deleted-agent" + dir := writeV3Session(t, bucket, id, "saved notes", time.Date(2026, 10, 29, 9, 0, 0, 0, time.UTC)) + meta, err := session.LoadMeta(dir) + if err != nil { + t.Fatal(err) + } + meta.Workspace, meta.Title = root, "Deleted notes" + if err := session.SaveMeta(dir, meta); err != nil { + t.Fatal(err) + } + store, err := workspace.Open(home.Join("v3", "collections.db")) + if err != nil { + t.Fatal(err) + } + collection, err := store.Create(context.Background(), "Saved conversations") + if err != nil { + t.Fatal(err) + } + ref := workspace.Ref{Kind: workspace.ConversationKind, ID: id} + if err := store.Add(context.Background(), collection.ID, ref); err != nil { + t.Fatal(err) + } + if err := store.Close(); err != nil { + t.Fatal(err) + } + + transcript := session.Place{Dir: dir}.Transcript() + if pending { + // A staged deletion keeps its cleanup receipt and pending journal. + if err := os.WriteFile(filepath.Join(dir, ".delete-task-cleanup.json"), []byte("{}"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, teams.ConversationDeletedFile), []byte("deleted\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Rename(transcript, transcript+".delete-pending"); err != nil { + t.Fatal(err) + } + if row, ok := session.ReadRows([]string{transcript})[filepath.Clean(transcript)]; !ok || !row.DeletionPending { + t.Fatalf("pending deletion fixture: %+v, found=%t", row, ok) + } + } else { + if err := session.DeleteConversationUnder(session.PlacesRoot(), config.ProfileDir(), transcript, nil, nil); err != nil { + t.Fatal(err) + } + } + if _, err := os.Stat(transcript); !os.IsNotExist(err) { + t.Fatalf("permanent deletion kept the transcript: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, teams.ConversationDeletedFile)); err != nil { + t.Fatalf("permanent deletion lost its lifetime marker: %v", err) + } + if saved, err := session.LoadMeta(dir); err != nil || saved.ID != id || saved.Title != meta.Title { + t.Fatalf("permanent deletion lost its saved metadata: %+v, %v", saved, err) + } + + b := newFakeBackend(newAppRecorder(), nil) + t.Cleanup(func() { _ = b.Close() }) + b.profileDir = config.ProfileDir() + open, err := b.OpenCollection(collection.ID, 1, 500, "") + if err != nil || len(open.Members) != 1 { + t.Fatalf("collections.open: %+v, %v", open, err) + } + if member := open.Members[0]; member.ID != id || member.Target.State != appwire.TargetDeleted { + t.Fatalf("collections.open revived a permanently deleted conversation: %+v", member) + } + expanded, err := b.ExpandCollection(collection.ID, 3, 200) + if err != nil || expanded.Missing != 1 || expanded.Unavailable != 0 || len(expanded.Refs) != 0 { + t.Fatalf("collections.expand: %+v, %v", expanded, err) + } + + other, err := b.CreateCollection("Other conversations") + if err != nil { + t.Fatal(err) + } + from := appwire.Ref{Kind: appwire.RefConversation, ID: "another-agent"} + if _, err := b.AddToCollection(other.ID, []appwire.Member{{Ref: from}}); err != nil { + t.Fatal(err) + } + to := appwire.Ref{Kind: appwire.RefConversation, ID: id} + if _, err := b.RelinkCollection(other.ID, from, to); wireCode(err) != appwire.CodeNotFound { + t.Fatalf("relink to a permanently deleted conversation: %v", err) + } + open, err = b.OpenCollection(other.ID, 1, 500, "") + if err != nil || len(open.Members) != 1 || open.Members[0].ID != from.ID { + t.Fatalf("refused relink changed the collection: %+v, %v", open, err) + } + }) + } +} + +// Contract (g): a restart recovers every identity field from a never-spoken meta. +func TestCollectionHeldConversationIsFoundAndOpenedAfterRestart(t *testing.T) { + root, bucket := collectionKeepEnvironment(t) + const id = "reserved-agent" + dir := writeV3Session(t, bucket, id, "", time.Time{}) + did, err := agentdid.New() + if err != nil { + t.Fatal(err) + } + created := time.Date(2026, 10, 29, 9, 0, 0, 0, time.UTC) + meta := session.Meta{ID: id, Workspace: root, Owned: true, Title: "Reserved notes", Model: "vendor/model", Archived: true, SpentUSD: 1.25, Created: created, + AgentIdentity: &session.AgentIdentity{DID: did, Session: id}} + if err := session.SaveMeta(dir, meta); err != nil { + t.Fatal(err) + } + store, err := workspace.Open(home.Join("v3", "collections.db")) + if err != nil { + t.Fatal(err) + } + collection, err := store.Create(context.Background(), "Reserved") + if err != nil { + t.Fatal(err) + } + if err := store.Add(context.Background(), collection.ID, workspace.Ref{Kind: workspace.ConversationKind, ID: id}); err != nil { + t.Fatal(err) + } + _ = store.Close() + want := session.SessionRow{ID: id, AgentDID: did, Dir: dir, Transcript: (session.Place{Dir: dir}).Transcript(), Workspace: root, Owned: true, + Title: meta.Title, Model: meta.Model, Archived: true, Spend: meta.SpentUSD, At: created, Created: created} + row, ok := appRowOnDisk(id) + if !ok || !reflect.DeepEqual(row, want) { + t.Fatalf("identity after restart: %+v want %+v", row, want) + } + opened := 0 + b := newFakeBackend(newAppRecorder(), func(ask appOpenAsk) (*appConn, error) { + opened++ + if ask.Transcript != want.Transcript || ask.Workspace != root || ask.Model != meta.Model { + t.Errorf("open identity: %+v", ask) + } + return &appConn{Conv: &fakeConversation{model: ask.Model}, Workspace: ask.Workspace, Transcript: ask.Transcript, Release: func() {}}, nil + }) + t.Cleanup(func() { _ = b.Close() }) + found, err := b.find(id) + if err != nil || found.view.DID != did || found.view.Title != meta.Title || opened != 0 { + t.Fatalf("find: %+v %v", found, err) + } + view, err := b.OpenAgent(id) + if err != nil || view.ID != id || view.DID != did || view.Title != meta.Title || view.Model != meta.Model || opened != 1 { + t.Fatalf("agents.open: %+v %v", view, err) + } + if view.State == appwire.StateWorking { + t.Fatal("opening started a turn") + } +} diff --git a/cmd/codeaf/appcollections.go b/cmd/codeaf/appcollections.go index 4471d60792..1d25860c2c 100644 --- a/cmd/codeaf/appcollections.go +++ b/cmd/codeaf/appcollections.go @@ -10,12 +10,12 @@ package main // // RESOLUTION ASKS EACH OWNER ONCE PER CALL, NEVER ONCE PER MEMBER. A call // takes one agents listing (every state, closed and archived included), one -// crews listing, one task list and one process list per distinct agent, and -// one look for a git work tree per directory; a collection of a thousand -// files in one repository costs one walk up to its .git. No transcript is -// read to resolve anything, and a member whose owner cannot be asked is -// "unknown", which says nothing about the member itself, with a reason that -// names the owner and why it could not be asked. +// folder lookup per unlisted conversation id, one crews listing, one task list +// and one process list per distinct listed agent, and one look for a git work +// tree per directory; a collection of a thousand files in one repository costs +// one walk up to its .git. No transcript is read to resolve anything. A member +// whose owner cannot be asked is "unknown", which says nothing about the member +// itself, with a reason naming the owner and why it could not be asked. import ( "bufio" @@ -34,6 +34,7 @@ import ( "github.com/Agent-Field/codeaf/internal/appwire" "github.com/Agent-Field/codeaf/internal/home" "github.com/Agent-Field/codeaf/internal/redact" + "github.com/Agent-Field/codeaf/internal/teams" "github.com/Agent-Field/codeaf/internal/workspace" ) @@ -123,11 +124,11 @@ func (b *appBackend) collectionChanged(id string, deleted bool) { } func appStoreRef(r appwire.Ref) workspace.Ref { - return workspace.Ref{Kind: workspace.Kind(r.Kind), ID: r.ID, SessionID: r.SessionID} + return workspace.Ref{Kind: workspace.Kind(r.Kind), ID: r.ID, SessionID: r.SessionID, Factory: r.Factory} } func appWireRef(r workspace.Ref) appwire.Ref { - return appwire.Ref{Kind: string(r.Kind), ID: r.ID, SessionID: r.SessionID} + return appwire.Ref{Kind: string(r.Kind), ID: r.ID, SessionID: r.SessionID, Factory: r.Factory} } // appMoment is a stored moment on the wire: absent when unknown. @@ -308,6 +309,7 @@ func (b *appBackend) OpenCollection(id string, depth, limit int, after string) ( return appwire.CollectionOpenResult{}, appCollectionError(err) } r := newCollectionResolver(b) + r.prepareFactoryTree(tree.Level) var nodes func(level workspace.Level) []appwire.Node nodes = func(level workspace.Level) []appwire.Node { out := make([]appwire.Node, 0, len(level.Members)) @@ -359,6 +361,7 @@ func (b *appBackend) ExpandCollection(id string, depth, limit int) (appwire.Coll Missing: e.Deleted, Unread: e.Unread, Collections: e.Collections, More: e.More, } r := newCollectionResolver(b) + r.prepareFactoryRefs(e.Refs) for _, m := range e.Refs { target := r.target(m.Ref) switch target.State { @@ -490,18 +493,20 @@ func appUnaskedReason(owner string, err error) string { type collectionResolver struct { b *appBackend - agentsRead bool - agents map[string]appwire.Agent - agentsErr error + agentsRead bool + agents map[string]appwire.Agent + agentsErr error + savedAgents map[string]appwire.Agent crewsRead bool crews []appwire.Crew crewByID map[string]int crewsErr error - tasks map[string]appOwned[appwire.Task] - procs map[string]appOwned[appwire.Process] - git map[string]*appGitTree + tasks map[string]appOwned[appwire.Task] + procs map[string]appOwned[appwire.Process] + git map[string]*appGitTree + factoryTargets map[workspace.Ref]appwire.Target } // appOwned is one agent's tasks or processes by wire id, or why they could @@ -517,8 +522,13 @@ func newCollectionResolver(b *appBackend) *collectionResolver { func (r *collectionResolver) target(ref workspace.Ref) appwire.Target { switch ref.Kind { + case workspace.FactoryJobKind: + if _, ok := r.factoryTargets[ref]; !ok { + r.prepareFactoryRefs([]workspace.Member{{Ref: ref}}) + } + return r.factoryTargets[ref] case workspace.ConversationKind: - a, not := r.agent(ref.ID) + a, not := r.conversation(ref.ID) if not != nil { return *not } @@ -573,8 +583,8 @@ func (r *collectionResolver) target(ref workspace.Ref) appwire.Target { return appwire.Target{State: appwire.TargetDeleted} } -// agent is one agent off the one listing, or the target a member of it is -// when there is no such agent (deleted) or no listing (unknown, and why). +// agent is one agent off the listing, or the target a member +// of it is when there is no such agent (deleted) or no listing (unknown, and why). func (r *collectionResolver) agent(id string) (appwire.Agent, *appwire.Target) { if !r.agentsRead { r.agentsRead = true @@ -589,13 +599,42 @@ func (r *collectionResolver) agent(id string) (appwire.Agent, *appwire.Target) { t := appUnknown("the conversation list", r.agentsErr) return appwire.Agent{}, &t } - a, ok := r.agents[id] - if !ok { + a := r.agents[id] + if a.ID == "" { return appwire.Agent{}, &appwire.Target{State: appwire.TargetDeleted} } return a, nil } +// conversation also checks saved folders for explicitly referenced agents. +// Its cache stays separate so child references still require a listed owner. +func (r *collectionResolver) conversation(id string) (appwire.Agent, *appwire.Target) { + a, not := r.agent(id) + if not == nil || not.State != appwire.TargetDeleted { + return a, not + } + if r.savedAgents == nil { + r.savedAgents = map[string]appwire.Agent{} + } + a, ok := r.savedAgents[id] + if !ok { + // Named blanks stay out of agents.list but can still be held by a + // collection. Cache misses too, since several members can name one agent. + if row, found := appRowOnDisk(id); found && !row.DeletionPending { + // Permanent deletion retains metadata, but its lifetime marker + // prevents the saved folder from becoming a live target again. + if _, err := os.Stat(filepath.Join(row.Dir, teams.ConversationDeletedFile)); errors.Is(err, fs.ErrNotExist) { + a = appRowView(row) + } + } + r.savedAgents[id] = a + } + if a.ID == "" { + return appwire.Agent{}, not + } + return a, nil +} + // appFindOwned is one task or process of an agent, read with list once per // agent per call, or the target it is when it is not there; owner names the // list in an unknown target's reason. diff --git a/cmd/codeaf/appcollections_factory.go b/cmd/codeaf/appcollections_factory.go new file mode 100644 index 0000000000..d5927afd69 --- /dev/null +++ b/cmd/codeaf/appcollections_factory.go @@ -0,0 +1,246 @@ +package main + +import ( + "context" + "errors" + "strconv" + "sync" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/factory/api" + "github.com/Agent-Field/codeaf/internal/guard" + "github.com/Agent-Field/codeaf/internal/workspace" +) + +const ( + factoryReadTimeout = 3 * time.Second + factoryQueueFresh = 10 * time.Second + factoryQueueCacheMax = 32 + factoryMissingJobsMax = 20 + factoryReadParallel = 8 +) + +// collectionFactory belongs to one backend so cached queue reads cannot cross profiles. +type collectionFactory struct { + queue func(context.Context, string) (api.Queue, error) + job func(context.Context, string, int) (api.Job, error) + cacheMu sync.Mutex + cache map[string]factoryQueueRead + flights map[string]chan struct{} +} + +type factoryJob struct { + Factory string + Number int +} + +type factoryQueueRead struct { + at time.Time + jobs map[int]api.Job + err error +} + +// factoryQueue shares successful and failed reads briefly, including an +// in-flight read, so concurrent list/get calls cannot multiply Factory work. +func (b *appBackend) factoryQueue(ctx context.Context, factory string) (entry factoryQueueRead) { + state := &b.collectionFactory + entry, flight, readQueue := state.factoryQueueStart(factory, b.now()) + if flight == nil { + return entry + } + if !readQueue { + select { + case <-flight: + return b.factoryQueue(ctx, factory) + case <-ctx.Done(): + return factoryQueueRead{err: ctx.Err()} + } + } + // Complete ownership even when the guarded reader panics. Only a completed + // read replaces the unavailable entry, so a fault cannot publish success. + entry = factoryQueueRead{err: errors.New("the Factory queue read did not complete")} + defer func() { + entry.at = b.now() + state.factoryQueueFinish(factory, flight, entry) + }() + read := state.queue + if read == nil { + read = b.FactoryQueue + } + queue, err := read(ctx, factory) + jobs := map[int]api.Job{} + for _, j := range queue.Jobs { + jobs[j.ID] = j + } + entry = factoryQueueRead{err: err, jobs: jobs} + return entry +} + +func (state *collectionFactory) factoryQueueStart(factory string, now time.Time) (factoryQueueRead, chan struct{}, bool) { + state.cacheMu.Lock() + defer state.cacheMu.Unlock() + if entry, ok := state.cache[factory]; ok && now.Sub(entry.at) < factoryQueueFresh { + return entry, nil, false + } + if flight := state.flights[factory]; flight != nil { + return factoryQueueRead{}, flight, false + } + if state.flights == nil { + state.flights = map[string]chan struct{}{} + } + flight := make(chan struct{}) + state.flights[factory] = flight + return factoryQueueRead{}, flight, true +} + +func (state *collectionFactory) factoryQueueFinish(factory string, flight chan struct{}, entry factoryQueueRead) { + state.cacheMu.Lock() + defer state.cacheMu.Unlock() + if state.cache == nil { + state.cache = map[string]factoryQueueRead{} + } + if len(state.cache) >= factoryQueueCacheMax { + oldest := "" + var at time.Time + for id, v := range state.cache { + if oldest == "" || v.at.Before(at) { + oldest, at = id, v.at + } + } + delete(state.cache, oldest) + } + state.cache[factory] = entry + delete(state.flights, factory) + close(flight) +} + +type factoryJobReading struct { + job api.Job + err error + notFound bool +} + +// factoryJobReads gives each factory one concurrent queue read. Missing +// jobs share a global fallback allowance and the same overall deadline. +func (b *appBackend) factoryJobReads(jobs []factoryJob) map[factoryJob]factoryJobReading { + out := map[factoryJob]factoryJobReading{} + grouped := map[string][]factoryJob{} + for _, j := range jobs { + if _, seen := out[j]; !seen { + out[j] = factoryJobReading{err: context.DeadlineExceeded} + grouped[j.Factory] = append(grouped[j.Factory], j) + } + } + if len(grouped) == 0 { + return out + } + ctx, cancel := context.WithTimeout(context.Background(), factoryReadTimeout) + defer cancel() + var wg sync.WaitGroup + var mu sync.Mutex + missing := 0 + factoryMissingAllowed := func() bool { + mu.Lock() + defer mu.Unlock() + if missing >= factoryMissingJobsMax { + return false + } + missing++ + return true + } + factoryRememberRead := func(j factoryJob, read factoryJobReading) { + mu.Lock() + defer mu.Unlock() + out[j] = read + } + slots := make(chan struct{}, factoryReadParallel) + for factory, members := range grouped { + wg.Add(1) + guard.Go("app/collection-factory", func() { + defer wg.Done() + select { + case slots <- struct{}{}: + defer func() { <-slots }() + case <-ctx.Done(): + return + } + queue := b.factoryQueue(ctx, factory) + for _, j := range members { + job, ok := queue.jobs[j.Number] + err := queue.err + if err == nil && !ok { + allowed := factoryMissingAllowed() + switch { + case !allowed: + err = errors.New("the missing-job read allowance was reached") + case ctx.Err() != nil: + err = ctx.Err() + default: + read := b.collectionFactory.job + if read == nil { + read = b.FactoryJob + } + job, err = read(ctx, j.Factory, j.Number) + if err == nil && job.ID != j.Number { + err = errors.New("the Factory returned another job") + } + } + } + var wire *appwire.Error + notFound := queue.err == nil && errors.As(err, &wire) && wire.Code == appwire.CodeNotFound + factoryRememberRead(j, factoryJobReading{job: job, err: err, notFound: notFound}) + } + }) + } + wg.Wait() + return out +} + +// Collection resolution uses the same bounded batch reader, with each job +// asked once even when it appears through several nested collections. +func (r *collectionResolver) prepareFactoryRefs(members []workspace.Member) { + jobs := []factoryJob{} + refs := map[factoryJob]workspace.Ref{} + for _, member := range members { + if member.Kind != workspace.FactoryJobKind { + continue + } + number, _ := strconv.Atoi(member.ID) + job := factoryJob{Factory: member.Factory, Number: number} + jobs = append(jobs, job) + refs[job] = member.Ref + } + r.factoryTargets = map[workspace.Ref]appwire.Target{} + for job, read := range r.b.factoryJobReads(jobs) { + var target appwire.Target + if read.err != nil { + if read.notFound { + target = appwire.Target{State: appwire.TargetDeleted} + } else { + target = appUnknown("the Factory", read.err) + target.Status = "unknown" + } + } else { + target = appwire.Target{State: appwire.TargetLive, Title: read.job.Title, Status: read.job.State, Repo: read.job.Repo} + } + r.factoryTargets[refs[job]] = target + } +} + +func (r *collectionResolver) prepareFactoryTree(level workspace.Level) { + members := []workspace.Member{} + var collect func(workspace.Level) + collect = func(level workspace.Level) { + for _, node := range level.Members { + if node.Kind == workspace.FactoryJobKind { + members = append(members, node.Member) + } + if node.Children != nil { + collect(*node.Children) + } + } + } + collect(level) + r.prepareFactoryRefs(members) +} diff --git a/cmd/codeaf/appcollections_factory_test.go b/cmd/codeaf/appcollections_factory_test.go new file mode 100644 index 0000000000..0530e2c7e7 --- /dev/null +++ b/cmd/codeaf/appcollections_factory_test.go @@ -0,0 +1,319 @@ +package main + +import ( + "context" + "errors" + "fmt" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/factory/api" + "github.com/Agent-Field/codeaf/internal/guard" +) + +func collectionFactoryRef(factory string, job int) appwire.Member { + return appwire.Member{Ref: appwire.Ref{Kind: appwire.RefFactoryJob, ID: fmt.Sprint(job), Factory: factory}} +} + +func collectionFactoryCreate(t *testing.T, b *appBackend, name string, refs ...appwire.Member) string { + t.Helper() + c, err := b.CreateCollection(name) + if err != nil { + t.Fatal(err) + } + if len(refs) > 0 { + if _, err := b.AddToCollection(c.ID, refs); err != nil { + t.Fatal(err) + } + } + return c.ID +} + +// Contract (b), G1: the moved batch reader resolves nested refs once per job, +// reads distinct factories concurrently, and shares cache entries with expand. +func TestCollectionFactoryBatchCacheAndNestedTargets(t *testing.T) { + b, _, _, _ := collectionsBackend(t) + nested := collectionFactoryCreate(t, b, "Nested", collectionFactoryRef("f", 7), collectionFactoryRef("g", 9)) + root := collectionFactoryCreate(t, b, "Root", collectionFactoryRef("f", 7), appwire.Member{Ref: appwire.Ref{Kind: appwire.RefCollection, ID: nested}}) + entered := make(chan string, 2) + release := make(chan struct{}) + var releaseOnce sync.Once + defer releaseOnce.Do(func() { close(release) }) + var mu sync.Mutex + calls := map[string]int{} + b.collectionFactory.queue = func(ctx context.Context, factory string) (api.Queue, error) { + func() { mu.Lock(); defer mu.Unlock(); calls[factory]++ }() + entered <- factory + select { + case <-release: + case <-ctx.Done(): + return api.Queue{}, ctx.Err() + } + id := 7 + if factory == "g" { + id = 9 + } + return api.Queue{Jobs: []api.Job{{ID: id, Num: 1107, Title: "Recorded title", State: "backlog", Repo: "acme/api"}}}, nil + } + done := make(chan error, 1) + guard.Go("app/collection-factory-open-test", func() { + open, err := b.OpenCollection(root, 3, 200, "") + if err == nil { + var visit func([]appwire.Node) + found := 0 + visit = func(nodes []appwire.Node) { + for _, node := range nodes { + if node.Kind == appwire.RefFactoryJob { + found++ + if node.Target.State != appwire.TargetLive || node.Target.Title != "Recorded title" || node.Target.Status != "backlog" || node.Target.Repo != "acme/api" { + t.Errorf("target: %+v", node.Target) + } + } + if node.Children != nil { + visit(node.Children.Members) + } + } + } + visit(open.Members) + if found != 3 { + t.Errorf("resolved %d job refs, want 3", found) + } + } + done <- err + }) + for range 2 { + select { + case <-entered: + case <-time.After(time.Second): + t.Fatal("factories were not read concurrently") + } + } + releaseOnce.Do(func() { close(release) }) + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("open did not finish") + } + if _, err := b.ExpandCollection(root, 3, 200); err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + if calls["f"] != 1 || calls["g"] != 1 { + t.Fatalf("queue calls: %v", calls) + } +} + +// Contract (b), G1: missing-job reads have a shared deadline and global bound; +// duplicate refs are asked once and unavailable queues never trigger fallbacks. +func TestCollectionFactoryMissingJobReadsAreBounded(t *testing.T) { + b, _, _, _ := collectionsBackend(t) + refs := []appwire.Member{} + for i := 1; i <= 25; i++ { + refs = append(refs, collectionFactoryRef("f", i)) + } + id := collectionFactoryCreate(t, b, "Missing", refs...) + queueReads, jobReads := 0, 0 + b.collectionFactory.queue = func(ctx context.Context, _ string) (api.Queue, error) { + queueReads++ + if deadline, ok := ctx.Deadline(); !ok || time.Until(deadline) > factoryReadTimeout { + t.Error("queue read has no bounded deadline") + } + return api.Queue{}, nil + } + b.collectionFactory.job = func(ctx context.Context, _ string, job int) (api.Job, error) { + jobReads++ + if _, ok := ctx.Deadline(); !ok { + t.Error("job read has no deadline") + } + return api.Job{ID: job, Num: 999, State: "backlog"}, nil + } + open, err := b.OpenCollection(id, 1, 200, "") + live, unknown := 0, 0 + for _, node := range open.Members { + switch node.Target.State { + case appwire.TargetLive: + live++ + case appwire.TargetUnknown: + unknown++ + } + } + if err != nil || queueReads != 1 || jobReads != 20 || live != 20 || unknown != 5 { + t.Fatalf("bounded fallback: %+v, %v; queue %d jobs %d", open, err, queueReads, jobReads) + } + b.collectionFactory.cache = nil + jobReads = 0 + b.collectionFactory.queue = func(context.Context, string) (api.Queue, error) { return api.Queue{}, errors.New("unreachable") } + open, err = b.OpenCollection(id, 1, 200, "") + if err != nil || jobReads != 0 { + t.Fatalf("unavailable queue triggered fallbacks: %v, %d", err, jobReads) + } + for _, node := range open.Members { + if node.Target.State != appwire.TargetUnknown || node.Target.Status != "unknown" { + t.Fatalf("unavailable target: %+v", node.Target) + } + } + b.collectionFactory.cache = nil + b.collectionFactory.queue = func(context.Context, string) (api.Queue, error) { return api.Queue{}, nil } + jobReads = 0 + reads := b.factoryJobReads([]factoryJob{{Factory: "f", Number: 7}, {Factory: "f", Number: 7}}) + if len(reads) != 1 || jobReads != 1 { + t.Fatalf("duplicate job read %d times: %v", jobReads, reads) + } +} + +// Contract (b), G1: a missing job is deleted, a queue error or a wrong returned +// job is unknown, and a short cache shares both successes and failures. +func TestCollectionFactoryDeletedUnknownAndCacheExpiry(t *testing.T) { + b, _, _, _ := collectionsBackend(t) + id := collectionFactoryCreate(t, b, "Targets", collectionFactoryRef("f", 7)) + clock := time.Now() + b.now = func() time.Time { return clock } + calls := 0 + b.collectionFactory.queue = func(context.Context, string) (api.Queue, error) { calls++; return api.Queue{}, nil } + b.collectionFactory.job = func(context.Context, string, int) (api.Job, error) { + return api.Job{}, appwire.Errorf(appwire.CodeNotFound, "gone") + } + assertTarget := func(state string) { + t.Helper() + open, err := b.OpenCollection(id, 1, 200, "") + if err != nil || len(open.Members) != 1 || open.Members[0].Target.State != state { + t.Fatalf("target: %+v, %v", open, err) + } + } + assertTarget(appwire.TargetDeleted) + b.collectionFactory.job = func(context.Context, string, int) (api.Job, error) { return api.Job{ID: 8}, nil } + assertTarget(appwire.TargetUnknown) + if calls != 1 { + t.Fatalf("queue was not shared: %d", calls) + } + clock = clock.Add(factoryQueueFresh) + b.collectionFactory.queue = func(context.Context, string) (api.Queue, error) { + calls++ + return api.Queue{}, errors.New("unreachable") + } + assertTarget(appwire.TargetUnknown) + assertTarget(appwire.TargetUnknown) + if calls != 2 { + t.Fatalf("failed queue cache: %d", calls) + } + clock = clock.Add(factoryQueueFresh) + b.collectionFactory.queue = func(context.Context, string) (api.Queue, error) { + calls++ + return api.Queue{Jobs: []api.Job{{ID: 7, Title: "Back", State: "running"}}}, nil + } + assertTarget(appwire.TargetLive) + if calls != 3 { + t.Fatalf("queue did not retry after expiry: %d", calls) + } +} + +// factoryQueueWaitContext signals when a coalesced reader selects on Done, +// so the panic cannot race ahead of the waiter entering the flight branch. +type factoryQueueWaitContext struct { + context.Context + waiting chan struct{} + once sync.Once +} + +func (c *factoryQueueWaitContext) Done() <-chan struct{} { + c.once.Do(func() { close(c.waiting) }) + return c.Context.Done() +} + +// Contract (b), G1: the moved guarded reader releases its flight on panic, +// caches the unavailable result and permits a successful retry after expiry. +func TestCollectionFactoryPanicFinishesFlight(t *testing.T) { + b, _, _, _ := collectionsBackend(t) + id := collectionFactoryCreate(t, b, "Panic", collectionFactoryRef("f", 7)) + var clock atomic.Int64 + clock.Store(time.Now().UnixMilli()) + b.now = func() time.Time { return time.UnixMilli(clock.Load()) } + entered, release := make(chan struct{}), make(chan struct{}) + var releaseOnce sync.Once + defer releaseOnce.Do(func() { close(release) }) + var calls atomic.Int32 + b.collectionFactory.queue = func(ctx context.Context, _ string) (api.Queue, error) { + if calls.Add(1) == 1 { + close(entered) + select { + case <-release: + panic("queue reader fault") + case <-ctx.Done(): + return api.Queue{}, ctx.Err() + } + } + return api.Queue{Jobs: []api.Job{{ID: 7, State: "backlog"}}}, nil + } + b.collectionFactory.job = func(context.Context, string, int) (api.Job, error) { + t.Error("failed queue triggered fallback") + return api.Job{}, errors.New("unexpected fallback") + } + owner := make(chan appwire.CollectionOpenResult, 1) + guard.Go("app/collection-factory-panic-owner-test", func() { + open, err := b.OpenCollection(id, 1, 200, "") + if err != nil { + t.Error(err) + } + owner <- open + }) + select { + case <-entered: + case <-time.After(time.Second): + t.Fatal("reader did not enter") + } + _, flight, owns := b.collectionFactory.factoryQueueStart("f", b.now()) + if flight == nil || owns { + t.Fatal("reader did not own a shared flight") + } + ctx, cancel := context.WithTimeout(context.Background(), factoryReadTimeout) + defer cancel() + waitCtx := &factoryQueueWaitContext{Context: ctx, waiting: make(chan struct{})} + waiter := make(chan factoryQueueRead, 1) + guard.Go("app/collection-factory-panic-waiter-test", func() { waiter <- b.factoryQueue(waitCtx, "f") }) + select { + case <-waitCtx.waiting: + case <-time.After(time.Second): + t.Fatal("waiter did not enter flight") + } + releaseOnce.Do(func() { close(release) }) + select { + case open := <-owner: + if len(open.Members) != 1 || open.Members[0].Target.State != appwire.TargetUnknown { + t.Fatalf("panic target: %+v", open) + } + case <-time.After(time.Second): + t.Fatal("owner did not finish") + } + select { + case <-flight: + default: + t.Fatal("panic left flight open") + } + select { + case read := <-waiter: + if read.err == nil || errors.Is(read.err, context.DeadlineExceeded) || !read.at.Equal(b.now()) { + t.Fatalf("cached failure: %+v", read) + } + case <-time.After(time.Second): + t.Fatal("panic stranded waiter") + } + entry, flight, owns := b.collectionFactory.factoryQueueStart("f", b.now()) + if entry.err == nil || flight != nil || owns || calls.Load() != 1 { + t.Fatal("panic did not cache failure and release flight") + } + clock.Add((factoryQueueFresh - time.Millisecond).Milliseconds()) + if got := b.factoryQueue(ctx, "f"); got.err == nil || calls.Load() != 1 { + t.Fatalf("failure not cached: %+v", got) + } + clock.Add(1) + if got := b.factoryQueue(ctx, "f"); got.err != nil || got.jobs[7].ID != 7 || calls.Load() != 2 { + t.Fatalf("retry after expiry: %+v", got) + } +} diff --git a/cmd/codeaf/apptools.go b/cmd/codeaf/apptools.go index 430092b0f4..9ef7f9dd25 100644 --- a/cmd/codeaf/apptools.go +++ b/cmd/codeaf/apptools.go @@ -133,11 +133,15 @@ func appFileName(id string) string { // appToolKind is a call's kind, read off the engine's own family for the // tool and the account it acts through, never a list kept here. -func appToolKind(tool, service string) string { +func appToolKind(tool, service string, declared ...session.ActionCategory) string { if service != "" { return appwire.ToolConnector } - switch session.ActionCategoryForTool(tool) { + category := session.ActionCategoryForTool(tool) + if len(declared) > 0 && declared[0] != "" { + category = declared[0] + } + switch category { case session.ActionSearch: if strings.HasPrefix(tool, "web_") { return appwire.ToolWeb @@ -198,6 +202,7 @@ func (b *appBackend) newTurnCalls(a *appAgent) { // callBegan records a call that began and answers its row. func (b *appBackend) callBegan(a *appAgent, agent string, turn uint64, event session.Event, title string) appwire.TurnTool { + category := b.clientToolEventCategory(a, event) log := b.callLog(a) var workspace string b.locked(func() { workspace = a.view.Workspace }) @@ -227,7 +232,7 @@ func (b *appBackend) callBegan(a *appAgent, agent string, turn uint64, event ses } c.run = event.RunID c.tool, c.args, c.hint, c.title, c.turn = event.Tool, event.Args, event.Hint, title, turn - kind := appToolKind(event.Tool, event.Service) + kind := appToolKind(event.Tool, event.Service, category) if c.facts.Ask != "" { kind = appwire.ToolAsksFirst } @@ -389,6 +394,7 @@ func (b *appBackend) callFinished(a *appAgent, event session.Event) { // callEnded records a call's end and answers its row. func (b *appBackend) callEnded(a *appAgent, agent string, turn uint64, event session.Event, title, summary, detail string, ok bool) appwire.TurnTool { + category := b.clientToolEventCategory(a, event) log := b.callLog(a) process := "" if ok && !event.HarnessMade && !event.Refused && !event.RejectedBeforeRun { @@ -399,7 +405,7 @@ func (b *appBackend) callEnded(a *appAgent, agent string, turn uint64, event ses c := log.calls[event.CallID] if c == nil { c = &appCall{id: event.CallID, tool: event.Tool, args: event.Args, turn: turn} - c.facts.Kind = appToolKind(event.Tool, "") + c.facts.Kind = appToolKind(event.Tool, "", category) if event.CallID != "" { log.calls[event.CallID] = c log.order = append(log.order, event.CallID) @@ -627,7 +633,7 @@ func (b *appBackend) appEntryFacts(a *appAgent, item session.DisplayEntry) appwi facts.MediaAttempt = item.ToolResult.MediaAttempt() var workspace string b.locked(func() { workspace = a.view.Workspace }) - facts.Kind = appToolKind(item.Tool, "") + facts.Kind = appToolKind(item.Tool, "", b.clientToolEventCategory(a, session.Event{Tool: item.Tool})) facts.Input = appToolInput(facts.Kind, item.Tool, item.Args, item.Hint, workspace) if path, adds, dels, ok := tui3.ToolFileChange(item.Tool, item.Args); ok { facts.Files = []appwire.FileChange{{Path: path, Add: adds, Del: dels}} diff --git a/cmd/codeaf/chatv3_layout.go b/cmd/codeaf/chatv3_layout.go index d447661ee1..81ef673d68 100644 --- a/cmd/codeaf/chatv3_layout.go +++ b/cmd/codeaf/chatv3_layout.go @@ -677,8 +677,9 @@ func v3ScanBucket(bucket string) (spoken, empty []v3Folder) { } } if v3EmptyUnassignedSession(dir) { - // A team's reference keeps it, and so does a store that cannot say. - if referenced, err := teams.ConversationReferenced(config.ProfileDir(), place.Transcript()); err == nil && !referenced { + // A team or collection reference keeps a blank out of both piles. + // So does a store that cannot say whether it is referenced. + if referenced, err := teams.ConversationReferenced(config.ProfileDir(), place.Transcript()); err == nil && !referenced && !collectionKeepsConversation(dir) { empty = append(empty, folder) } continue @@ -710,14 +711,17 @@ func v3ScanBucket(bucket string) (spoken, empty []v3Folder) { // A team reference makes an empty conversation intentional, not reusable // litter. Only the teams that name this conversation are read (the store's // index), and a store that cannot say keeps the folder. +// A collection reference keeps it too, including when its store cannot be read. func v3EmptySession(dir string) bool { referenced, err := teams.ConversationReferenced(config.ProfileDir(), session.Place{Dir: dir}.Transcript()) if err != nil || referenced { return false } - return v3EmptyUnassignedSession(dir) + return v3EmptyUnassignedSession(dir) && !collectionKeepsConversation(dir) } +// v3EmptyUnassignedSession reads only content. References decide whether a blank +// is kept, never whether it is a conversation somebody has spoken in. func v3EmptyUnassignedSession(dir string) bool { place := session.Place{Dir: dir, Owned: true} spoken, sure := session.SpokeIn(place.Transcript()) @@ -751,7 +755,7 @@ func v3ReapEmpty(empty []v3Folder, keep string) { } transcript := session.Place{Dir: folder.dir}.Transcript() _ = teams.InspectConversation(config.ProfileDir(), transcript, func(referenced bool) error { - if !referenced && !session.InUse(transcript) && v3EmptyUnassignedSession(folder.dir) { + if !referenced && !session.InUse(transcript) && v3EmptyUnassignedSession(folder.dir) && !collectionKeepsConversation(folder.dir) { _ = os.RemoveAll(folder.dir) } return nil diff --git a/cmd/codeaf/chatv3_sweep.go b/cmd/codeaf/chatv3_sweep.go index 0c38d8c1d6..f204396515 100644 --- a/cmd/codeaf/chatv3_sweep.go +++ b/cmd/codeaf/chatv3_sweep.go @@ -42,7 +42,7 @@ const sweepLogName = "sweep.log" // directory to walk so that the rule can be pointed at a temp directory and // proved. [v3ErrandsRoot] is the one answer to where that is, and // [errandHeldByAutomation] keeps the errands an automation was saved from. -var sweepHome func(context.Context, string, func(string) bool, func(string)) = session.SweepHomeContext +var sweepHome func(context.Context, string, func(string) bool, func(string) bool, func(string)) = session.SweepHomeContext func (p *v3Process) startPlaceSweep() { ctx, cancel := context.WithCancel(context.Background()) @@ -51,7 +51,7 @@ func (p *v3Process) startPlaceSweep() { p.sweepDone = done guard.Go("chatv3/sweep-home", func() { defer close(done) - sweepHome(ctx, v3ErrandsRoot(), errandHeldByAutomation(), func(line string) { + sweepHome(ctx, v3ErrandsRoot(), errandHeldByAutomation(), collectionKeepsConversation, func(line string) { if ctx.Err() == nil { noteSweep(line) } diff --git a/cmd/codeaf/chatv3_sweep_test.go b/cmd/codeaf/chatv3_sweep_test.go index d4b6f6ee8f..e84a0e580b 100644 --- a/cmd/codeaf/chatv3_sweep_test.go +++ b/cmd/codeaf/chatv3_sweep_test.go @@ -28,7 +28,7 @@ func TestCloseJoinsPlaceSweepAndNextProcessStillSweeps(t *testing.T) { t.Cleanup(func() { sweepHome = previousSweep }) var starts atomic.Int32 var destructive atomic.Int32 - sweepHome = func(ctx context.Context, _ string, _ func(string) bool, _ func(string)) { + sweepHome = func(ctx context.Context, _ string, _, _ func(string) bool, _ func(string)) { i := int(starts.Add(1)) - 1 if i >= len(passes) { t.Errorf("unexpected sweep start %d", i+1) diff --git a/cmd/codeaf/collections.go b/cmd/codeaf/collections.go index 70ed21b4e0..f0afef48ba 100644 --- a/cmd/codeaf/collections.go +++ b/cmd/codeaf/collections.go @@ -22,8 +22,8 @@ const collectionsUsage = collectionsSummary + ` codeaf collections show codeaf collections add|remove codeaf collections find - kinds: collection, conversation, task, process, crew, artifact (a file path) - tasks and processes need --session ; all accept --db and --json + kinds: collection, conversation, task, process, crew, factory_job, artifact (a file path) + factory_job needs --factory ; tasks and processes need --session ; all accept --db and --json organize references without moving files or starting work` func runCollections(args []string) error { return runCollectionsTo(args, os.Stdout) } @@ -32,6 +32,7 @@ func runCollectionsTo(args []string, output io.Writer) error { flags := commandFlags("collections") database := flags.String("db", home.Join("v3", "collections.db"), "collection database path; separate from learned memory") sessionID := flags.String("session", "", "owning conversation id for a task reference") + factoryID := flags.String("factory", "", "owning factory id for a factory_job reference") asJSON := flags.Bool("json", false, "print structured records") if err := parseCommandFlags(flags, reorder(flags, args)); err != nil { return err @@ -63,7 +64,7 @@ func runCollectionsTo(args []string, output io.Writer) error { } var ref workspace.Ref if verb == "add" || verb == "remove" || verb == "find" { - ref = workspace.Ref{Kind: workspace.Kind(rest[len(rest)-2]), ID: rest[len(rest)-1], SessionID: *sessionID} + ref = workspace.Ref{Kind: workspace.Kind(rest[len(rest)-2]), ID: rest[len(rest)-1], SessionID: *sessionID, Factory: *factoryID} if ref.Kind == workspace.ArtifactKind { if ref.ID == "" { return fmt.Errorf("%w: an artifact reference needs a file path", workspace.ErrInvalid) @@ -86,6 +87,8 @@ func runCollectionsTo(args []string, output io.Writer) error { if verb == "add" && ref.Kind == workspace.StandingKind { return fmt.Errorf("%w: standing items are gone, so a collection cannot gain one", workspace.ErrInvalid) } + } else if *factoryID != "" { + return fmt.Errorf("%w: --factory belongs to a factory_job reference in add, remove or find", workspace.ErrInvalid) } else if *sessionID != "" { return fmt.Errorf("%w: --session belongs to a task or process reference in add, remove or find", workspace.ErrInvalid) } @@ -205,6 +208,8 @@ func writeCollectionRef(output io.Writer, ref workspace.Ref) error { var err error if ref.Kind == workspace.TaskKind || ref.Kind == workspace.ProcessKind { _, err = fmt.Fprintf(output, "%s %s conversation %s\n", ref.Kind, ref.ID, ref.SessionID) + } else if ref.Kind == workspace.FactoryJobKind { + _, err = fmt.Fprintf(output, "%s %s factory %s\n", ref.Kind, ref.ID, ref.Factory) } else { _, err = fmt.Fprintf(output, "%s %s\n", ref.Kind, ref.ID) } diff --git a/cmd/codeaf/collections_factory_job_test.go b/cmd/codeaf/collections_factory_job_test.go new file mode 100644 index 0000000000..233234d8a0 --- /dev/null +++ b/cmd/codeaf/collections_factory_job_test.go @@ -0,0 +1,53 @@ +package main + +import ( + "bytes" + "encoding/json" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/appwire" + "github.com/Agent-Field/codeaf/internal/workspace" +) + +// Part A (A2): add/remove/find accept --factory, show names it readably, and +// omitting or misusing the flag is refused before opening storage. +func TestPartACollectionsCLIFactoryJob(t *testing.T) { + db := filepath.Join(t.TempDir(), "collections.db") + c := collectionCreate(t, db, "Jobs") + var out bytes.Buffer + if err := runCollectionsTo([]string{"add", c.ID, "factory_job", "7", "--db", db}, &out); err == nil || !strings.Contains(err.Error(), "factory") { + t.Fatalf("missing factory: %v", err) + } + collectionCommand(t, db, "add", c.ID, "factory_job", "7", "--factory", "f") + if err := runCollectionsTo([]string{"show", c.ID, "--db", db}, &out); err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), "factory_job 7 factory f") { + t.Fatalf("show: %q", out.String()) + } + var refs []workspace.Ref + if err := json.Unmarshal([]byte(collectionCommand(t, db, "show", c.ID)), &refs); err != nil || len(refs) != 1 || refs[0].Factory != "f" { + t.Fatalf("refs: %+v %v", refs, err) + } + var parents []workspace.Collection + if err := json.Unmarshal([]byte(collectionCommand(t, db, "find", "factory_job", "7", "--factory", "f")), &parents); err != nil || len(parents) != 1 || parents[0].ID != c.ID { + t.Fatalf("find: %+v %v", parents, err) + } + collectionCommand(t, db, "remove", c.ID, "factory_job", "7", "--factory", "f") + if got := collectionCommand(t, db, "show", c.ID); got != "[]\n" { + t.Fatalf("remove: %s", got) + } + out.Reset() + if err := runCollectionsTo([]string{"add", c.ID, "conversation", "chat", "--factory", "f", "--db", db}, &out); err == nil { + t.Fatal("factory on conversation accepted") + } +} + +func TestPartACollectionRefConversionsRetainFactory(t *testing.T) { + r := appwire.Ref{Kind: appwire.RefFactoryJob, ID: "7", Factory: "f"} + if got := appWireRef(appStoreRef(r)); got != r { + t.Fatalf("round trip: %+v", got) + } +} diff --git a/cmd/codeaf/engine.go b/cmd/codeaf/engine.go index 5e126b436c..c13491c837 100644 --- a/cmd/codeaf/engine.go +++ b/cmd/codeaf/engine.go @@ -1169,6 +1169,7 @@ func assembleEngine(hello remote.Hello, workspace, sessionFlag, launchDir string // Harnesses and RunHarness, properties of the machine that the shared // assembly fills for every door. cfg, open := v3Shape(cfg, v3LanesOverWire()) + open = appClientToolsOpener(open) // AND AUTOMATIONS ARE ON, which is the one capability on this list that a // connection does not take away. The store arrives already filled, from the diff --git a/docs/changes/unreleased/2296-codeaf-leg-finishes-in-ci.md b/docs/changes/unreleased/2296-codeaf-leg-finishes-in-ci.md new file mode 100644 index 0000000000..f99f06de54 --- /dev/null +++ b/docs/changes/unreleased/2296-codeaf-leg-finishes-in-ci.md @@ -0,0 +1,11 @@ +--- +kind: internal +title: The cmd/codeaf touched leg gets a 25-minute timeout +pr: 2296 +surface: [build] +invalidates: [] +--- + +The package's serial suite measured 17.8 minutes on the CI runner, over the +15-minute timeout, so tests after the cut went unchecked. Only the codeaf leg +changes; the other legs keep 15 minutes. diff --git a/docs/changes/unreleased/2336-app-generic-collections-and-client-tools.md b/docs/changes/unreleased/2336-app-generic-collections-and-client-tools.md new file mode 100644 index 0000000000..ea69b436b4 --- /dev/null +++ b/docs/changes/unreleased/2336-app-generic-collections-and-client-tools.md @@ -0,0 +1,29 @@ +--- +kind: added +title: Collections hold Factory jobs and keep blank conversations, and apps attach client tools +pr: 2336 +surface: [app, chat] +invalidates: + - The collections database becomes schema 7 on its first successful write; an engine built before this change refuses it, leaving collections unavailable there and data intact. + - A never-spoken conversation was removed at launch unless a team referenced it; a conversation any collection references is now kept as well. +--- + +**Factory jobs in collections.** A membership can name a Factory job: kind +`factory_job` with its `factory`. `codeaf collections add|remove|find` take +`--factory`. In the app, `collections.open` shows each job's title, status and +repository from one bounded, briefly shared read per Factory; a job that is gone +or unreadable keeps its row and says so. One job may sit in several collections. + +**Client tools.** An app attaches sets of tools to one conversation +(`agents.tools_set`, hello feature `agents.client_tools`). They follow the +built-in tools while that connection lasts. The engine sends every call as an +`agents.tool_call` event and waits for `agents.tool_result`; a lost connection, +the turn ending, or the deadline (default 30 s, ceiling 120 s) gives the model +an error saying the outcome is unknown, and a late or second answer is dropped. +Limits: 4 sets, 64 tools, no built-in names, never inherited by child tasks. +Remote-held conversations are refused for now. + +**Kept conversations.** Every cleanup path (launch, blank-chat sweep, background +sweep) now spares a never-spoken conversation that some collection holds, and an +unreadable collections file counts as holding it. After a restart, opening it +by id brings back its saved details. diff --git a/docs/changes/unreleased/2337-held-blank-conversations-never-resumed.md b/docs/changes/unreleased/2337-held-blank-conversations-never-resumed.md new file mode 100644 index 0000000000..3d745df9fc --- /dev/null +++ b/docs/changes/unreleased/2337-held-blank-conversations-never-resumed.md @@ -0,0 +1,12 @@ +--- +kind: fixed +title: A blank conversation a collection holds is never resumed and reads as live in its collection +pr: 2337 +surface: [app, chat] +invalidates: [] +--- + +A never-spoken conversation that a collection references is kept, but a launch +without an explicit session no longer resumes it ahead of the last spoken +conversation, and `collections.open` no longer reports it as deleted after a +restart. diff --git a/docs/changes/unreleased/2346-client-tool-source-message.md b/docs/changes/unreleased/2346-client-tool-source-message.md new file mode 100644 index 0000000000..2676eba4c7 --- /dev/null +++ b/docs/changes/unreleased/2346-client-tool-source-message.md @@ -0,0 +1,16 @@ +--- +kind: added +title: A client tool call names the message its turn answers +pr: 2346 +surface: [app, chat] +invalidates: [] +--- + +`agents.tool_call` carries an optional `source_message_id`: the durable id of +the message the running turn answers, the same string as `Entry.message_id` +on that message's chat entry. It is captured when the turn starts, so it is the +same after an engine restart and on a replayed call. A stopped turn asked again +carries its recorded message. Turns the engine starts itself, including a +carry-on after a quit or close, carry none. The hello feature +`agents.client_tools.source` is advertised with `agents.client_tools`. `turn` +remains the process-local counter. diff --git a/docs/design/app-v3/PROTOCOL.md b/docs/design/app-v3/PROTOCOL.md index 37cbed0702..dd57beb28c 100644 --- a/docs/design/app-v3/PROTOCOL.md +++ b/docs/design/app-v3/PROTOCOL.md @@ -79,6 +79,94 @@ decides which combinations are accepted. | `agents.stop` | `in.IDParams` | `Empty` | | `agents.suggest_reply` | `in.ReplySuggestionParams` | `ReplySuggestion` | +## Client tools + +The optional features `agents.client_tools` and `agents.client_tools.source` +appear together in `hello.features` exactly when the engine implements the +client-tools interface. The latter guarantees message identity on calls whenever +the running turn answers a message. Sets belong to this +stdio connection and one conversation. They are memory-only: register again +after reconnecting. Attachment does not open the conversation or start a turn. +Children, workers and clarification conversations inherit neither tools nor +instructions. Remote forwarding is unavailable. + +| method | params | result | +|---|---|---| +| `agents.tools_set` | `{agent, set, instructions?, deadline_ms?, tools?: [{name, description, input_schema, category}]}` | `{agent, set, tools: [name, ...]}` | +| `agents.tool_result` | `{call, result: }` or `{call, error: }` | `{accepted: true}` or `{accepted: false}` | + +`agents.tools_set` replaces that `agent` + `set` whole; other sets remain. +Omitted `tools` or `tools: []` removes only that set and returns `tools: []`. +An agent holds at most 4 sets, 32 tools per set and 64 tools total. All validation +is atomic: a refusal leaves every set unchanged. + +- `set` matches `^[a-z][a-z0-9_.-]{0,63}$`. +- `name` matches `^[a-z][a-z0-9_]{0,63}$`, unique within and across the agent's sets. +- `description` is 1–2048 bytes. `input_schema` is a JSON object whose `type` + is `"object"`, at most 16 KiB. Schema validation of call arguments is the app's job. +- `category` is `read`, `edit` or `create` and supplies this conversation's tool + category. Reading-only contexts refuse edits and creations. Ordinary tool + approval rules apply; attachment is not approval. +- `instructions` is at most 4096 bytes per set and may be empty. It appears in a + section headed `Tools from the app: `. A caller-supplied system page is + preserved and receives no section. Ordinary conversations gain no text. +- `deadline_ms` is 0 (the default 30000) or 1000–120000. + +Invalid fields, aggregate limits, duplicates and built-in tool names answer +`bad_params`; a collision message names the tool. Collision checks include the +live belt and all classified engine tools. A built-in arriving later wins and +the colliding client tool leaves the belt. No model request has duplicate names. +Unknown agents answer `not_found`. A remote-held conversation answers +`bad_scope`: `client tools need a conversation this engine runs`. +Tools and instructions update at the next step boundary, with client tools at +the tail after all built-ins. + +| event | data | +|---|---| +| `agents.tool_call` | `{agent, turn, call, turn_call, source_message_id?, set, name, args: , deadline_ms}` | + +`source_message_id` names the message the running turn answers: the same string +as `Entry.message_id` on that message's chat entry in `agents.read` (and in +`entry.added` when present). It is captured when the turn starts, never changed +by a mid-turn steer or note, and survives journal restarts and call replays. +It is absent when no message can be named exactly. A stopped answer reuses its +recorded message id. Turns the engine starts itself, including ordinary session +wakes, mixed batches and a carry-on after a quit or close, carry none. +With `agents.client_tools.source` advertised, +an absent field means no such message, rather than an older engine. + +One model call emits exactly one `agents.tool_call`. `turn` is the engine's +process-local conversation counter, which resets after restart; +`turn_call` matches `turn.tool.call`. `call` is unique +among pending calls and deterministic for the agent + provider call id. Without +a provider id the engine mints a fresh id. A concurrent redispatch joins the +pending wait and emits no second event. A settled redispatch returns the saved +answer without an event for 10 minutes, subject to a maximum of 1024 saved +answers per connection. Non-object model arguments become an invalid-arguments +error result without an event. + +`agents.tool_result` requires exactly one of `result` and `error`. `result` +accepts any JSON, including `null`, and reaches the model as compact JSON text, +cut at 64 KiB with a plain dropped-byte marker. `error` must be a nonempty string +of at most 4096 bytes and becomes an error tool result: +` could not do : `. Invalid answers are `bad_params`. +A pending answer returns `accepted: true`; unknown, late or duplicate ids return +`accepted: false` without an error or any other effect. + +Connection end (EOF, shutdown or context cancellation) disconnects the registry +before conversations close: all sets are dropped and all pending calls settle +immediately. These failures reach the model as error tool results, with no +engine retry: + +- A stale call after removal or disconnection: + `client_unavailable: the app that provides is not connected; nothing was done.` +- Disconnection during a call: + `client_unavailable: the app that provides disconnected before answering; the outcome is unknown, so check before trying again.` +- Deadline: + `client_timeout: the app that provides did not answer within s; the outcome is unknown, so check before trying again.` +- Turn cancellation: the turn ended before the app answered. The pending entry + is dropped immediately and later answers return `accepted: false`. + ## asks methods | method | params | result | @@ -147,6 +235,17 @@ decides which combinations are accepted. | `collections.remove` | `{id, refs}` | `CollectionRemoveResult` | | `collections.rename` | `in.collectionNameParams` | `CollectionResult` | +Collection refs include `factory_job`, with a canonical positive decimal job `id` +and a required `factory` field. The pair addresses the same job as `factory.job`. +Other ref kinds have no `factory`. Several collections can reference the same job. +Factory job targets carry the recorded title, state word and repository; an +unavailable Factory produces an `unknown` target and status. A missing job +produces a `deleted` target. Resolution uses one bounded batch across nested +collections: each distinct job is asked once, with a three-second deadline, +up to eight concurrent Factory queue reads and 20 missing-job fallbacks. Queue +reads, including unavailable results, share a ten-second cache of at most +32 factories. A failed queue read never triggers per-job fallbacks. + ## competence methods | method | params | result | diff --git a/docs/rules/ci.md b/docs/rules/ci.md index 26957c0922..0430a2e3d1 100644 --- a/docs/rules/ci.md +++ b/docs/rules/ci.md @@ -15,6 +15,9 @@ root-module packages. Module changes select the whole tree; changes without Go or module edits select no package leg. Tests run fresh with `-count=1` in separate chat, session, command and remaining-package jobs. Heavy packages are sharded from one compiled test binary, with a 15-minute timeout per shard. +`cmd/codeaf` runs unsharded in its own leg; its serial suite measured 17.8 +minutes on 2026-11-03, so that leg alone passes a 25-minute package timeout. +Drop this exception once the suite is back under 15 minutes. ## Local verification diff --git a/internal/appwire/client_tools.go b/internal/appwire/client_tools.go new file mode 100644 index 0000000000..a44a3fb508 --- /dev/null +++ b/internal/appwire/client_tools.go @@ -0,0 +1,162 @@ +package appwire + +import ( + "encoding/json" + "regexp" + + "github.com/Agent-Field/codeaf/internal/session" +) + +const FeatureClientTools = "agents.client_tools" +const FeatureClientToolsSource = "agents.client_tools.source" +const EventAgentsToolCall = "agents.tool_call" + +// ClientTooler is optional because only a connected app can answer these tools. +type ClientTooler interface { + SetClientTools(ClientToolsSetParams) (ClientToolsSetResult, error) + ClientToolResult(ClientToolResultParams) (ClientToolResultAck, error) +} + +type ClientToolsSetParams struct { + Agent string `json:"agent"` + Set string `json:"set"` + Instructions string `json:"instructions"` + DeadlineMS int `json:"deadline_ms"` + Tools []session.ClientTool `json:"tools"` +} + +type ClientToolsSetResult struct { + Agent string `json:"agent"` + Set string `json:"set"` + Tools []string `json:"tools"` +} + +type AgentsToolCall struct { + Agent string `json:"agent"` + Turn uint64 `json:"turn"` + Call string `json:"call"` + TurnCall string `json:"turn_call"` + SourceMessageID string `json:"source_message_id,omitempty"` + Set string `json:"set"` + Name string `json:"name"` + Args json.RawMessage `json:"args"` + DeadlineMS int `json:"deadline_ms"` +} + +type ClientToolResultParams struct { + Call string `json:"call"` + // RawMessage distinguishes an explicit JSON null result from an omitted one. + Result json.RawMessage `json:"result,omitempty"` + Error *string `json:"error,omitempty"` +} + +type ClientToolResultAck struct { + Accepted bool `json:"accepted"` +} + +func (p *ClientToolResultParams) UnmarshalJSON(data []byte) error { + type plain ClientToolResultParams + var decoded plain + if err := json.Unmarshal(data, &decoded); err != nil { + return err + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + return err + } + if _, present := fields["error"]; present && decoded.Error == nil { + return Errorf(CodeBadParams, "error must be a nonempty string") + } + *p = ClientToolResultParams(decoded) + return nil +} + +var clientToolSetName = regexp.MustCompile(`^[a-z][a-z0-9_.-]{0,63}$`) +var clientToolName = regexp.MustCompile(`^[a-z][a-z0-9_]{0,63}$`) + +// Validate rejects a whole replacement before the backend can change any set. +func (p ClientToolsSetParams) Validate() error { + if p.Agent == "" || !clientToolSetName.MatchString(p.Set) { + return Errorf(CodeBadParams, "agent and a valid set id are required") + } + if len(p.Tools) > session.ClientToolMaxPerSet || len(p.Instructions) > session.ClientToolInstructionsMax { + return Errorf(CodeBadParams, "a set allows at most %d tools and %d instruction bytes", session.ClientToolMaxPerSet, session.ClientToolInstructionsMax) + } + if p.DeadlineMS != 0 && (p.DeadlineMS < session.ClientToolMinDeadlineMS || p.DeadlineMS > session.ClientToolMaxDeadlineMS) { + return Errorf(CodeBadParams, "deadline_ms must be 0 or %d–%d", session.ClientToolMinDeadlineMS, session.ClientToolMaxDeadlineMS) + } + held := make(map[string]bool, len(p.Tools)) + for _, tool := range p.Tools { + if !clientToolName.MatchString(tool.Name) || held[tool.Name] { + return Errorf(CodeBadParams, "invalid or duplicate tool name %q", tool.Name) + } + held[tool.Name] = true + if err := validateClientTool(tool); err != nil { + return err + } + } + return nil +} + +func validateClientTool(tool session.ClientTool) error { + if len(tool.Description) == 0 || len(tool.Description) > session.ClientToolDescriptionMax { + return Errorf(CodeBadParams, "%s description must be 1–%d bytes", tool.Name, session.ClientToolDescriptionMax) + } + var object map[string]json.RawMessage + if len(tool.InputSchema) > session.ClientToolSchemaMax || json.Unmarshal(tool.InputSchema, &object) != nil || object == nil { + return Errorf(CodeBadParams, "%s input_schema must be an object of at most %d bytes", tool.Name, session.ClientToolSchemaMax) + } + var kind string + if json.Unmarshal(object["type"], &kind) != nil || kind != "object" { + return Errorf(CodeBadParams, "%s input_schema type must be object", tool.Name) + } + switch tool.Category { + case session.ActionRead, session.ActionEdit, session.ActionCreate: + return nil + } + return Errorf(CodeBadParams, "%s category must be read, edit or create", tool.Name) +} + +func (p ClientToolResultParams) Validate() error { + if (p.Result != nil) == (p.Error != nil) { + return Errorf(CodeBadParams, "exactly one of result or error is required") + } + if p.Error != nil && (len(*p.Error) == 0 || len(*p.Error) > session.ClientToolErrorMax) { + return Errorf(CodeBadParams, "error must be 1–%d bytes", session.ClientToolErrorMax) + } + if p.Result != nil && !json.Valid(p.Result) { + return Errorf(CodeBadParams, "result must be JSON") + } + return nil +} + +func withClientToolsFeature(hello Hello, engine Engine) Hello { + if _, ok := engine.(ClientTooler); ok { + return withFeature(withFeature(hello, FeatureClientTools), FeatureClientToolsSource) + } + return hello +} + +func setClientTools(e Engine, p ClientToolsSetParams) (*ClientToolsSetResult, error) { + door, ok := e.(ClientTooler) + if !ok { + return nil, Errorf(CodeUnknownMethod, "client tools are unavailable") + } + if err := p.Validate(); err != nil { + return nil, err + } + result, err := door.SetClientTools(p) + return &result, err +} + +func answerClientTool(e Engine, p ClientToolResultParams) (*ClientToolResultAck, error) { + door, ok := e.(ClientTooler) + if !ok { + return nil, Errorf(CodeUnknownMethod, "client tools are unavailable") + } + if err := p.Validate(); err != nil { + return nil, err + } + result, err := door.ClientToolResult(p) + return &result, err +} diff --git a/internal/appwire/client_tools_test.go b/internal/appwire/client_tools_test.go new file mode 100644 index 0000000000..70f2b779b8 --- /dev/null +++ b/internal/appwire/client_tools_test.go @@ -0,0 +1,209 @@ +package appwire + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/session" +) + +type clientToolsEngine struct { + sendEngine + sets []ClientToolsSetParams + answers []ClientToolResultParams +} + +func (e *clientToolsEngine) SetClientTools(p ClientToolsSetParams) (ClientToolsSetResult, error) { + e.sets = append(e.sets, p) + names := []string{} + for _, tool := range p.Tools { + names = append(names, tool.Name) + } + return ClientToolsSetResult{Agent: p.Agent, Set: p.Set, Tools: names}, nil +} +func (e *clientToolsEngine) ClientToolResult(p ClientToolResultParams) (ClientToolResultAck, error) { + e.answers = append(e.answers, p) + return ClientToolResultAck{Accepted: true}, nil +} + +func validClientToolsParams() ClientToolsSetParams { + return ClientToolsSetParams{Agent: "agent-1", Set: "app.notes", Tools: []session.ClientTool{{Name: "app_note", Description: "Read a note", + InputSchema: json.RawMessage(`{"type":"object"}`), Category: session.ActionRead}}} +} + +// Contract (f) and source contract (d): both features accompany the implemented door. +func TestClientToolsHelloFeature(t *testing.T) { + for _, test := range []struct { + engine Engine + want bool + }{{&sendEngine{}, false}, {&clientToolsEngine{}, true}} { + var out bytes.Buffer + server := NewServer(&out) + if err := server.Serve(context.Background(), strings.NewReader(""), Hello{}, test.engine); err != nil { + t.Fatal(err) + } + var frame struct { + Data Hello `json:"data"` + } + if err := json.Unmarshal(bytes.Split(out.Bytes(), []byte("\n"))[0], &frame); err != nil { + t.Fatal(err) + } + found, source := false, false + for _, feature := range frame.Data.Features { + found = found || feature == FeatureClientTools + source = source || feature == FeatureClientToolsSource + } + if found != test.want || source != test.want { + t.Fatalf("features=%v want client tools=%v", frame.Data.Features, test.want) + } + } +} + +// Source contract (d): an unknown message is omitted, and a known one is exact. +func TestClientToolCallSourceMessageJSON(t *testing.T) { + for _, source := range []string{"", "msg-recorded"} { + raw, err := json.Marshal(AgentsToolCall{SourceMessageID: source}) + if err != nil { + t.Fatal(err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(raw, &fields); err != nil { + t.Fatal(err) + } + value, present := fields["source_message_id"] + if present != (source != "") || present && string(value) != `"msg-recorded"` { + t.Fatalf("source %q encoded as %s", source, raw) + } + } +} + +// Source contract (d): the written feature guarantee and event shape match the wire. +func TestClientToolsSourceProtocol(t *testing.T) { + raw, err := os.ReadFile(protocolDoc) + if err != nil { + t.Fatal(err) + } + for _, phrase := range []string{FeatureClientToolsSource, "source_message_id?", "Entry.message_id", "process-local"} { + if !strings.Contains(string(raw), phrase) { + t.Errorf("protocol omits %q", phrase) + } + } +} + +// Contract (b), (h): both typed methods preserve their declared shapes. +func TestClientToolsTypedMethods(t *testing.T) { + e := &clientToolsEngine{} + raw, _ := json.Marshal(validClientToolsParams()) + result, err := methods["agents.tools_set"].call(e, raw) + if err != nil { + t.Fatal(err) + } + encoded, _ := json.Marshal(result) + if string(encoded) != `{"agent":"agent-1","set":"app.notes","tools":["app_note"]}` { + t.Fatal(string(encoded)) + } + for _, raw := range []string{`{"call":"ct_1","result":null}`, `{"call":"ct_1","result":[1,true]}`, `{"call":"ct_1","error":"no access"}`} { + result, err := methods["agents.tool_result"].call(e, json.RawMessage(raw)) + if err != nil { + t.Fatal(err) + } + encoded, _ := json.Marshal(result) + if string(encoded) != `{"accepted":true}` { + t.Fatal(string(encoded)) + } + } + if len(e.sets) != 1 || len(e.answers) != 3 || string(e.answers[0].Result) != "null" { + t.Fatal("typed arguments were lost") + } + if _, err := methods["agents.tools_set"].call(&sendEngine{}, raw); codeOf(err) != CodeUnknownMethod { + t.Fatal(err) + } +} + +// Contract (h): every scalar and per-set limit refuses before the optional engine. +func TestClientToolsValidationRefusals(t *testing.T) { + changes := map[string]func(*ClientToolsSetParams){ + "agent": func(p *ClientToolsSetParams) { p.Agent = "" }, + "set": func(p *ClientToolsSetParams) { p.Set = "Bad set" }, + "set length": func(p *ClientToolsSetParams) { p.Set = strings.Repeat("a", 65) }, + "name": func(p *ClientToolsSetParams) { p.Tools[0].Name = "Bad-name" }, + "name length": func(p *ClientToolsSetParams) { p.Tools[0].Name = strings.Repeat("a", 65) }, + "duplicate": func(p *ClientToolsSetParams) { p.Tools = append(p.Tools, p.Tools[0]) }, + "description empty": func(p *ClientToolsSetParams) { p.Tools[0].Description = "" }, + "description bytes": func(p *ClientToolsSetParams) { + p.Tools[0].Description = strings.Repeat("é", session.ClientToolDescriptionMax) + }, + "schema array": func(p *ClientToolsSetParams) { p.Tools[0].InputSchema = json.RawMessage(`[]`) }, + "schema null": func(p *ClientToolsSetParams) { p.Tools[0].InputSchema = json.RawMessage(`null`) }, + "schema type": func(p *ClientToolsSetParams) { p.Tools[0].InputSchema = json.RawMessage(`{"type":"array"}`) }, + "schema bytes": func(p *ClientToolsSetParams) { + p.Tools[0].InputSchema = json.RawMessage(`{"type":"object","padding":"` + strings.Repeat("x", session.ClientToolSchemaMax) + `"}`) + }, + "category": func(p *ClientToolsSetParams) { p.Tools[0].Category = session.ActionRun }, + "tools": func(p *ClientToolsSetParams) { p.Tools = make([]session.ClientTool, session.ClientToolMaxPerSet+1) }, + "instructions": func(p *ClientToolsSetParams) { + p.Instructions = strings.Repeat("x", session.ClientToolInstructionsMax+1) + }, + "negative deadline": func(p *ClientToolsSetParams) { p.DeadlineMS = -1 }, + "short deadline": func(p *ClientToolsSetParams) { p.DeadlineMS = session.ClientToolMinDeadlineMS - 1 }, + "long deadline": func(p *ClientToolsSetParams) { p.DeadlineMS = session.ClientToolMaxDeadlineMS + 1 }, + } + for name, change := range changes { + t.Run(name, func(t *testing.T) { + p := validClientToolsParams() + change(&p) + raw, err := json.Marshal(p) + if err != nil { + t.Fatal(err) + } + e := &clientToolsEngine{} + if _, err := methods["agents.tools_set"].call(e, raw); codeOf(err) != CodeBadParams { + t.Fatalf("accepted %s: %v", name, err) + } + if len(e.sets) != 0 { + t.Fatal("invalid set reached the engine") + } + }) + } +} + +// Contract (h): null is a result, while an error must be a nonempty string. +func TestClientToolResultValidation(t *testing.T) { + for _, raw := range []string{`{}`, `{"call":"c","result":1,"error":"bad"}`, `{"call":"c","error":""}`, `{"call":"c","error":null}`, + `{"call":"c","result":1,"error":null}`, `{"call":"c","error":7}`, `{"call":"c","error":"` + strings.Repeat("x", session.ClientToolErrorMax+1) + `"}`} { + e := &clientToolsEngine{} + if _, err := methods["agents.tool_result"].call(e, json.RawMessage(raw)); codeOf(err) != CodeBadParams { + t.Fatalf("accepted %.90s: %v", raw, err) + } + if len(e.answers) > 0 { + t.Fatal("invalid answer reached the engine") + } + } +} + +// Contract (h): the shipped section stays bounded and names the actual limits. +func TestClientToolsManualLimits(t *testing.T) { + raw, err := os.ReadFile("../manual/chat/client-tools.md") + if err != nil { + t.Fatal(err) + } + page := string(raw) + if strings.Count(page, "\n## ") != 1 || len(raw) >= 2000 { + t.Fatal("client tools manual section is not self-contained and bounded") + } + for _, phrase := range []string{ + fmt.Sprintf("%d sets", session.ClientToolMaxSets), fmt.Sprintf("%d tools per set", session.ClientToolMaxPerSet), + fmt.Sprintf("%d tools total", session.ClientToolMaxPerAgent), fmt.Sprintf("%d seconds", session.ClientToolDefaultDeadlineMS/1000), + fmt.Sprintf("%d seconds is the maximum", session.ClientToolMaxDeadlineMS/1000), fmt.Sprintf("%d KiB", session.ClientToolOutputMax/1024), + "outcome unknown", "Child tasks", "connected", + } { + if !strings.Contains(page, phrase) { + t.Errorf("manual omits %q", phrase) + } + } +} diff --git a/internal/appwire/collections.go b/internal/appwire/collections.go index 579d05e7a6..afb5e92008 100644 --- a/internal/appwire/collections.go +++ b/internal/appwire/collections.go @@ -6,6 +6,7 @@ import ( "strings" "time" "unicode" + "unicode/utf8" ) // WAVE 9: COLLECTIONS (PROTOCOL.md, V75). A collection is a hand-picked list @@ -31,6 +32,7 @@ const ( RefArtifact = "artifact" RefCrew = "crew" RefProcess = "process" + RefFactoryJob = "factory_job" RefStanding = "standing" ) @@ -60,9 +62,10 @@ const ( // Ref is one reference: a task and a process carry their agent as SessionID, // and an artifact is a clean absolute path. type Ref struct { - Kind string `json:"kind" enum:"collection,conversation,task,artifact,crew,process,standing"` + Kind string `json:"kind" enum:"collection,conversation,task,artifact,crew,process,standing,factory_job"` ID string `json:"id"` SessionID string `json:"session_id,omitempty"` + Factory string `json:"factory,omitempty"` } // Member is a reference as a collection holds it, with when it was added; @@ -252,6 +255,9 @@ func plainText(s string, limit int) bool { // CheckRef refuses a malformed reference as bad_params. adding refuses the // legacy standing kind, which can be found and removed and never added. func CheckRef(r Ref, adding bool) error { + if r.Kind != RefFactoryJob && r.Factory != "" { + return Errorf(CodeBadParams, "only a factory_job reference has a factory") + } if !plainText(r.ID, collectionIDMax) { return Errorf(CodeBadParams, "a reference needs an id without surrounding space or control characters") } @@ -263,6 +269,11 @@ func CheckRef(r Ref, adding bool) error { return Errorf(CodeBadParams, "only a task or a process reference has a session_id") } switch r.Kind { + case RefFactoryJob: + n, err := strconv.ParseUint(r.ID, 10, 63) + if err != nil || n == 0 || strconv.FormatUint(n, 10) != r.ID || (!plainText(r.Factory, collectionIDMax) || !utf8.ValidString(r.Factory)) { + return Errorf(CodeBadParams, "a factory_job needs a factory and a positive canonical job number") + } case RefCollection, RefConversation, RefCrew, RefProcess: case RefTask: n, err := strconv.ParseUint(r.ID, 10, 64) diff --git a/internal/appwire/collections_factory_job_test.go b/internal/appwire/collections_factory_job_test.go new file mode 100644 index 0000000000..be9fec713d --- /dev/null +++ b/internal/appwire/collections_factory_job_test.go @@ -0,0 +1,35 @@ +package appwire + +import ( + "encoding/json" + "strings" + "testing" +) + +// Part A (A2): the wire validator and optional fields match store ref identity. +func TestPartACollectionFactoryJobRefs(t *testing.T) { + for _, tc := range []struct { + r Ref + ok bool + }{ + {Ref{Kind: RefFactoryJob, ID: "7", Factory: "f"}, true}, + {Ref{Kind: RefFactoryJob, ID: "7"}, false}, + {Ref{Kind: RefFactoryJob, ID: "0", Factory: "f"}, false}, + {Ref{Kind: RefFactoryJob, ID: "07", Factory: "f"}, false}, + {Ref{Kind: RefFactoryJob, ID: "+7", Factory: "f"}, false}, + {Ref{Kind: RefFactoryJob, ID: "7", Factory: " f"}, false}, + {Ref{Kind: RefFactoryJob, ID: "7", Factory: strings.Repeat("x", 4097)}, false}, + {Ref{Kind: RefFactoryJob, ID: "7", Factory: "f", SessionID: "chat"}, false}, + {Ref{Kind: RefConversation, ID: "chat", Factory: "f"}, false}, + } { + for _, adding := range []bool{false, true} { + if err := CheckRef(tc.r, adding); (err == nil) != tc.ok { + t.Errorf("%+v adding %v: %v", tc.r, adding, err) + } + } + } + raw, err := json.Marshal(Member{Ref: Ref{Kind: RefFactoryJob, ID: "7", Factory: "f"}}) + if err != nil || !strings.Contains(string(raw), `"factory":"f"`) { + t.Fatalf("factory: %s %v", raw, err) + } +} diff --git a/internal/appwire/contract_test.go b/internal/appwire/contract_test.go index 1c6b32e270..19fad5e978 100644 --- a/internal/appwire/contract_test.go +++ b/internal/appwire/contract_test.go @@ -140,6 +140,7 @@ func TestTheSchemaIsGeneratedForTheWholeWire(t *testing.T) { Events map[string]json.RawMessage `json:"events"` Defs map[string]struct { Properties map[string]map[string]any `json:"properties"` + Required []string `json:"required"` } `json:"$defs"` } if err := json.Unmarshal(out, &doc); err != nil { @@ -148,6 +149,15 @@ func TestTheSchemaIsGeneratedForTheWholeWire(t *testing.T) { if len(doc.Methods) != len(Methods()) || len(doc.Events) != len(Events()) { t.Fatalf("schema has %d methods and %d events, want %d and %d", len(doc.Methods), len(doc.Events), len(Methods()), len(Events())) } + // Source contract (d): the event registry exposes the optional durable id. + if got := doc.Defs["AgentsToolCall"].Properties["source_message_id"]["type"]; got != "string" { + t.Fatalf("AgentsToolCall.source_message_id schema type=%v", got) + } + for _, field := range doc.Defs["AgentsToolCall"].Required { + if field == "source_message_id" { + t.Fatal("source_message_id became required") + } + } for _, field := range []string{"invoked", "rejected_before_run", "harness_made", "refused", "media_attempt"} { for _, name := range []string{"TurnTool", "Entry"} { if got := doc.Defs[name].Properties[field]["type"]; got != "boolean" { diff --git a/internal/appwire/events.go b/internal/appwire/events.go index 84ebe55db1..84388ab23c 100644 --- a/internal/appwire/events.go +++ b/internal/appwire/events.go @@ -6,6 +6,7 @@ import "reflect" // other type is a contract break; the test recorders check every event they // see against this table, and the schema is generated from it. var eventShapes = map[string]reflect.Type{ + EventAgentsToolCall: reflect.TypeFor[AgentsToolCall](), EventPairChanged: reflect.TypeFor[PairResult](), EventChatChanged: reflect.TypeFor[ChatChanged](), EventChatRemoved: reflect.TypeFor[ChatRemoved](), diff --git a/internal/appwire/server.go b/internal/appwire/server.go index b52f76ee1b..4291351ad7 100644 --- a/internal/appwire/server.go +++ b/internal/appwire/server.go @@ -214,6 +214,7 @@ func (s *Server) Serve(ctx context.Context, in io.Reader, hello Hello, engine En hello = withReplySuggestionFeature(hello, engine) hello = withUnreadFeature(hello, engine) hello = withErrandFeature(hello, engine) + hello = withClientToolsFeature(hello, engine) hello = withAsksDrawnFeature(hello, engine) hello = withTaskModelsAuxiliaryFeature(hello, engine) hello = withMemorySettingsFeature(hello, engine) @@ -396,7 +397,9 @@ func (s *Server) handle(ctx context.Context, line []byte, engine Engine, infligh // methods is the dispatch table. shutdown is not in it: it is answered by the // loop itself, because it ends the loop. var methods = map[string]method{ - "ping": typed(func(Engine, struct{}) (*Empty, error) { return &Empty{}, nil }), + "agents.tools_set": typed(setClientTools), + "agents.tool_result": typed(answerClientTool), + "ping": typed(func(Engine, struct{}) (*Empty, error) { return &Empty{}, nil }), "agents.list": typed(func(e Engine, p AgentsListParams) (*AgentsResult, error) { // A STATE AND A QUERY are answered by an engine that keeps them. if t, ok := e.(Tidier); ok { diff --git a/internal/manual/chat/client-tools.md b/internal/manual/chat/client-tools.md new file mode 100644 index 0000000000..944eb3eaca --- /dev/null +++ b/internal/manual/chat/client-tools.md @@ -0,0 +1,33 @@ +# Client tools: tools an app adds to a conversation + +## Client tools: connection, limits and waiting + +An app connected through `codeaf app` can attach its own tools and instructions +to one conversation. The model sees each tool's name, description and input +schema, and codeaf sends its calls to that app for an answer. The app checks the +arguments against its schema. Child tasks, workers and clarification +conversations do not receive these tools or instructions. + +Tools appear only while the app is connected. Attaching them does not open the +conversation or start a turn. Changes take effect at the next model step. +Each conversation can hold 4 sets, with 32 tools per set and 64 tools total. +Replacing a set replaces all its tools and instructions; an empty set removes +only that set. Tool names must be unique within the conversation and cannot +reuse an engine tool's name. + +A conversation held by a collection is kept even before its first message; +it can be reopened by id after a restart. + +Set ids start with a lowercase letter, followed by lowercase letters, digits, +underscores, dots or hyphens, up to 64 characters. Tool names follow the same +rule without dots or hyphens. Descriptions take 1–2048 bytes; each input schema +is a JSON object with `type: "object"`, up to 16 KiB. Instructions take up to +4096 bytes per set. Each tool declares `read`, `edit` or `create`; reading-only +contexts refuse edits and creations. The usual tool approval rules still apply. + +The default wait is 30 seconds. An app can choose 1–120 seconds per set; +120 seconds is the maximum. Stopping the turn ends the wait immediately. +A timeout or a disconnect during a call leaves its outcome unknown. Check what +the app did before trying again: codeaf does not retry that call automatically. +Results are compact JSON, cut at 64 KiB with the dropped-byte count. Error +answers take 1–4096 bytes. diff --git a/internal/manual/chat/collections.md b/internal/manual/chat/collections.md index 0b878f3181..4816a949d2 100644 --- a/internal/manual/chat/collections.md +++ b/internal/manual/chat/collections.md @@ -3,7 +3,7 @@ ## How do I group chats in logical folders `codeaf collections` is a local command for organizing references to conversations, -tasks and files. A collection is a logical folder with a stable ID +tasks, Factory jobs and files. A collection is a logical folder with a stable ID and name. Several collections can reference the same record. Collections can contain other collections, including one child shared by several parents; circular membership is refused. Renaming a collection preserves its ID. @@ -71,7 +71,8 @@ belong to collections. ## Adding, removing and finding a chat or work reference Use `codeaf collections add `. -The kinds are `collection`, `conversation`, `task`, `process`, `crew` and `artifact`. There is no kind +The kinds are `collection`, `conversation`, `task`, `process`, `crew`, +`factory_job` and `artifact`. There is no kind for an automation. `standing` named a standing item, which codeaf no longer has: a row that already holds one can still be found and removed, and adding a new one is refused with `standing items are gone, so a collection cannot gain one`. @@ -100,6 +101,19 @@ something that was never there is not an error. Automatic organization, inherited instructions and communication between conversations are not implemented by these commands. +## Adding, finding and removing a Factory job + +Use `codeaf collections add factory_job --factory +`. The job number must be a positive decimal without a sign or leading +zeroes. The factory ID is required: the same number in two factories names two +different jobs. Use the same `--factory` with `find` and `remove`. Other kinds +refuse a factory ID. `show` prints `factory_job factory `; +`--json` includes the `factory` field. + +A collection can reference a job alongside any other records, and several +collections can hold the same job. These commands retain references without +asking the Factory whether the job is available and never start a job. + ## Deleting or duplicating a collection `codeaf collections delete ` deletes the collection, its @@ -108,3 +122,13 @@ memberships, and every membership naming it in another collection, answering referenced is deleted, moved or stopped. `codeaf collections duplicate ` makes a new collection holding the same references, in the same order; the new name follows the same uniqueness rule as `create`. + + +## Collections database versions and older engines + +Opening and reading a collections database never migrate it. The first successful +write upgrades supported versions 1–6 to schema 7; a failed write leaves the file +unchanged. Existing collections and references survive the upgrade, including +Factory job references. Older engines that do not support schema 7 refuse the +database with `unsupported collections database`. Collections are unavailable +in those engines; the data remains intact. Use a compatible engine to read it. diff --git a/internal/session/agent.go b/internal/session/agent.go index d9ea313935..6b7c970668 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -90,6 +90,8 @@ func New(config Config) (*Agent, error) { // parent's account-keyed pool and resolve their own model before their first // request, rather than inheriting whichever adapter the parent last used. func (a *Agent) newChildAgent(config Config) (*Agent, error) { + config.ClientTools = nil + config.System = clientToolsWithoutSections(config.System) if pair, ok := a.ConversationModelPair(); ok && config.ConversationModels == nil { pair.Main = config.Model if config.OneModel || pair.Fallback == pair.Main { @@ -1421,6 +1423,9 @@ type userMessage struct { // leave it empty, and the turn is then stamped with no person message — an // unknown, never a guess at the latest one. answersMessage string + // sourceMessage is the exact recorded message a resumed answer continues. + // Only ResumeStoppedTurn sets it; ordinary notes never inherit the latest message. + sourceMessage string // said is THE PERSON'S OWN WORDS, when what the model reads is not only // them. Empty in every ordinary case, and set where the session writes @@ -2040,7 +2045,7 @@ func (a *Agent) startTurnLocked(ctx context.Context, user userMessage, watcher * // turn, or one a task or member report opened, has a run and no person // message, which is the truth. A steer that lands later is NOT the opener // and changes nothing here. - turnCtx = withTurnOrigin(turnCtx, turnOrigin{run: runIdentity(turnCtx), turn: openingMessage(user)}) + turnCtx = withTurnOrigin(turnCtx, turnOriginLocked(user, runIdentity(turnCtx))) // AND THE TURN CARRIES THE CREATION CONTEXT the accepting door stated on // its opening message (creation_team.go), only when it stated one: any other // turn leaves the team unknown. Never rewritten from who manages a team now. diff --git a/internal/session/bashbelt_worker.go b/internal/session/bashbelt_worker.go index e10c177e40..fa469f45f1 100644 --- a/internal/session/bashbelt_worker.go +++ b/internal/session/bashbelt_worker.go @@ -351,6 +351,7 @@ func (a *Agent) TaskReport() string { return taskReport(a) } // The shared event hub stays asynchronous; only this producer waits, outside // every agent and hub lock. Cancellation also releases a failed event reader. func (a *Agent) sendBeltStep(ctx context.Context, hub *eventHub, event Event) { + event = a.clientToolEvent(event) if !a.config.WaitForBeltSteps { hub.send(event) return diff --git a/internal/session/client_tools.go b/internal/session/client_tools.go new file mode 100644 index 0000000000..ee710beac6 --- /dev/null +++ b/internal/session/client_tools.go @@ -0,0 +1,333 @@ +package session + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "strings" + "time" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/exec/bare" +) + +// Client tool limits are shared by the wire validator and the conversation door. +const ( + ClientToolMaxSets = 4 + ClientToolMaxPerSet = 32 + ClientToolMaxPerAgent = 64 + ClientToolDescriptionMax = 2048 + ClientToolSchemaMax = 16 << 10 + ClientToolInstructionsMax = 4096 + ClientToolErrorMax = 4096 + ClientToolOutputMax = 64 << 10 + ClientToolDefaultDeadlineMS = 30000 + ClientToolMinDeadlineMS = 1000 + ClientToolMaxDeadlineMS = 120000 +) + +// ClientToolDoor belongs to one conversation and one connected app. +type ClientToolDoor interface { + // Snapshots are immutable, so an unchanged version costs no copying. + ClientToolSets() (version uint64, sets []ClientToolSet) + CallClientTool(context.Context, ClientToolCall) (ClientToolAnswer, error) +} + +type ClientTool struct { + Name string `json:"name"` + Description string `json:"description"` + InputSchema json.RawMessage `json:"input_schema"` + Category ActionCategory `json:"category"` +} + +type ClientToolSet struct { + ID string + Instructions string + DeadlineMS int + Tools []ClientTool +} + +type ClientToolCall struct { + Set, Name, TurnCall string + SourceMessage string + Args json.RawMessage + DeadlineMS int +} + +type ClientToolAnswer struct { + Text string + IsError bool +} + +// clientToolState is protected by armMu, along with the belt it describes. +type clientToolState struct { + version uint64 + seen bool + sets []ClientToolSet + names map[string]ActionCategory + retired map[string]bool + section string +} + +func (c Config) clientToolSets() (uint64, []ClientToolSet) { + if c.ClientTools == nil || c.InTask || c.Errand || c.promptProfile().chat() { + return 0, nil + } + return c.ClientTools.ClientToolSets() +} + +const clientToolHeading = "\n\n## Tools from the app: " +const clientToolSectionEnd = "\n" + +func clientToolsSection(config Config) string { + _, sets := config.clientToolSets() + return clientToolsSectionOf(sets) +} + +func clientToolsSectionOf(sets []ClientToolSet) string { + var out strings.Builder + for _, set := range sets { + out.WriteString(clientToolHeading) + out.WriteString(set.ID) + out.WriteByte('\n') + out.WriteString(set.Instructions) + out.WriteString(clientToolSectionEnd) + } + return out.String() +} + +// Child pages lose the app's instructions as well as the execution door. +func clientToolsWithoutSections(page string) string { + start := strings.Index(page, clientToolHeading) + if start < 0 { + return page + } + // All app sections form one tail. The last renderer-owned end also covers + // delimiter text that an app may have included inside its instructions. + end := strings.LastIndex(page, clientToolSectionEnd) + if end < start { + return page[:start] + } + return page[:start] + page[end+len(clientToolSectionEnd):] +} + +// appendClientTools runs during construction and complete belt rebuilds. +func (a *Agent) appendClientTools(builtins []bare.Tool) []bare.Tool { + a.armMu.Lock() + defer a.armMu.Unlock() + version, sets := a.config.clientToolSets() + a.clientTools.version, a.clientTools.seen = version, true + a.clientTools.sets = sets + a.clientTools.section = clientToolsSectionOf(sets) + return a.clientToolTailLocked(builtins, sets) +} + +func (a *Agent) clientToolTailLocked(builtins []bare.Tool, sets []ClientToolSet) []bare.Tool { + a.noteClientToolBuiltinsLocked(builtins) + for name := range a.clientTools.names { + if a.clientTools.retired == nil { + a.clientTools.retired = make(map[string]bool) + } + a.clientTools.retired[name] = true + } + a.clientTools.names = nil + if len(sets) == 0 { + return builtins + } + tools := make([]bare.Tool, len(builtins), len(builtins)+ClientToolMaxPerAgent) + copy(tools, builtins) + held := make(map[string]bool, len(builtins)) + for _, tool := range builtins { + held[tool.Name] = true + } + a.clientTools.names = make(map[string]ActionCategory) + for _, set := range sets { + for _, spec := range set.Tools { + if held[spec.Name] { + continue + } + held[spec.Name] = true + a.clientTools.names[spec.Name] = spec.Category + delete(a.clientTools.retired, spec.Name) + tools = append(tools, a.clientTool(set, spec)) + } + } + return tools +} + +func (a *Agent) noteClientToolBuiltinsLocked(builtins []bare.Tool) { + observer, ok := a.config.ClientTools.(interface{ ClientToolBuiltins([]string) }) + if !ok { + return + } + names := make([]string, 0, len(builtins)) + for _, tool := range builtins { + names = append(names, tool.Name) + } + for _, tools := range a.shelf { + for _, tool := range tools { + names = append(names, tool.Name) + } + } + observer.ClientToolBuiltins(names) +} + +func (a *Agent) clientToolRetiredNotice(name string) (string, bool) { + a.armMu.Lock() + defer a.armMu.Unlock() + if !a.clientTools.retired[name] { + return "", false + } + return ClientToolUnavailable(name).Text, true +} + +func ClientToolUnavailable(name string) ClientToolAnswer { + return ClientToolAnswer{Text: fmt.Sprintf("client_unavailable: the app that provides %s is not connected; nothing was done.", name), IsError: true} +} + +// BoundClientToolOutput reuses the engine's rune-safe cut and spells the loss plainly. +func BoundClientToolOutput(text string) string { + bounded := capBytes(text, ClientToolOutputMax) + if len(text) <= ClientToolOutputMax { + return bounded + } + cut := strings.LastIndex(bounded, "… (") + return bounded[:cut] + fmt.Sprintf("\n[cut: %d bytes dropped]", len(text)-cut) +} + +func (a *Agent) clientTool(set ClientToolSet, spec ClientTool) bare.Tool { + return bare.Tool{Name: spec.Name, Description: spec.Description, Schema: spec.InputSchema, + Execute: func(ctx context.Context, args json.RawMessage) (string, bool, error) { + var object map[string]json.RawMessage + data := bytes.TrimSpace(args) + if len(data) == 0 || data[0] != '{' || !json.Valid(data) || decodeToolArguments(data, &object) != nil || object == nil { + return rejectToolArguments(ctx, invalidArgumentsPrefix+"expected a JSON object"), true, nil + } + answer, err := a.config.ClientTools.CallClientTool(ctx, ClientToolCall{ + Set: set.ID, Name: spec.Name, Args: args, TurnCall: callIDFrom(ctx), DeadlineMS: set.DeadlineMS, + SourceMessage: turnOriginFrom(ctx).answers, + }) + if err != nil { + return err.Error(), true, nil + } + return answer.Text, answer.IsError, nil + }, + } +} + +// A boundary swaps fresh arrays; existing requests can keep walking their snapshot. +func (a *Agent) clientToolsBoundary() { + if a.config.ClientTools == nil { + return + } + version, sets := a.config.clientToolSets() + a.armMu.Lock() + if a.clientTools.seen && version == a.clientTools.version { + a.armMu.Unlock() + return + } + section := clientToolsSectionOf(sets) + changed := section != a.clientTools.section + builtins, definitions, err := a.clientToolBuiltinsLocked(0) + if err != nil { + a.armMu.Unlock() + return + } + a.tools = a.clientToolTailLocked(builtins, sets) + tail, _ := toolDefinitions(a.tools[len(builtins):]) + a.definitions = append(definitions, tail...) + a.clientTools.version, a.clientTools.seen = version, true + a.clientTools.sets, a.clientTools.section = sets, section + a.armMu.Unlock() + if changed { + a.mu.Lock() + a.rerenderSystemLocked(time.Now()) + a.mu.Unlock() + } +} + +func (a *Agent) clientToolBuiltinsLocked(room int) ([]bare.Tool, []ai.ToolDefinition, error) { + cache := a.definitions + if len(cache) < len(a.tools) { + // Lightweight callers may have no cache, or only a previously armed family's definitions. + var err error + cache, err = toolDefinitions(a.tools) + if err != nil { + return nil, nil, err + } + } + tools := make([]bare.Tool, 0, len(a.tools)+room) + definitions := make([]ai.ToolDefinition, 0, len(a.tools)+room) + for i, tool := range a.tools { + if _, client := a.clientTools.names[tool.Name]; client { + continue + } + tools = append(tools, tool) + definitions = append(definitions, cache[i]) + } + return tools, definitions, nil +} + +func (a *Agent) withoutClientToolsLocked(tools []bare.Tool) []bare.Tool { + if len(a.clientTools.names) == 0 { + return tools + } + builtins := make([]bare.Tool, 0, len(tools)) + for _, tool := range tools { + if _, client := a.clientTools.names[tool.Name]; !client { + builtins = append(builtins, tool) + } + } + return builtins +} + +// ActionCategoryForTool resolves this conversation's declaration before the static floor. +func (a *Agent) ActionCategoryForTool(name string) ActionCategory { + a.armMu.Lock() + category, ok := a.clientTools.names[name] + a.armMu.Unlock() + if ok { + return category + } + return ActionCategoryForTool(name) +} + +func (a *Agent) clientToolCategory(name string) (ActionCategory, bool) { + a.armMu.Lock() + defer a.armMu.Unlock() + category, ok := a.clientTools.names[name] + return category, ok +} + +func (a *Agent) clientToolEvent(event Event) Event { + if category, ok := a.clientToolCategory(event.Tool); ok { + event.Category = category + } + return event +} + +func toolEventCategory(event Event) ActionCategory { + if event.Category != "" { + return event.Category + } + return ActionCategoryForTool(event.Tool) +} + +// ClientToolBuiltinNames includes the live belt and deferred tools for collision checks. +func (a *Agent) ClientToolBuiltinNames() []string { + a.armMu.Lock() + defer a.armMu.Unlock() + var names []string + for _, tool := range a.tools { + if _, client := a.clientTools.names[tool.Name]; !client { + names = append(names, tool.Name) + } + } + for _, tools := range a.shelf { + for _, tool := range tools { + names = append(names, tool.Name) + } + } + return names +} diff --git a/internal/session/client_tools_test.go b/internal/session/client_tools_test.go new file mode 100644 index 0000000000..9f5f1e1d76 --- /dev/null +++ b/internal/session/client_tools_test.go @@ -0,0 +1,677 @@ +package session + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/approval" + "github.com/Agent-Field/codeaf/internal/exec/bare" +) + +type clientToolsFakeDoor struct { + version uint64 + sets []ClientToolSet + calls []ClientToolCall + answer ClientToolAnswer +} + +func clientToolSourceStep(id string) step { + return func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse(id, "app_note", `{}`), nil + } +} + +// Source contract (a): every submission door and a queued follow-up name their own line. +func TestClientToolsSourceSubmissionDoors(t *testing.T) { + for _, kind := range []string{"plain", "message", "composed", "follow-up"} { + t.Run(kind, func(t *testing.T) { + door := clientToolsFixture() + entered, release := make(chan struct{}), make(chan struct{}) + first := clientToolSourceStep("first") + if kind == "follow-up" { + first = func(ctx context.Context, messages []ai.Message) (*ai.Response, error) { + close(entered) + select { + case <-release: + case <-ctx.Done(): + return nil, ctx.Err() + } + return clientToolSourceStep("first")(ctx, messages) + } + } + a, _ := newTestAgent(t, &scriptedCompleter{steps: []step{first, finishingStep(), clientToolSourceStep("second"), finishingStep()}}, + func(c *Config) { c.ClientTools = door; c.PromptProfile = "full" }) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + var events, next <-chan Event + var err error + switch kind { + case "plain", "follow-up": + events, err = a.Submit(ctx, "first question") + case "message": + events, err = a.SubmitMessage(ctx, Submission{Text: "first question", ClientID: "another-window"}) + case "composed": + _, events, err = a.SubmitComposed(ctx, Submission{Text: "first question"}) + } + if err != nil { + t.Fatal(err) + } + if kind == "follow-up" { + waitSignal(t, entered, "the first provider step before queuing the follow-up") + next, err = a.FollowUp("second question") + close(release) + if err != nil { + t.Fatal(err) + } + } + collect(t, events) + if next == nil { + next = mustSubmit(t, a, "second question") + } + collect(t, next) + var ids []string + for _, entry := range a.Transcript() { + if entry.Role == "user" && entry.Person != nil && entry.Person.Composed != nil { + ids = append(ids, entry.Person.Composed.MessageID) + } + } + if len(ids) != 2 || ids[0] == "" || ids[0] == ids[1] || len(door.calls) != 2 { + t.Fatalf("messages=%v calls=%+v", ids, door.calls) + } + for i, call := range door.calls { + if call.SourceMessage != ids[i] { + t.Errorf("call %d source=%q want %q", i, call.SourceMessage, ids[i]) + } + } + }) + } +} + +// Source contract (b): takeover replay reuses the recorded id without changing crew provenance. +func TestClientToolsSourceStoppedTurnReplay(t *testing.T) { + path := takenOverMidThought(t, func(a *Agent) { a.InterruptFor(StopByTakeover) }) + want := journalPersonIDs(t, path)[0] + door := clientToolsFixture() + var crewOpener string + a, _ := newTestAgent(t, &scriptedCompleter{steps: []step{ + func(ctx context.Context, messages []ai.Message) (*ai.Response, error) { + crewOpener = turnOriginFrom(ctx).turn + return clientToolSourceStep("resumed")(ctx, messages) + }, finishingStep(), + }}, func(c *Config) { c.SessionFile = path; c.ClientTools = door; c.PromptProfile = "full" }) + events, resumed := a.ResumeStoppedTurn(context.Background()) + if !resumed { + t.Fatal("stopped turn did not resume") + } + collect(t, events) + if len(door.calls) != 1 || door.calls[0].SourceMessage != want || crewOpener != "" { + t.Fatalf("calls=%+v crew opener=%q want message %q", door.calls, crewOpener, want) + } +} + +// Source contract (b), (d): resuming a legacy line never invents a message id. +func TestClientToolsSourceLegacyStoppedTurn(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + journal := `{"type":"session","version":1,"id":"legacy"}` + "\n" + + `{"type":"message","role":"user","content":"old question"}` + "\n" + + `{"type":"error","error":{"door":"taken over","message":"turn ended: taken over"}}` + "\n" + if err := os.WriteFile(path, []byte(journal), 0600); err != nil { + t.Fatal(err) + } + door := clientToolsFixture() + a, _ := newTestAgent(t, &scriptedCompleter{steps: []step{clientToolSourceStep("legacy"), finishingStep()}}, + func(c *Config) { c.SessionFile = path; c.ClientTools = door; c.PromptProfile = "full" }) + events, resumed := a.ResumeStoppedTurn(context.Background()) + if !resumed { + t.Fatal("legacy stopped turn did not resume") + } + collect(t, events) + if len(door.calls) != 1 || door.calls[0].SourceMessage != "" { + t.Fatalf("legacy line gained identity: %+v", door.calls) + } +} + +// Source contract (b), (d): quit and close carry-ons never name a source message, +// even when a person opened the interrupted run in the reopened journal. +func TestClientToolsSourceCarryOnReplay(t *testing.T) { + for _, closeKey := range []bool{false, true} { + for _, wake := range []bool{false, true} { + t.Run(fmt.Sprintf("close=%t/wake=%t", closeKey, wake), func(t *testing.T) { + path := filepath.Join(t.TempDir(), "session.jsonl") + started := make(chan struct{}) + steps := []step{thinkingStep(started)} + if wake { + steps = append([]step{finishingStep()}, steps...) + } + a, _ := newTestAgent(t, &scriptedCompleter{steps: steps}, func(c *Config) { c.SessionFile = path }) + t.Cleanup(a.Interrupt) + var events <-chan Event + if wake { + collect(t, mustSubmit(t, a, "original question")) + wakes, stop := a.WatchWakes() + defer stop() + if !a.enqueueNote(wakeNote("a job finished")) { + t.Fatal("initial wake note refused") + } + events = awaitWallStream(t, wakes, 10*time.Second) + } else { + events = mustSubmit(t, a, "original question") + } + waitSignal(t, started, "the interrupted provider step") + a.InterruptFor(StopByClosing) + collect(t, events) + if err := a.Close(); err != nil { + t.Fatal(err) + } + ids := journalPersonIDs(t, path) + if len(ids) != 1 { + t.Fatalf("recorded person messages=%v want one", ids) + } + door := clientToolsFixture() + fresh, _ := newTestAgent(t, &scriptedCompleter{steps: []step{clientToolSourceStep("carry"), finishingStep()}}, + func(c *Config) { c.SessionFile = path; c.ClientTools = door; c.PromptProfile = "full" }) + wakes, stop := fresh.WatchWakes() + defer stop() + accepted := false + if closeKey { + accepted = fresh.ResumeAfterCloseKey("continuing") + } else { + accepted = fresh.ResumeAfterQuit() + } + if !accepted { + t.Fatal("carry-on refused") + } + finishJobWake(t, wakes) + if len(door.calls) != 1 || door.calls[0].SourceMessage != "" { + t.Fatalf("carry-on calls=%+v want no source; recorded message=%q", door.calls, ids[0]) + } + }) + } + } +} + +// Source contract (c): only a wake answering one drained person's line names it. +func TestClientToolsSourceWakes(t *testing.T) { + for _, kind := range []string{"note", "one person", "two people", "mixed", "coalesced"} { + t.Run(kind, func(t *testing.T) { + door := clientToolsFixture() + var a *Agent + ending := func(context.Context, []ai.Message) (*ai.Response, error) { + var queued []userMessage + switch kind { + case "note": + queued = []userMessage{wakeNote("a job finished")} + case "one person": + queued = []userMessage{userText("drained question")} + case "two people": + queued = []userMessage{userText("drained question"), userText("another question")} + case "mixed": + queued = []userMessage{userText("drained question"), wakeNote("a job finished")} + case "coalesced": + queued = []userMessage{wakeNote("a job finished"), wakeNote("another job finished")} + } + for i := range queued { + if queued[i].wake { + queued[i].authored = true + } + } + // Land after the final request, through the same queue the end drain reads. + a.mu.Lock() + a.steering = append(a.steering, queued...) + a.mu.Unlock() + return textResponse("first answer"), nil + } + a, _ = newTestAgent(t, &scriptedCompleter{steps: []step{clientToolSourceStep("person"), ending, clientToolSourceStep("wake"), finishingStep()}}, + func(c *Config) { c.ClientTools = door; c.PromptProfile = "full" }) + wakes, stop := a.WatchWakes() + defer stop() + collect(t, mustSubmit(t, a, "original question")) + finishJobWake(t, wakes) + want := "" + for _, entry := range a.Transcript() { + if kind == "one person" && entry.Text == "drained question" && entry.Person != nil && entry.Person.Composed != nil { + want = entry.Person.Composed.MessageID + } + } + if kind == "one person" && want == "" { + t.Fatal("drained message has no recorded id") + } + if len(door.calls) != 2 || door.calls[0].SourceMessage == "" || door.calls[1].SourceMessage != want { + t.Fatalf("calls=%+v want wake source %q", door.calls, want) + } + }) + } +} + +// Source contract (d): a steer and a runtime note cannot replace a running turn's source. +func TestClientToolsSourceSurvivesSteering(t *testing.T) { + door := clientToolsFixture() + entered, release := make(chan struct{}), make(chan struct{}) + first := func(ctx context.Context, messages []ai.Message) (*ai.Response, error) { + close(entered) + select { + case <-release: + case <-ctx.Done(): + return nil, ctx.Err() + } + return clientToolSourceStep("before")(ctx, messages) + } + a, _ := newTestAgent(t, &scriptedCompleter{steps: []step{first, clientToolSourceStep("after"), finishingStep()}}, + func(c *Config) { c.ClientTools = door; c.PromptProfile = "full" }) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + events, err := a.Submit(ctx, "original question") + if err != nil { + t.Fatal(err) + } + waitSignal(t, entered, "the first provider step before steering") + steer := mustSubmit(t, a, "a correction") + if !a.enqueueNote(wakeNote("a job finished")) { + t.Fatal("runtime note refused") + } + close(release) + collect(t, events) + collect(t, steer) + var ids []string + for _, entry := range a.Transcript() { + if entry.Role == "user" && entry.Person != nil && entry.Person.Composed != nil { + ids = append(ids, entry.Person.Composed.MessageID) + } + } + if len(ids) != 2 || ids[0] == ids[1] || len(door.calls) != 2 { + t.Fatalf("ids=%v calls=%+v", ids, door.calls) + } + for _, call := range door.calls { + if call.SourceMessage != ids[0] { + t.Errorf("source=%q want opener %q, steer=%q", call.SourceMessage, ids[0], ids[1]) + } + } +} + +func (d *clientToolsFakeDoor) ClientToolSets() (uint64, []ClientToolSet) { return d.version, d.sets } +func (d *clientToolsFakeDoor) CallClientTool(_ context.Context, call ClientToolCall) (ClientToolAnswer, error) { + d.calls = append(d.calls, call) + return d.answer, nil +} + +func clientToolsFixture() *clientToolsFakeDoor { + return &clientToolsFakeDoor{version: 1, sets: []ClientToolSet{{ID: "notes", Instructions: "Use the app's notes.", DeadlineMS: ClientToolDefaultDeadlineMS, + Tools: []ClientTool{{Name: "app_note", Description: "Read a note", InputSchema: json.RawMessage(`{"type":"object","properties":{"id":{"type":"string"}}}`), Category: ActionRead}}}}} +} + +// Contract (a), (e): definitions and instructions follow only this conversation's sets. +func TestClientToolsTailInstructionsAndRemoval(t *testing.T) { + door := clientToolsFixture() + a := &Agent{config: Config{ClientTools: door, profile: profileFull}} + builtins := []bare.Tool{{Name: "read", Schema: json.RawMessage(`{"type":"object"}`)}} + a.tools = a.appendClientTools(builtins) + a.definitions, _ = toolDefinitions(a.tools) + defs := a.beltDefinitions() + if len(defs) != 2 || a.tools[1].Name != "app_note" { + t.Fatalf("client tool is not the tail: %+v", a.tools) + } + encoded, _ := json.Marshal(defs[1]) + for _, word := range []string{"app_note", "Read a note", "properties", "id"} { + if !strings.Contains(string(encoded), word) { + t.Errorf("definition lost %q: %s", word, encoded) + } + } + page := renderSystem(a.config) + if !strings.Contains(page, "## Tools from the app: notes\nUse the app's notes.") { + t.Fatal("instructions absent") + } + if strings.Contains(renderSystem(Config{}), "Tools from the app") { + t.Fatal("ordinary prompt gained client bytes") + } + if got := testing.AllocsPerRun(10, a.clientToolsBoundary); got != 0 { + t.Fatalf("unchanged boundary allocated %v", got) + } + door.version++ + door.sets = nil + a.clientToolsBoundary() + if len(a.beltTools()) != 1 || len(defs) != 2 { + t.Fatal("removal changed an old reader or kept the tool") + } + if strings.Contains(renderSystem(a.config), "Tools from the app") { + t.Fatal("removed instructions remain") + } +} + +// Contract (b), (h): object arguments only, errors and provider ids cross the door. +func TestClientToolsExecuteArgumentsAndError(t *testing.T) { + door := clientToolsFixture() + a := &Agent{config: Config{ClientTools: door}} + tool := a.appendClientTools(nil)[0] + for _, args := range []string{"null", "[]", "1", `"x"`, `"{\"id\":3}"`, "{"} { + text, failed, err := tool.Execute(context.Background(), json.RawMessage(args)) + if !failed || err != nil || !strings.Contains(text, "Invalid arguments") { + t.Fatalf("%s: %q %v %v", args, text, failed, err) + } + } + if len(door.calls) != 0 { + t.Fatal("non-object reached the client") + } + door.answer = ClientToolAnswer{Text: "notes could not do app_note: unavailable", IsError: true} + text, failed, err := tool.Execute(withCallID(context.Background(), "provider-7"), json.RawMessage(`{}`)) + if err != nil || !failed || text != door.answer.Text || door.calls[0].TurnCall != "provider-7" { + t.Fatalf("answer/id lost: %q %+v", text, door.calls) + } +} + +// Source contract (a), (d): Execute reads only the immutable call context. +func TestClientToolsExecuteSourceMessage(t *testing.T) { + door := clientToolsFixture() + a := &Agent{config: Config{ClientTools: door}} + tool := a.appendClientTools(nil)[0] + for _, test := range []struct { + ctx context.Context + want string + }{ + {withTurnOrigin(context.Background(), turnOrigin{turn: "crew-opener", answers: "msg-answer"}), "msg-answer"}, + {withTurnOrigin(context.Background(), turnOrigin{turn: "crew-opener"}), ""}, + {context.Background(), ""}, + } { + if _, failed, err := tool.Execute(test.ctx, json.RawMessage(`{}`)); failed || err != nil { + t.Fatalf("execute: failed=%v err=%v", failed, err) + } + if got := door.calls[len(door.calls)-1].SourceMessage; got != test.want { + t.Fatalf("source=%q want %q", got, test.want) + } + } +} + +// Contract (h): classification is per conversation and client tools never start early. +func TestClientToolsCategoriesAndBuiltinWins(t *testing.T) { + door := clientToolsFixture() + a := &Agent{config: Config{ClientTools: door}} + a.tools = a.appendClientTools(nil) + a.definitions, _ = toolDefinitions(a.tools) + if a.ActionCategoryForTool("app_note") != ActionRead || (&Agent{}).ActionCategoryForTool("app_note") != ActionWork { + t.Fatal("category escaped the conversation") + } + otherDoor := clientToolsFixture() + otherDoor.sets[0].Tools[0].Category = ActionCreate + other := &Agent{config: Config{ClientTools: otherDoor}} + other.tools = other.appendClientTools(nil) + if other.ActionCategoryForTool("app_note") != ActionCreate || a.ActionCategoryForTool("app_note") != ActionRead { + t.Fatal("two conversations shared a category declaration") + } + if earlyTools["app_note"] { + t.Fatal("client tool can start speculatively") + } + old := a.beltTools() + if _, err := a.armFamily([]bare.Tool{{Name: "app_note", Description: "Built-in", Schema: json.RawMessage(`{"type":"object"}`)}}); err != nil { + t.Fatal(err) + } + if len(a.tools) != 1 || a.tools[0].Description != "Built-in" || old[0].Description == "Built-in" { + t.Fatal("late built-in did not win with fresh arrays") + } +} + +// Contract (a), (h): newly armed built-ins keep their prefix ahead of the app tail. +func TestClientToolsLateBuiltinPrecedesTail(t *testing.T) { + door := clientToolsFixture() + a := &Agent{config: Config{ClientTools: door}} + a.tools = a.appendClientTools([]bare.Tool{{Name: "read", Schema: json.RawMessage(`{"type":"object"}`)}}) + a.definitions, _ = toolDefinitions(a.tools) + oldTools, oldDefinitions := a.beltTools(), a.beltDefinitions() + if _, err := a.armFamily([]bare.Tool{{Name: "new_builtin", Schema: json.RawMessage(`{"type":"object"}`)}}); err != nil { + t.Fatal(err) + } + tools := a.beltTools() + if len(tools) != 3 || tools[0].Name != "read" || tools[1].Name != "new_builtin" || tools[2].Name != "app_note" { + t.Fatalf("belt order: %+v", tools) + } + if len(oldTools) != 2 || oldTools[1].Name != "app_note" || len(oldDefinitions) != 2 { + t.Fatal("arming rewrote an old request") + } +} + +// Contract (a), (h): automation looks arm a family without a complete definition cache. +func TestClientToolsArmFamilyWithMissingDefinitions(t *testing.T) { + for _, withClient := range []bool{false, true} { + for _, cached := range []string{"nil", "short prefix", "short tail"} { + t.Run(fmt.Sprintf("client=%t/cache=%s", withClient, cached), func(t *testing.T) { + a := &Agent{} + a.tools = []bare.Tool{ + {Name: "read", Schema: json.RawMessage(`{"type":"object"}`)}, + {Name: "bash", Schema: json.RawMessage(`{"type":"object"}`)}, + } + if withClient { + a.config.ClientTools = clientToolsFixture() + a.tools = a.appendClientTools(a.tools) + } + if cached != "nil" { + from := a.tools[:1] + if cached == "short tail" { + from = a.tools[len(a.tools)-1:] + } + var err error + a.definitions, err = toolDefinitions(from) + if err != nil { + t.Fatal(err) + } + } + cachedCount := len(a.definitions) + oldTools, oldDefinitions := a.beltTools(), a.beltDefinitions() + names, err := a.armFamily([]bare.Tool{{Name: "account_read", Schema: json.RawMessage(`{"type":"object"}`)}}) + if err != nil || len(names) != 1 || names[0] != "account_read" { + t.Fatalf("arming: %v, %v", names, err) + } + want := []string{"read", "bash", "account_read"} + if withClient { + want = append(want, "app_note") + } + tools, definitions := a.beltTools(), a.beltDefinitions() + if len(tools) != len(want) || len(definitions) != len(want) { + t.Fatalf("belt/cache lengths: %d/%d, want %d", len(tools), len(definitions), len(want)) + } + for i, name := range want { + if tools[i].Name != name || definitions[i].Function.Name != name { + t.Fatalf("belt/cache at %d: %s/%s, want %s", i, tools[i].Name, definitions[i].Function.Name, name) + } + } + if len(oldDefinitions) != cachedCount || oldTools[1].Name != "bash" || (withClient && oldTools[2].Name != "app_note") { + t.Fatal("arming changed an earlier snapshot") + } + }) + } + } +} + +// Contract (a), (b): the ordinary provider loop offers definitions and consumes the answer. +func TestClientToolsModelRequestAndToolResult(t *testing.T) { + for _, failed := range []bool{false, true} { + t.Run(map[bool]string{false: "result", true: "error"}[failed], func(t *testing.T) { + door := clientToolsFixture() + door.answer = ClientToolAnswer{Text: `{"note":"found"}`, IsError: failed} + if failed { + door.answer.Text = "notes could not do app_note: unavailable" + } + recorder := &blockRecorder{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse("provider-7", "app_note", `{"id":"n"}`), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, + }} + a, _ := newTestAgent(t, recorder, func(c *Config) { c.System = ""; c.ClientTools = door; c.PromptProfile = "full" }) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + events, err := a.Submit(ctx, "read the app note") + if err != nil { + t.Fatal(err) + } + collected := collect(t, events) + if len(door.calls) != 1 || door.calls[0].TurnCall != "provider-7" { + t.Fatalf("dispatches: %+v", door.calls) + } + if got := recorder.resultAt(t, 1, "provider-7"); got != door.answer.Text { + t.Fatalf("model result: %q", got) + } + recorder.mu.Lock() + block := recorder.blocks[0] + page := messageText(recorder.sent[0][0]) + recorder.mu.Unlock() + definition, _ := json.Marshal(block[len(block)-1]) + if !strings.Contains(string(definition), "app_note") || !strings.Contains(page, "## Tools from the app: notes") { + t.Fatalf("request lost its tail/page: %s", definition) + } + ended := false + for _, event := range collected { + if event.CallID != "provider-7" { + continue + } + if event.Kind == EventToolBegin && toolEventCategory(event) != ActionRead { + t.Fatal("step lost the declared category") + } + if event.Kind == EventToolEnd || event.Kind == EventToolFailed { + ended = true + if (event.Kind == EventToolFailed) != failed { + t.Fatalf("error flag lost: %+v", event) + } + } + } + if !ended { + t.Fatal("tool result lifecycle absent") + } + }) + } +} + +// Contract (e): instructions rerender only on this agent's own page at a boundary. +func TestClientToolsBoundaryRefreshesOwnedPageAndPreservesCallerPage(t *testing.T) { + for _, supplied := range []string{"", "The caller's page"} { + door := clientToolsFixture() + a, _ := newTestAgent(t, &scriptedCompleter{}, func(c *Config) { c.System = supplied; c.ClientTools = door }) + door.version++ + door.sets = []ClientToolSet{{ID: "notes", Instructions: "Read the updated notes.", Tools: door.sets[0].Tools}} + a.clientToolsBoundary() + if supplied != "" && a.system != supplied { + t.Fatal("the caller's page was rewritten") + } + if supplied == "" && !strings.Contains(a.system, "Read the updated notes.") { + t.Fatal("instruction change did not rerender") + } + door.version++ + door.sets = nil + a.clientToolsBoundary() + if strings.Contains(a.system, "Tools from the app") { + t.Fatal("disconnected page kept instructions") + } + if notice, ok := a.clientToolRetiredNotice("app_note"); !ok || notice != ClientToolUnavailable("app_note").Text { + t.Fatal("stale call lost the unavailable answer") + } + } +} + +// Contract (h): declared reads alone can cross a reading-only context. +func TestClientToolsReadingCategoriesAndApprovalPolicy(t *testing.T) { + door := clientToolsFixture() + a := &Agent{config: Config{ClientTools: door, taskControl: &TaskNode{spec: taskSpec{quick: &quickTaskSpec{reading: true}}}}} + a.tools = a.appendClientTools(nil) + if a.readingToolRefusal("app_note") != "" { + t.Fatal("declared read refused") + } + for _, category := range []ActionCategory{ActionEdit, ActionCreate} { + door.sets[0].Tools[0].Category = category + a.tools = a.appendClientTools(nil) + if a.readingToolRefusal("app_note") == "" { + t.Fatalf("reading allowed %s", category) + } + } + for _, action := range []approval.Action{approval.ActionAllow, approval.ActionPrompt, approval.ActionDeny} { + a.config.ApprovalPolicy = &approval.Policy{Default: action} + for _, name := range []string{"app_note", "factory_add", "change_setting"} { + decision, _ := a.decide(ai.ToolCall{Function: ai.ToolCallFunction{Name: name, Arguments: `{}`}}) + if decision.Action != action { + t.Fatalf("%s changed the ordinary policy: %+v", name, decision) + } + } + } +} + +// Contract (a), (h), source contract (c): errands and children have no client capability. +func TestClientToolsDoNotTravelToChildren(t *testing.T) { + door := clientToolsFixture() + door.sets[0].Instructions += clientToolSectionEnd + "\nThese are still the app's instructions." + page := "The caller's page" + clientToolsSection(Config{ClientTools: door}) + "\nFooter" + if got := clientToolsWithoutSections(page); got != "The caller's page\nFooter" { + t.Fatalf("child page: %q", got) + } + for _, config := range []Config{{ClientTools: door, InTask: true}, {ClientTools: door, Errand: true}} { + _, sets := config.clientToolSets() + if len(sets) != 0 || clientToolsSection(config) != "" { + t.Fatal("child inherited a set") + } + if tools := (&Agent{config: config}).appendClientTools(nil); len(tools) != 0 || len(door.calls) != 0 { + t.Fatal("an errand or child gained a client tool or emitted a call") + } + } + started := make(chan struct{}) + parent, _ := newTestAgent(t, &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return toolResponse("original", "write", `{"path":"note.txt","content":"note"}`), nil + }, + func(ctx context.Context, _ []ai.Message) (*ai.Response, error) { + close(started) + <-ctx.Done() + return nil, ctx.Err() + }, + }}, func(c *Config) { + c.System, c.PromptProfile = "", "full" + c.ClientTools = door + c.AskConsent, c.Interactive = true, true + c.ApprovalPolicy = promptAll() + }) + t.Cleanup(parent.Interrupt) + if parent.config.ClientTools == nil || !strings.Contains(parent.system, "Tools from the app: notes") { + t.Fatal("parent lacks the capability this test must exclude") + } + cfg := parent.config + cfg.System = parent.system + cfg.SessionFile, cfg.Place = "", Place{} + cfg.InTask = true + child, err := parent.newChildAgent(cfg) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = child.Close() }) + clientToolsAssertExcluded(t, child) + + watch, stop := parent.WatchQuestions() + defer stop() + mustSubmit(t, parent, "write note.txt") + q := discussionWait(t, watch, func(e Event) bool { return e.Kind == EventQuestion }).Question + if err := parent.ResolveQuestion(Answer{Kind: q.Kind, ID: q.ID, Clarify: true, AskedBack: []Exchange{{Asked: "why this note?"}}}); err != nil { + t.Fatal(err) + } + waitSignal(t, started, "the clarification provider step") + parent.mu.Lock() + clarification := parent.discussions["clarify-1"].child + parent.mu.Unlock() + clientToolsAssertExcluded(t, clarification) +} + +func clientToolsAssertExcluded(t *testing.T, child *Agent) { + t.Helper() + if child.config.ClientTools != nil { + t.Fatal("child inherited the client door") + } + for _, page := range []string{child.config.System, child.system} { + if strings.Contains(page, "Tools from the app") || strings.Contains(page, "These are still the app's instructions.") { + t.Fatal("child inherited app instructions") + } + } + for _, definition := range child.beltDefinitions() { + if definition.Function.Name == "app_note" { + t.Fatal("child offers a client tool") + } + } +} diff --git a/internal/session/connect.go b/internal/session/connect.go index 2d52f4fbc8..103d00f84c 100644 --- a/internal/session/connect.go +++ b/internal/session/connect.go @@ -565,7 +565,9 @@ func (a *Agent) armFamily(tools []bare.Tool) ([]string, error) { held := make(map[string]bool, len(a.tools)) for _, tool := range a.tools { - held[tool.Name] = true + if _, client := a.clientTools.names[tool.Name]; !client { + held[tool.Name] = true + } } var arriving []bare.Tool var names []string @@ -575,6 +577,7 @@ func (a *Agent) armFamily(tools []bare.Tool) ([]string, error) { } arriving = append(arriving, tool) names = append(names, tool.Name) + held[tool.Name] = true } if len(arriving) == 0 { return nil, nil @@ -586,12 +589,14 @@ func (a *Agent) armFamily(tools []bare.Tool) ([]string, error) { // Fresh arrays, exact length: the old ones are still being walked by // whatever took a snapshot a moment ago, and appending into spare capacity // would write into the array they are reading. - grownTools := make([]bare.Tool, 0, len(a.tools)+len(arriving)) - grownTools = append(append(grownTools, a.tools...), arriving...) - grownDefinitions := make([]ai.ToolDefinition, 0, len(a.definitions)+len(definitions)) - grownDefinitions = append(append(grownDefinitions, a.definitions...), definitions...) - a.tools = grownTools - a.definitions = grownDefinitions + builtins, builtinDefinitions, err := a.clientToolBuiltinsLocked(len(arriving)) + if err != nil { + return nil, err + } + builtins = append(builtins, arriving...) + a.tools = a.clientToolTailLocked(builtins, a.clientTools.sets) + tail, _ := toolDefinitions(a.tools[len(builtins):]) + a.definitions = append(append(builtinDefinitions, definitions...), tail...) for _, tool := range arriving { known := false for _, kept := range a.profileArmed { diff --git a/internal/session/inherit.go b/internal/session/inherit.go index 3113ed686e..ccbeb8115a 100644 --- a/internal/session/inherit.go +++ b/internal/session/inherit.go @@ -387,6 +387,7 @@ func adoptSeed(child *Agent, quick *quickTaskSpec) { // carries a System gets at construction (agent.go), said here because this // page arrives after the agent was built. child.system = promotedSystem(quick.system) + child.system = clientToolsWithoutSections(child.system) child.systemOwn = false messages[0] = textMessage("system", child.system) child.messages = messages diff --git a/internal/session/loop.go b/internal/session/loop.go index e59816da6e..e29cc2a8bd 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -3547,7 +3547,7 @@ func (a *Agent) runToolsWarm(ctx context.Context, ep *episode, calls []ai.ToolCa if service != "" { serviceName = a.serviceName(service) } - hub.send(Event{ + hub.send(a.clientToolEvent(Event{ Kind: EventToolBegin, Tool: call.Function.Name, Hint: a.gloss(call), @@ -3556,7 +3556,7 @@ func (a *Agent) runToolsWarm(ctx context.Context, ep *episode, calls []ai.ToolCa Batch: batch, Service: service, ServiceName: serviceName, - }) + })) } // THE NARRATOR ARMS BESIDE THE WORK, not after a long silence. A half-second @@ -3948,6 +3948,9 @@ func (a *Agent) dispatchTool(ctx context.Context, ep *episode, hub *eventHub, ca if notice, retired := a.teamRetiredNotice(call.Function.Name); retired { return toolResult{text: notice, isError: true, harness: true} } + if notice, retired := a.clientToolRetiredNotice(call.Function.Name); retired { + return toolResult{text: notice, isError: true, harness: true} + } // A name nobody ever had keeps the old answer, and keeps it word for word: // that one IS a sentence about the model. return toolResult{text: "Unknown tool: " + call.Function.Name, isError: true} diff --git a/internal/session/prompt.go b/internal/session/prompt.go index 582229d166..9d2ca93d82 100644 --- a/internal/session/prompt.go +++ b/internal/session/prompt.go @@ -400,6 +400,7 @@ func renderSystemAt(config Config, now time.Time) string { } out.WriteString(workerFooter(config, now)) + out.WriteString(clientToolsSection(config)) return out.String() } diff --git a/internal/session/promptprofile_live.go b/internal/session/promptprofile_live.go index 4a1d55e675..a2abd43f3b 100644 --- a/internal/session/promptprofile_live.go +++ b/internal/session/promptprofile_live.go @@ -52,6 +52,7 @@ func (a *Agent) preparePromptProfile(model string) error { state.current.Store(next) tools := a.belt() a.armMu.Lock() + tools = a.withoutClientToolsLocked(tools) // Explicitly loaded capabilities and connected services remain usable. // Only the default belt is repartitioned when the window changes. held := make(map[string]bool, len(tools)) @@ -80,6 +81,11 @@ func (a *Agent) preparePromptProfile(model string) error { held[tool.Name] = true } } + if !next.chat() { + tools = a.clientToolTailLocked(tools, a.clientTools.sets) + } else { + a.clientTools.names = nil + } definitions, err := toolDefinitions(tools) if err != nil { a.shelf, a.shelfOrder, a.prearm = oldShelf, oldOrder, oldPrearm diff --git a/internal/session/questionconversation.go b/internal/session/questionconversation.go index 613d5336df..3d4b1b90b3 100644 --- a/internal/session/questionconversation.go +++ b/internal/session/questionconversation.go @@ -96,6 +96,7 @@ func (a *Agent) clarifyQuestion(answer Answer) error { } a.mu.Lock() cfg := a.config + cfg.ClientTools = nil cfg.Model = a.model cfg.SessionFile, cfg.Place = "", Place{} cfg.rootSession = a.id @@ -108,6 +109,7 @@ func (a *Agent) clarifyQuestion(answer Answer) error { cfg.ApprovalPolicy = a.config.ApprovalPolicy } cfg.System = a.system + cfg.System = clientToolsWithoutSections(cfg.System) if a.guardianOverride != nil { cfg.Guardian = *a.guardianOverride } diff --git a/internal/session/resume.go b/internal/session/resume.go index 424416f76f..e7f33b7dca 100644 --- a/internal/session/resume.go +++ b/internal/session/resume.go @@ -155,6 +155,9 @@ func (a *Agent) ResumeStoppedTurn(ctx context.Context) (<-chan Event, bool) { resumed := userMessage{message: last, resumed: true} if mark := a.presentation.of(last); mark != nil { resumed.creation = acceptedCreation(mark.Creation) + if mark.Composed != nil { + resumed.sourceMessage = mark.Composed.MessageID + } } events := a.startTurnLocked(ctx, resumed, nil) a.mu.Unlock() diff --git a/internal/session/session.go b/internal/session/session.go index 58af7d3258..9eddab8534 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -852,6 +852,8 @@ type Event struct { // a surface reads as "unknown" and never as "nothing happened". Compaction *CompactionFact `json:"Compaction,omitempty"` + // Tool lifecycle events can also carry the conversation's declared client-tool + // category, so surfaces need no name-only guess for those dynamic tools. // Category is the FAMILY OF WORK an EventCaption's sentence is about — one // word from the closed list in actioncategory.go — and it is zero on every // other kind. @@ -2523,6 +2525,9 @@ type Config struct { // foremanFloorDoor), on a launch that has a Factory door. Floor FloorDoor + // ClientTools is the door onto tools an app attached to this conversation. Nil is no door. + ClientTools ClientToolDoor + // factoryWindow overrides how long one factory card holds its tool call // open, for tests that watch the window end. Zero is [factoryCardWindow]. factoryWindow time.Duration @@ -2905,21 +2910,20 @@ type Agent struct { systemAt time.Time systemOwn bool // tools is the belt and definitions is its wire form, built once at - // construction — rebuilding them per step would re-marshal every schema on - // the hot path — and thereafter grown only at the tail under armMu - // (connect.go), with one exception: the team verbs leave when the role that - // gave them does (team.go's [Agent.retireTeamTools]). + // construction. Built-ins grow under armMu (connect.go), before the replaceable + // client-tool tail. Team verbs leave when their role ends (team.go). // // Both are COPY-ON-WRITE: arming allocates a new array and swaps the header, // so a reader that took a snapshot under armMu may walk it without the lock // and can never see a half-written slice. Nothing already in either is ever - // moved or rewritten, and nothing but a team verb is removed, because the - // definition block rides at the front of every request and a definition that - // shifts re-bills the whole prompt behind it (internal/exec's tools.go states - // the law). + // moved or rewritten in place. Built-ins keep their order while client sets + // can be replaced. The definition block rides at the front of every request, + // so a definition that shifts re-bills the whole prompt behind it + // (internal/exec's tools.go states the law). presentation *presentationIndex tools []bare.Tool definitions []ai.ToolDefinition + clientTools clientToolState // served is what the belt cannot say about the tools an ACCOUNT named // rather than this build (served.go): whose account each one is, what the // account calls it, and which capability governs it. Keyed by the name the diff --git a/internal/session/sweep.go b/internal/session/sweep.go index 6ffd199a76..eaea595da6 100644 --- a/internal/session/sweep.go +++ b/internal/session/sweep.go @@ -92,18 +92,19 @@ const ( // an errand's folder is still somebody's — an automation was saved from it — // and may be nil. func SweepHome(errandsRoot string, held func(dir string) bool, note func(string)) { - SweepHomeContext(context.Background(), errandsRoot, held, note) + SweepHomeContext(context.Background(), errandsRoot, held, nil, note) } // SweepHomeContext is SweepHome with cancellation. Cancellation is checked // before resolving the home so a stopped launch cannot target a home selected -// after it started. -func SweepHomeContext(ctx context.Context, errandsRoot string, held func(dir string) bool, note func(string)) { +// after it started. A non-nil keep preserves project conversations retained by +// the caller; held independently preserves errands saved by automations. +func SweepHomeContext(ctx context.Context, errandsRoot string, held, keep func(dir string) bool, note func(string)) { if contextDone(ctx) { return } now := time.Now() - SweepPlacesContext(ctx, home.Join("v3", placesDirName), now, note) + SweepPlacesContext(ctx, home.Join("v3", placesDirName), now, keep, note) if contextDone(ctx) { return } @@ -115,12 +116,13 @@ func SweepHomeContext(ctx context.Context, errandsRoot string, held func(dir str // A root that is not there is not a failure — it is a machine that has not held // a conversation yet — and answers silently. func SweepPlaces(root string, now time.Time, note func(string)) { - SweepPlacesContext(context.Background(), root, now, note) + SweepPlacesContext(context.Background(), root, now, nil, note) } // SweepPlacesContext is SweepPlaces with cancellation between entries and -// immediately before each destructive operation. -func SweepPlacesContext(ctx context.Context, root string, now time.Time, note func(string)) { +// immediately before each destructive operation. A non-nil keep can retain a +// conversation that would otherwise be litter, without preventing log expiry. +func SweepPlacesContext(ctx context.Context, root string, now time.Time, keep func(dir string) bool, note func(string)) { if contextDone(ctx) { return } @@ -155,7 +157,7 @@ func SweepPlacesContext(ctx context.Context, root string, now time.Time, note fu if !entry.IsDir() { continue } - sweepSession(ctx, filepath.Join(path, entry.Name()), now, note) + sweepSession(ctx, filepath.Join(path, entry.Name()), now, keep, note) } } } @@ -163,7 +165,7 @@ func SweepPlacesContext(ctx context.Context, root string, now time.Time, note fu // sweepSession is the three rules over one session folder, in the order that // makes rule 3 unconditional: the live check first, and nothing at all happens // to a folder that fails it. -func sweepSession(ctx context.Context, dir string, now time.Time, note func(string)) { +func sweepSession(ctx context.Context, dir string, now time.Time, keep func(dir string) bool, note func(string)) { if contextDone(ctx) || sessionIsOpen(dir) { // Somebody is talking to it. Not its logs, not its folder, nothing. return @@ -185,6 +187,9 @@ func sweepSession(ctx context.Context, dir string, now time.Time, note func(stri retireCheckpointForks(ctx, dir, true, note) return } + if keep != nil && keep(dir) { + return + } reapSession(ctx, dir, meta, note) } diff --git a/internal/session/sweep_test.go b/internal/session/sweep_test.go index a5287ee018..14b150e8cc 100644 --- a/internal/session/sweep_test.go +++ b/internal/session/sweep_test.go @@ -543,7 +543,7 @@ func TestSweepHomeContextCancelledBeforeStartTouchesNothing(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - SweepHomeContext(ctx, errandsRoot, nil, func(line string) { t.Fatalf("cancelled sweep said %q", line) }) + SweepHomeContext(ctx, errandsRoot, nil, nil, func(line string) { t.Fatalf("cancelled sweep said %q", line) }) for _, kept := range []string{place, staleLog, exchange} { if _, err := os.Stat(kept); err != nil { @@ -564,7 +564,7 @@ func TestSweepPlacesContextCancellationStopsLaterDestructiveOperations(t *testin first, firstTree := newSweptUniverseSession(t, root, "1111111111111111", repo, now) second, secondTree := newSweptUniverseSession(t, root, "2222222222222222", repo, now) ctx, cancel := context.WithCancel(context.Background()) - SweepPlacesContext(ctx, root, now, func(line string) { + SweepPlacesContext(ctx, root, now, nil, func(line string) { if strings.Contains(line, firstTree.universe) { cancel() } diff --git a/internal/session/task_app_steps.go b/internal/session/task_app_steps.go index d1fbf05064..80e1fc2aa9 100644 --- a/internal/session/task_app_steps.go +++ b/internal/session/task_app_steps.go @@ -248,7 +248,7 @@ func (l *taskLive) openStep(event Event, at time.Time) { ID: appLiveStepID(event, len(l.calls)), Call: event.CallID, Tool: strings.TrimSpace(event.Tool), - Category: ActionCategoryForTool(event.Tool), + Category: toolEventCategory(event), Target: appStepText(event.Hint), Args: event.Args, Running: true, diff --git a/internal/session/task_quick_files.go b/internal/session/task_quick_files.go index b7174a0bb1..fc56495bf8 100644 --- a/internal/session/task_quick_files.go +++ b/internal/session/task_quick_files.go @@ -108,6 +108,9 @@ func (a *Agent) executeTaskTool(ctx context.Context, tool bare.Tool, call ai.Too // readingToolRefusal also guards calls named off the belt. The names come from // the same set that arms the hand-off, so its promise cannot drift from it. func (a *Agent) readingToolRefusal(name string) string { + if category, client := a.clientToolCategory(name); client && category == ActionRead { + return "" + } if !a.config.readingTask() || earlyTools[name] { return "" } diff --git a/internal/session/team.go b/internal/session/team.go index 35ce4313b0..027fbc340d 100644 --- a/internal/session/team.go +++ b/internal/session/team.go @@ -488,6 +488,7 @@ func memberOf(team teams.Team, keys []string) (teams.Member, bool) { // and a demotion is rare enough that paying once for it is cheap. A role // gained again appends its verbs at the tail as before. func (a *Agent) teamBoundary() string { + a.clientToolsBoundary() profile := a.config.teamProfile() if profile == "" { return "" diff --git a/internal/session/tools.go b/internal/session/tools.go index dd5d1177b6..40e6597c19 100644 --- a/internal/session/tools.go +++ b/internal/session/tools.go @@ -334,7 +334,7 @@ func (a *Agent) belt() []bare.Tool { // (tools_capabilities.go). It removes no capability and adds none: a machine // with no media models still has no media group, and a shelf with nothing on // it puts no verb on this belt at all. - return a.shelveDeferred(tools) + return a.appendClientTools(a.shelveDeferred(tools)) } // toolDefinitions builds the wire form of the belt, carrying each tool's diff --git a/internal/session/turn_origin.go b/internal/session/turn_origin.go index 1804318433..98e513e508 100644 --- a/internal/session/turn_origin.go +++ b/internal/session/turn_origin.go @@ -6,26 +6,39 @@ import ( "github.com/Agent-Field/codeaf/internal/teams" ) -// turnOrigin is the two facts that say which turn caused a line of team -// traffic: the run that is writing it and the person's message that OPENED that -// run. It is built once, at the moment a turn starts, and never changed. +// turnOrigin holds the run, the person's message that opened it for team +// traffic, and the message it answers for client tools. It is built once, at +// the moment a turn starts, and never changed. // // IT IS CARRIED ON THE TURN'S CONTEXT AND NEVER READ FROM THE AGENT. A line is // stamped from the context of the call that produced it, or from a copy taken // when a write was queued, so a turn that was abandoned and replaced, a late // callback and a steer that landed mid-turn can never charge their line to the // turn that happens to be current when the write finally runs. The same law as -// [withRunIdentity], one value wider. +// [withRunIdentity], with the message identities beside it. type turnOrigin struct { // run is the turn's run identity, the id its journal records as `usage_run`. run string // turn is the durable id of the person's message that opened the turn, "" // when no person message opened it or none can be named exactly. turn string + // answers is the durable id of the message this turn answers. Resuming an + // answer can name it without changing who opened the run for crew traffic. + answers string } type turnOriginKey struct{} +// turnOriginLocked captures the message being answered before the turn runs. +// A resumed stopped answer reuses its recorded id independently of crew provenance. +func turnOriginLocked(user userMessage, run string) turnOrigin { + origin := turnOrigin{run: run, turn: openingMessage(user), answers: openingMessage(user)} + if user.resumed { + origin.answers = user.sourceMessage + } + return origin +} + // withTurnOrigin hangs origin on ctx for the life of the turn. func withTurnOrigin(ctx context.Context, origin turnOrigin) context.Context { return context.WithValue(ctx, turnOriginKey{}, origin) diff --git a/internal/workspace/schema3_test.go b/internal/workspace/schema3_test.go new file mode 100644 index 0000000000..fdb8483631 --- /dev/null +++ b/internal/workspace/schema3_test.go @@ -0,0 +1,206 @@ +package workspace + +import ( + "context" + "database/sql" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// Part A (a): this literal is schema 2, independent of the schema being written. +const schemaTwoDDL = ` +CREATE TABLE collections (seq INTEGER PRIMARY KEY AUTOINCREMENT,id TEXT NOT NULL UNIQUE,name TEXT NOT NULL,created_at INTEGER NOT NULL DEFAULT 0); +CREATE TABLE memberships ( + seq INTEGER PRIMARY KEY AUTOINCREMENT,collection_id TEXT NOT NULL REFERENCES collections(id) ON DELETE CASCADE, + kind TEXT NOT NULL CHECK(kind IN ('collection','conversation','task','standing','artifact','crew','process')), + ref_id TEXT NOT NULL,session_id TEXT NOT NULL,target_collection TEXT REFERENCES collections(id) ON DELETE CASCADE, + added_at INTEGER NOT NULL DEFAULT 0, + CHECK ((kind='collection' AND target_collection IS NOT NULL AND target_collection=ref_id) OR (kind!='collection' AND target_collection IS NULL)), + UNIQUE(collection_id,kind,ref_id,session_id)); +CREATE INDEX memberships_reference ON memberships(kind,ref_id,session_id); +CREATE INDEX memberships_target ON memberships(target_collection); +PRAGMA application_id=1095123788; PRAGMA user_version=2; +INSERT INTO collections(seq,id,name,created_at) VALUES (4,'aaaa','Product',1234),(9,'bbbb','Nested',2345); +INSERT INTO memberships(seq,collection_id,kind,ref_id,session_id,target_collection,added_at) VALUES + (2,'aaaa','conversation','chat','',NULL,4567),(6,'aaaa','task','3','chat',NULL,5678), + (11,'aaaa','collection','bbbb','','bbbb',6789); +` + +func writeSchemaTwo(t *testing.T) string { + t.Helper() + path := filepath.Join(t.TempDir(), "two.db") + db, err := sql.Open("sqlite", path) + if err != nil { + t.Fatal(err) + } + if _, err := db.Exec(schemaTwoDDL); err != nil { + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + return path +} + +func TestPartASchemaTwoReadsWithoutWritingAndMigratesAllRows(t *testing.T) { + ctx := context.Background() + path := writeSchemaTwo(t) + before, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + s := openTestStore(t, path) + wantC, wantM := legacyRows(t, s.db) + list, err := s.List(ctx, &Ref{Kind: ConversationKind, ID: "chat"}) + if err != nil || len(list) != 2 || list[0].CreatedAt != 1234 || list[0].LastAddedAt != 6789 || !*list[0].Holds { + t.Fatalf("schema 2 read: %+v, %v", list, err) + } + tree, err := s.Tree(ctx, "aaaa", 0, 20, "") + if err != nil || len(tree.Members) != 3 || tree.Members[0].AddedAt != 6789 { + t.Fatalf("tree: %+v, %v", tree, err) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(before) != string(after) { + t.Fatal("read changed schema 2") + } + if err := s.Rename(ctx, "aaaa", "Products"); err != nil { + t.Fatal(err) + } + if userVersion(t, s) != schemaVersion { + t.Fatal("first write did not migrate to the current schema") + } + gotC, gotM := legacyRows(t, s.db) + wantC[0] = strings.Replace(wantC[0], "Product|", "Products|", 1) + if !reflect.DeepEqual(gotC, wantC) || !reflect.DeepEqual(gotM, wantM) { + t.Fatalf("migration lost rows: %v %v", gotC, gotM) + } + rows, err := s.db.Query("SELECT seq,created_at FROM collections ORDER BY seq") + if err != nil { + t.Fatal(err) + } + var collectionTimes [][2]int64 + for rows.Next() { + var seq, at int64 + if err := rows.Scan(&seq, &at); err != nil { + t.Fatal(err) + } + collectionTimes = append(collectionTimes, [2]int64{seq, at}) + } + if err := rows.Err(); err != nil { + t.Fatal(err) + } + if err := rows.Close(); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(collectionTimes, [][2]int64{{4, 1234}, {9, 2345}}) { + t.Fatalf("collection times changed: %+v", collectionTimes) + } + rows, err = s.db.Query("SELECT seq,added_at FROM memberships ORDER BY seq") + if err != nil { + t.Fatal(err) + } + var memberTimes [][2]int64 + for rows.Next() { + var seq, at int64 + if err := rows.Scan(&seq, &at); err != nil { + t.Fatal(err) + } + memberTimes = append(memberTimes, [2]int64{seq, at}) + } + if err := rows.Err(); err != nil { + t.Fatal(err) + } + if err := rows.Close(); err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(memberTimes, [][2]int64{{2, 4567}, {6, 5678}, {11, 6789}}) { + t.Fatalf("membership times changed: %+v", memberTimes) + } + if err := s.Add(ctx, "aaaa", Ref{Kind: FactoryJobKind, ID: "17", Factory: "factory-a"}); err != nil { + t.Fatal(err) + } +} + +func TestPartASchemaTwoMigrationRollsBackWithFailedWrite(t *testing.T) { + path := writeSchemaTwo(t) + before, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + s := openTestStore(t, path) + if err := s.Rename(context.Background(), "aaaa", "Nested"); err != ErrNameTaken { + t.Fatalf("want name conflict: %v", err) + } + after, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(before) != string(after) || userVersion(t, s) != 2 { + t.Fatal("failed first write changed file") + } +} + +func TestPartAFactoryJobRefValidation(t *testing.T) { + for _, tc := range []struct { + ref Ref + ok bool + }{ + {Ref{Kind: FactoryJobKind, ID: "17", Factory: "f"}, true}, + {Ref{Kind: FactoryJobKind, ID: "0", Factory: "f"}, false}, + {Ref{Kind: FactoryJobKind, ID: "017", Factory: "f"}, false}, + {Ref{Kind: FactoryJobKind, ID: "+17", Factory: "f"}, false}, + {Ref{Kind: FactoryJobKind, ID: "17"}, false}, + {Ref{Kind: FactoryJobKind, ID: "17", Factory: " f"}, false}, + {Ref{Kind: FactoryJobKind, ID: "17", Factory: "f", SessionID: "chat"}, false}, + {Ref{Kind: ConversationKind, ID: "chat", Factory: "f"}, false}, + } { + if err := tc.ref.Validate(); (err == nil) != tc.ok { + t.Errorf("%+v: %v", tc.ref, err) + } + } +} + +// Part A (a): deleted sequence numbers remain retired across both migrations, +// preserving the monotonic keys used by existing listing and member cursors. +func TestPartAMigrationPreservesSequenceHighWatermarks(t *testing.T) { + for _, tc := range []struct { + name string + fixture func(*testing.T) string + }{ + {"schema 1", writeLegacyStore}, {"schema 2", writeSchemaTwo}, + } { + t.Run(tc.name, func(t *testing.T) { + s := openTestStore(t, tc.fixture(t)) + ctx := context.Background() + if _, err := s.db.Exec(`INSERT INTO collections(seq,id,name) VALUES (100,'gone','Gone'); +DELETE FROM collections WHERE id='gone'; +INSERT INTO memberships(seq,collection_id,kind,ref_id,session_id) VALUES (200,'aaaa','conversation','gone',''); +DELETE FROM memberships WHERE ref_id='gone';`); err != nil { + t.Fatal(err) + } + c, err := s.Create(ctx, "After") + if err != nil { + t.Fatal(err) + } + if err := s.Add(ctx, c.ID, Ref{Kind: FactoryJobKind, ID: "1", Factory: "f"}); err != nil { + t.Fatal(err) + } + var collectionSeq, memberSeq int64 + if err := s.db.QueryRow("SELECT seq FROM collections WHERE id=?", c.ID).Scan(&collectionSeq); err != nil { + t.Fatal(err) + } + if err := s.db.QueryRow("SELECT seq FROM memberships WHERE collection_id=?", c.ID).Scan(&memberSeq); err != nil { + t.Fatal(err) + } + if collectionSeq <= 100 || memberSeq <= 200 { + t.Fatalf("reused retired sequences: collection %d member %d", collectionSeq, memberSeq) + } + }) + } +} diff --git a/internal/workspace/schema7.go b/internal/workspace/schema7.go new file mode 100644 index 0000000000..404c0d7ef9 --- /dev/null +++ b/internal/workspace/schema7.go @@ -0,0 +1,32 @@ +package workspace + +// schemaSevenCleanup removes tables left by development builds (schemas 3–6), +// including their indexes and triggers. +const schemaSevenCleanup = ` +DROP TRIGGER IF EXISTS workstream_collection_keep; +DROP TRIGGER IF EXISTS workstream_job_promote; +DROP TRIGGER IF EXISTS workstream_job_update; +DROP TRIGGER IF EXISTS workstream_job_insert; +DROP INDEX IF EXISTS workstream_suggestion_page; +DROP INDEX IF EXISTS workstream_sources_order; +DROP INDEX IF EXISTS workstream_wake_item_list; +DROP INDEX IF EXISTS workstream_wake_list; +DROP INDEX IF EXISTS workstream_goahead_list; +DROP INDEX IF EXISTS workstream_record_list; +DROP INDEX IF EXISTS workstream_overview; +DROP INDEX IF EXISTS workstream_page_tree; +DROP INDEX IF EXISTS workstream_job_owner; +DROP INDEX IF EXISTS workstreams_client; +DROP TABLE IF EXISTS workstream_suggestion_edits; +DROP TABLE IF EXISTS workstream_suggestions; +DROP TABLE IF EXISTS workstream_sources; +DROP TABLE IF EXISTS workstream_wake_items; +DROP TABLE IF EXISTS workstream_wakes; +DROP TABLE IF EXISTS workstream_plans; +DROP TABLE IF EXISTS workstream_goaheads; +DROP TABLE IF EXISTS workstream_records; +DROP TABLE IF EXISTS workstream_page_versions; +DROP TABLE IF EXISTS workstream_pages; +DROP TABLE IF EXISTS workstream_jobs; +DROP TABLE IF EXISTS workstreams; +` diff --git a/internal/workspace/schema7_test.go b/internal/workspace/schema7_test.go new file mode 100644 index 0000000000..af5dfed7fa --- /dev/null +++ b/internal/workspace/schema7_test.go @@ -0,0 +1,574 @@ +package workspace + +import ( + "context" + "database/sql" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/sqlitefile" +) + +// Contract (c): frozen SQL from the schema-6 writer and the schema-2 merge base. +// These fixtures deliberately do not call the current schema builders. +const schemaSevenTwoDDL = ` +CREATE TABLE collections ( + seq INTEGER PRIMARY KEY AUTOINCREMENT, + id TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + created_at INTEGER NOT NULL DEFAULT 0 +); +CREATE TABLE memberships ( + seq INTEGER PRIMARY KEY AUTOINCREMENT, + collection_id TEXT NOT NULL REFERENCES collections(id) ON DELETE CASCADE, + kind TEXT NOT NULL CHECK(kind IN ('collection','conversation','task','standing','artifact','crew','process')), + ref_id TEXT NOT NULL, + session_id TEXT NOT NULL, + target_collection TEXT REFERENCES collections(id) ON DELETE CASCADE, + added_at INTEGER NOT NULL DEFAULT 0, + CHECK ((kind='collection' AND target_collection IS NOT NULL AND target_collection=ref_id) + OR (kind!='collection' AND target_collection IS NULL)), + UNIQUE(collection_id,kind,ref_id,session_id) +); + +CREATE INDEX memberships_reference ON memberships(kind,ref_id,session_id); +CREATE INDEX memberships_target ON memberships(target_collection); +` + +const schemaSevenCoreDDL = ` +CREATE TABLE collections ( + seq INTEGER PRIMARY KEY AUTOINCREMENT, + id TEXT NOT NULL UNIQUE, + name TEXT NOT NULL, + created_at INTEGER NOT NULL DEFAULT 0 +); +CREATE TABLE memberships ( + seq INTEGER PRIMARY KEY AUTOINCREMENT, + collection_id TEXT NOT NULL REFERENCES collections(id) ON DELETE CASCADE, + kind TEXT NOT NULL CHECK(kind IN ('collection','conversation','task','standing','artifact','crew','process','factory_job')), + ref_id TEXT NOT NULL, + session_id TEXT NOT NULL, + target_collection TEXT REFERENCES collections(id) ON DELETE CASCADE, + added_at INTEGER NOT NULL DEFAULT 0, + factory TEXT NOT NULL DEFAULT '', + CHECK ((kind='factory_job' AND length(factory)>0) OR (kind!='factory_job' AND factory='')), + CHECK ((kind='collection' AND target_collection IS NOT NULL AND target_collection=ref_id) + OR (kind!='collection' AND target_collection IS NULL)), + UNIQUE(collection_id,kind,ref_id,session_id,factory) +); + +CREATE INDEX memberships_reference ON memberships(kind,ref_id,session_id,factory); +CREATE INDEX memberships_target ON memberships(target_collection); + +` + +const schemaSevenThreeDDL = `CREATE TABLE workstreams ( + id TEXT PRIMARY KEY REFERENCES collections(id) ON DELETE CASCADE, + why TEXT NOT NULL DEFAULT '', origin_chat_id TEXT NOT NULL DEFAULT '', agent_id TEXT NOT NULL, + autonomy TEXT NOT NULL DEFAULT 'propose' CHECK(autonomy IN ('propose','small','off')), + wake_morning INTEGER NOT NULL DEFAULT 0, wake_related_issues INTEGER NOT NULL DEFAULT 0, + budget_usd REAL CHECK(budget_usd IS NULL OR budget_usd>0), + created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL, started_at INTEGER, done_at INTEGER, + how_it_went TEXT NOT NULL DEFAULT '', client_id TEXT NOT NULL DEFAULT '' +); +CREATE UNIQUE INDEX workstreams_client ON workstreams(client_id) WHERE client_id!=''; +CREATE TABLE workstream_jobs ( + member_seq INTEGER PRIMARY KEY REFERENCES memberships(seq) ON DELETE CASCADE, + workstream_id TEXT NOT NULL REFERENCES workstreams(id) ON DELETE CASCADE, + factory TEXT NOT NULL, job TEXT NOT NULL +); +CREATE UNIQUE INDEX workstream_job_owner ON workstream_jobs(factory,job); +CREATE TRIGGER workstream_job_insert AFTER INSERT ON memberships + WHEN NEW.kind='factory_job' AND EXISTS(SELECT 1 FROM workstreams WHERE id=NEW.collection_id) + BEGIN INSERT INTO workstream_jobs VALUES(NEW.seq,NEW.collection_id,NEW.factory,NEW.ref_id); END; +CREATE TRIGGER workstream_job_update AFTER UPDATE ON memberships + BEGIN + DELETE FROM workstream_jobs WHERE member_seq=OLD.seq; + INSERT INTO workstream_jobs SELECT NEW.seq,NEW.collection_id,NEW.factory,NEW.ref_id + WHERE NEW.kind='factory_job' AND EXISTS(SELECT 1 FROM workstreams WHERE id=NEW.collection_id); + END; +CREATE TRIGGER workstream_job_promote AFTER INSERT ON workstreams + BEGIN INSERT INTO workstream_jobs SELECT seq,collection_id,factory,ref_id FROM memberships + WHERE collection_id=NEW.id AND kind='factory_job'; END; +CREATE TABLE workstream_pages ( + id TEXT PRIMARY KEY, workstream_id TEXT NOT NULL REFERENCES workstreams(id) ON DELETE CASCADE, + parent_id TEXT REFERENCES workstream_pages(id) ON DELETE CASCADE, + title TEXT NOT NULL, owner TEXT NOT NULL CHECK(owner IN ('you','agent')), + read_by_jobs INTEGER NOT NULL DEFAULT 0, position INTEGER NOT NULL CHECK(position>=0), + version INTEGER NOT NULL CHECK(version>0), updated_at INTEGER NOT NULL, + updated_by TEXT NOT NULL CHECK(updated_by IN ('you','agent')), updated_from TEXT, + doc TEXT NOT NULL, overview INTEGER NOT NULL DEFAULT 0 CHECK(overview IN (0,1)), + deleted_at INTEGER, deleted_root TEXT +); +CREATE INDEX workstream_page_tree ON workstream_pages(workstream_id,parent_id,position); +CREATE UNIQUE INDEX workstream_overview ON workstream_pages(workstream_id) WHERE overview=1; +CREATE TABLE workstream_page_versions ( + page_id TEXT NOT NULL REFERENCES workstream_pages(id) ON DELETE CASCADE, + version INTEGER NOT NULL, saved_at INTEGER NOT NULL, saved_by TEXT NOT NULL CHECK(saved_by IN ('you','agent')), + source TEXT, title TEXT NOT NULL, doc TEXT NOT NULL, bytes INTEGER NOT NULL, + PRIMARY KEY(page_id,version) +); +CREATE TABLE workstream_records ( + id TEXT PRIMARY KEY, workstream_id TEXT NOT NULL REFERENCES workstreams(id) ON DELETE CASCADE, + kind TEXT NOT NULL CHECK(kind IN ('decision','question','goahead')), text TEXT NOT NULL, + state TEXT NOT NULL CHECK(state IN ('proposed','kept','dropped','superseded')), + actor TEXT NOT NULL CHECK(actor IN ('you','agent')), source TEXT, created_at INTEGER NOT NULL, + decided_at INTEGER, supersedes TEXT REFERENCES workstream_records(id), superseded_by TEXT REFERENCES workstream_records(id), + goahead TEXT, goahead_id TEXT +); +CREATE INDEX workstream_record_list ON workstream_records(workstream_id,created_at); +CREATE TABLE workstream_goaheads ( + id TEXT PRIMARY KEY, workstream_id TEXT NOT NULL REFERENCES workstreams(id) ON DELETE CASCADE, + scope TEXT NOT NULL CHECK(scope IN ('job','wave','milestone')), + factory TEXT, job INTEGER, wave INTEGER, milestone_id TEXT, + cap_usd REAL NOT NULL CHECK(cap_usd>0 AND cap_usd<=10000), ends_at INTEGER NOT NULL, + state TEXT NOT NULL CHECK(state IN ('active','ended')), + end_reason TEXT CHECK(end_reason IN ('expired','revoked','workstream_done')), + granted_at INTEGER NOT NULL, ended_at INTEGER, + CHECK((scope='job' AND factory IS NOT NULL AND length(factory)>0 AND job IS NOT NULL AND job>0 AND wave IS NULL AND milestone_id IS NULL) + OR (scope='wave' AND wave IS NOT NULL AND wave>0 AND factory IS NULL AND job IS NULL AND milestone_id IS NULL) + OR (scope='milestone' AND milestone_id IS NOT NULL AND length(milestone_id)>0 AND factory IS NULL AND job IS NULL AND wave IS NULL)), + CHECK(ends_at>granted_at AND ends_at<=granted_at+7776000000) +); +CREATE INDEX workstream_goahead_list ON workstream_goaheads(workstream_id,state); +CREATE TABLE workstream_plans ( + workstream_id TEXT PRIMARY KEY REFERENCES workstreams(id) ON DELETE CASCADE, + version INTEGER NOT NULL CHECK(version>0), updated_at INTEGER NOT NULL, + updated_by TEXT NOT NULL CHECK(updated_by IN ('you','agent')), + milestones TEXT NOT NULL, waves TEXT NOT NULL +); + +CREATE TRIGGER IF NOT EXISTS workstream_collection_keep BEFORE DELETE ON collections + WHEN EXISTS(SELECT 1 FROM workstreams WHERE id=OLD.id) + BEGIN SELECT RAISE(ABORT,'mark a workstream done instead'); END; + +` + +const schemaSevenFourDDL = `CREATE TABLE workstream_wakes ( + id TEXT PRIMARY KEY, workstream_id TEXT NOT NULL REFERENCES workstreams(id) ON DELETE CASCADE, + event_key TEXT NOT NULL, event_json TEXT NOT NULL, + state TEXT NOT NULL CHECK(state IN ('running','settled','failed')), + summary TEXT NOT NULL DEFAULT '', error TEXT NOT NULL DEFAULT '', cost_usd REAL, + started_at INTEGER NOT NULL, settled_at INTEGER, dismissed_at INTEGER, + UNIQUE(workstream_id,event_key) +); +CREATE INDEX workstream_wake_list ON workstream_wakes(workstream_id,started_at); +CREATE TABLE workstream_wake_items ( + id TEXT PRIMARY KEY, wake_id TEXT NOT NULL REFERENCES workstream_wakes(id) ON DELETE CASCADE, + position INTEGER NOT NULL, kind TEXT NOT NULL CHECK(kind IN ('move','draft','page','decision','question')), + state TEXT NOT NULL CHECK(state IN ('applied','undone','proposed','dismissed')), + why TEXT NOT NULL DEFAULT '', item_json TEXT NOT NULL, undo_json TEXT, + note TEXT NOT NULL DEFAULT '', applied_at INTEGER, undone_at INTEGER +); +CREATE INDEX workstream_wake_item_list ON workstream_wake_items(wake_id,position); + +` + +const schemaSevenFiveDDL = `CREATE TABLE workstream_sources ( + seq INTEGER PRIMARY KEY AUTOINCREMENT, + id TEXT NOT NULL UNIQUE, + workstream_id TEXT NOT NULL REFERENCES workstreams(id) ON DELETE CASCADE, + kind TEXT NOT NULL CHECK(kind IN ('file','repo','issue','link','kept')), + target TEXT NOT NULL, + title TEXT NOT NULL, + path TEXT, repo TEXT, number INTEGER, url TEXT, ref TEXT, why TEXT, + read_by_jobs INTEGER NOT NULL DEFAULT 0, + proposed INTEGER NOT NULL DEFAULT 0, + added_by TEXT NOT NULL CHECK(added_by IN ('you','agent')), + added_at INTEGER NOT NULL, + UNIQUE(workstream_id,kind,target) +); +CREATE INDEX workstream_sources_order ON workstream_sources(workstream_id,seq); + +` + +const schemaSevenSixDDL = `CREATE TABLE workstream_suggestions ( + id TEXT PRIMARY KEY, workstream_id TEXT NOT NULL REFERENCES workstreams(id) ON DELETE CASCADE, + page_id TEXT NOT NULL REFERENCES workstream_pages(id) ON DELETE CASCADE, + base_version INTEGER NOT NULL, why TEXT NOT NULL, actor TEXT NOT NULL, source TEXT, + created_at INTEGER NOT NULL +); +CREATE INDEX workstream_suggestion_page ON workstream_suggestions(page_id,created_at); +CREATE TABLE workstream_suggestion_edits ( + suggestion_id TEXT NOT NULL REFERENCES workstream_suggestions(id) ON DELETE CASCADE, + id TEXT NOT NULL, position INTEGER NOT NULL, block TEXT NOT NULL, at_offset INTEGER NOT NULL, + del TEXT NOT NULL, ins TEXT NOT NULL, block_text TEXT NOT NULL, why TEXT NOT NULL, + state TEXT NOT NULL CHECK(state IN ('proposed','kept','dropped')), decided_at INTEGER, + PRIMARY KEY(suggestion_id,id) +); +` + +const schemaSevenRows = ` +INSERT INTO collections(seq,id,name,created_at) VALUES + (4,'root','Root',1234),(9,'nested','Nested',2345),(12,'former','Former owner',3456); +INSERT INTO memberships(seq,collection_id,kind,ref_id,session_id,target_collection,added_at) VALUES + (2,'root','conversation','chat','',NULL,4567), + (6,'root','task','3','chat',NULL,5678), + (11,'root','collection','nested','','nested',6789), + (15,'nested','collection','former','','former',7000), + (16,'nested','crew','crew','',NULL,7100), + (17,'nested','process','chat:p1','chat',NULL,7200), + (18,'former','conversation','agent','',NULL,7300), + (19,'nested','artifact','/tmp/report.md','',NULL,7400), + (20,'nested','standing','retired','',NULL,7500); +INSERT INTO collections(seq,id,name) VALUES (100,'gone','Deleted'); +DELETE FROM collections WHERE id='gone'; +INSERT INTO memberships(seq,collection_id,kind,ref_id,session_id) VALUES (200,'root','conversation','gone',''); +DELETE FROM memberships WHERE ref_id='gone'; +` + +const schemaSevenThreeRows = ` +INSERT INTO workstreams(id,agent_id,created_at,updated_at,client_id) VALUES ('former','agent',3456,4000,'client'); +INSERT INTO memberships(seq,collection_id,kind,ref_id,session_id,added_at,factory) VALUES + (21,'former','factory_job','7','',7600,'factory-a'),(22,'root','factory_job','7','',7700,'factory-a'); +INSERT INTO workstream_pages(id,workstream_id,title,owner,position,version,updated_at,updated_by,doc,overview) + VALUES ('page','former','Overview','you',0,1,4000,'you','{}',1); +INSERT INTO workstream_page_versions(page_id,version,saved_at,saved_by,title,doc,bytes) + VALUES ('page',1,4000,'you','Overview','{}',2); +INSERT INTO workstream_records(id,workstream_id,kind,text,state,actor,created_at) + VALUES ('record','former','decision','Keep','kept','you',4000); +INSERT INTO workstream_goaheads(id,workstream_id,scope,wave,cap_usd,ends_at,state,granted_at) + VALUES ('grant','former','wave',1,1,5000,'active',4000); +INSERT INTO workstream_plans(workstream_id,version,updated_at,updated_by,milestones,waves) + VALUES ('former',1,4000,'you','[]','[]'); +` +const schemaSevenFourRows = ` +INSERT INTO workstream_wakes(id,workstream_id,event_key,event_json,state,started_at) + VALUES ('wake','former','morning:2026-11-03','{}','settled',4000); +INSERT INTO workstream_wake_items(id,wake_id,position,kind,state,item_json) + VALUES ('item','wake',0,'page','applied','{}'); +` +const schemaSevenFiveRows = ` +INSERT INTO workstream_sources(id,workstream_id,kind,target,title,added_by,added_at) + VALUES ('source','former','link','https://example.com','Example','you',4000); +` +const schemaSevenSixRows = ` +INSERT INTO workstream_suggestions(id,workstream_id,page_id,base_version,why,actor,created_at) + VALUES ('suggestion','former','page',1,'Clarify','agent',4000); +INSERT INTO workstream_suggestion_edits(suggestion_id,id,position,block,at_offset,del,ins,block_text,why,state) + VALUES ('suggestion','edit',0,'[0]',0,'Old','New','Old','Clearer','proposed'); +` + +func schemaSevenFixture(t *testing.T, version int) string { + t.Helper() + ddl, rows := schemaSevenTwoDDL, schemaSevenRows + if version >= 3 { + ddl = schemaSevenCoreDDL + schemaSevenThreeDDL + rows += schemaSevenThreeRows + } + if version >= 4 { + ddl += schemaSevenFourDDL + rows += schemaSevenFourRows + } + if version >= 5 { + ddl += schemaSevenFiveDDL + rows += schemaSevenFiveRows + } + if version >= 6 { + ddl += schemaSevenSixDDL + rows += schemaSevenSixRows + } + path := filepath.Join(t.TempDir(), "collections.db") + db, err := sql.Open("sqlite", sqlitefile.URI(path, "_pragma=foreign_keys(1)")) + if err != nil { + t.Fatal(err) + } + defer db.Close() + if _, err := db.Exec(ddl + rows + fmt.Sprintf("PRAGMA application_id=%d; PRAGMA user_version=%d;", applicationID, version)); err != nil { + t.Fatal(err) + } + return path +} + +func schemaSevenFile(t *testing.T, path string) []byte { + t.Helper() + b, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return b +} + +func schemaSevenQuery(t *testing.T, db *sql.DB, query string) [][]any { + t.Helper() + rows, err := db.Query(query) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + columns, err := rows.Columns() + if err != nil { + t.Fatal(err) + } + var out [][]any + for rows.Next() { + values, pointers := make([]any, len(columns)), make([]any, len(columns)) + for i := range values { + pointers[i] = &values[i] + } + if err := rows.Scan(pointers...); err != nil { + t.Fatal(err) + } + out = append(out, values) + } + if err := rows.Err(); err != nil { + t.Fatal(err) + } + return out +} + +func schemaSevenSchema(t *testing.T, s *Store) [][]any { + t.Helper() + schema := schemaSevenQuery(t, s.db, "SELECT type,name,tbl_name,sql FROM sqlite_master ORDER BY type,name") + // ALTER TABLE quotes renamed table identifiers; the logical shape is identical. + for _, row := range schema { + if text, ok := row[3].(string); ok { + row[3] = strings.ReplaceAll(text, `"`, "") + } + } + return schema +} + +func schemaSevenSequences(t *testing.T, s *Store) [][]any { + t.Helper() + return schemaSevenQuery(t, s.db, "SELECT name,seq FROM sqlite_sequence WHERE name IN ('collections','memberships') ORDER BY name") +} + +// Contract (c), G2: every supported predecessor reads without writing, migrates +// on the requested successful write, preserves every row and cursor high-water +// mark, permits former-owner deletion, and performs no DDL on subsequent writes. +func TestSchemaSevenMigratesAndPreservesGenericData(t *testing.T) { + for _, version := range []int{2, 3, 4, 5, 6} { + t.Run(fmt.Sprint(version), func(t *testing.T) { + ctx := context.Background() + path := schemaSevenFixture(t, version) + before := schemaSevenFile(t, path) + s := openTestStore(t, path) + wantCollections := schemaSevenQuery(t, s.db, "SELECT seq,id,name,created_at FROM collections ORDER BY seq") + projection := "seq,collection_id,kind,ref_id,session_id,target_collection,added_at,''" + if version >= 3 { + projection = "seq,collection_id,kind,ref_id,session_id,target_collection,added_at,factory" + } + wantMembers := schemaSevenQuery(t, s.db, "SELECT "+projection+" FROM memberships ORDER BY seq") + wantSequences := schemaSevenSequences(t, s) + if !reflect.DeepEqual(wantSequences, [][]any{{"collections", int64(100)}, {"memberships", int64(200)}}) { + t.Fatalf("fixture lacks deleted-row gaps: %v", wantSequences) + } + if version == 6 { + // Every frozen legacy table contains data; cleanup is exercised with foreign keys on. + for _, row := range schemaSevenQuery(t, s.db, "SELECT name FROM sqlite_master WHERE type='table' AND name NOT IN ('collections','memberships','sqlite_sequence')") { + count := schemaSevenQuery(t, s.db, "SELECT count(*) FROM "+row[0].(string)) + if count[0][0].(int64) == 0 { + t.Fatalf("empty fixture table: %s", row[0]) + } + } + } + collections, err := s.Collections(ctx) + if err != nil || len(collections) != 3 { + t.Fatalf("collections: %v, %v", collections, err) + } + summaries, err := s.List(ctx, nil) + if err != nil || len(summaries) != 3 || summaries[0].CreatedAt != 1234 { + t.Fatalf("summaries: %+v, %v", summaries, err) + } + wantRefs := map[string][]Ref{} + wantTrees := map[string]Tree{} + for _, c := range collections { + refs, err := s.Members(ctx, c.ID) + if err != nil { + t.Fatal(err) + } + wantRefs[c.ID] = refs + tree, err := s.Tree(ctx, c.ID, 3, 200, "") + if err != nil { + t.Fatal(err) + } + wantTrees[c.ID] = tree + expected := []Ref{} + for _, row := range wantMembers { + if row[1] == c.ID { + expected = append(expected, Ref{Kind: Kind(row[2].(string)), ID: row[3].(string), SessionID: row[4].(string), Factory: row[7].(string)}) + } + } + if !reflect.DeepEqual(refs, expected) { + t.Fatalf("read refs: %v, want %v", refs, expected) + } + } + if version >= 3 { + parents, err := s.CollectionsFor(ctx, Ref{Kind: FactoryJobKind, ID: "7", Factory: "factory-a"}) + if err != nil || !reflect.DeepEqual(parents, []Collection{{ID: "root", Name: "Root"}, {ID: "former", Name: "Former owner"}}) { + t.Fatalf("shared job: %v, %v", parents, err) + } + } + if userVersion(t, s) != version || !reflect.DeepEqual(schemaSevenFile(t, path), before) { + t.Fatal("open/read modified predecessor") + } + // Renaming to the existing name is still a successful public write and + // lets us compare every original row without editing its expected values. + if err := s.Rename(ctx, "root", "Root"); err != nil { + t.Fatal(err) + } + if userVersion(t, s) != 7 { + t.Fatal("first write did not install schema 7") + } + if got := schemaSevenQuery(t, s.db, "SELECT seq,id,name,created_at FROM collections ORDER BY seq"); !reflect.DeepEqual(got, wantCollections) { + t.Fatalf("collections changed: %v", got) + } + if got := schemaSevenQuery(t, s.db, "SELECT seq,collection_id,kind,ref_id,session_id,target_collection,added_at,factory FROM memberships ORDER BY seq"); !reflect.DeepEqual(got, wantMembers) { + t.Fatalf("memberships changed: %v", got) + } + if got := schemaSevenSequences(t, s); !reflect.DeepEqual(got, wantSequences) { + t.Fatalf("high-water marks changed: %v", got) + } + if got, err := s.List(ctx, nil); err != nil || !reflect.DeepEqual(got, summaries) { + t.Fatalf("summaries changed: %v, %v", got, err) + } + for _, c := range collections { + if got, err := s.Members(ctx, c.ID); err != nil || !reflect.DeepEqual(got, wantRefs[c.ID]) { + t.Fatalf("refs changed: %v, %v", got, err) + } + if got, err := s.Tree(ctx, c.ID, 3, 200, ""); err != nil || !reflect.DeepEqual(got, wantTrees[c.ID]) { + t.Fatalf("tree changed: %v, %v", got, err) + } + } + fresh := openTestStore(t, filepath.Join(t.TempDir(), "fresh.db")) + if err := fresh.ensureSchema(ctx); err != nil { + t.Fatal(err) + } + if userVersion(t, fresh) != 7 { + t.Fatal("blank file did not initialize as schema 7") + } + if got, want := schemaSevenSchema(t, s), schemaSevenSchema(t, fresh); !reflect.DeepEqual(got, want) { + t.Fatalf("migration left legacy objects or wrong schema: %v; want %v", got, want) + } + schema := schemaSevenSchema(t, s) + var schemaCookie int + if err := s.db.QueryRow("PRAGMA schema_version").Scan(&schemaCookie); err != nil { + t.Fatal(err) + } + if err := s.Rename(ctx, "root", "Root again"); err != nil { + t.Fatal(err) + } + var afterCookie int + if err := s.db.QueryRow("PRAGMA schema_version").Scan(&afterCookie); err != nil { + t.Fatal(err) + } + if got := schemaSevenSchema(t, s); !reflect.DeepEqual(got, schema) || afterCookie != schemaCookie { + t.Fatal("second write ran DDL") + } + // IF EXISTS makes the predecessor cleanup safe when repeated or partially absent. + if _, err := s.db.Exec(schemaSevenCleanup); err != nil { + t.Fatal(err) + } + if got := schemaSevenSchema(t, s); !reflect.DeepEqual(got, schema) { + t.Fatal("repeated cleanup changed schema") + } + if _, err := s.Delete(ctx, "former"); err != nil { + t.Fatalf("former owner is not deletable: %v", err) + } + c, err := s.Create(ctx, "After") + if err != nil { + t.Fatal(err) + } + if err := s.Add(ctx, c.ID, Ref{Kind: FactoryJobKind, ID: "7", Factory: "factory-a"}); err != nil { + t.Fatal(err) + } + var collectionSeq, memberSeq int + if err := s.db.QueryRow("SELECT seq FROM collections WHERE id=?", c.ID).Scan(&collectionSeq); err != nil { + t.Fatal(err) + } + if err := s.db.QueryRow("SELECT seq FROM memberships WHERE collection_id=?", c.ID).Scan(&memberSeq); err != nil { + t.Fatal(err) + } + if collectionSeq <= 100 || memberSeq <= 200 { + t.Fatalf("reused retired sequences: %d, %d", collectionSeq, memberSeq) + } + }) + } +} + +// Contract (c): a failed requested write rolls back DDL, data and version bytes. +func TestSchemaSevenFailedFirstWriteLeavesPredecessorUntouched(t *testing.T) { + for _, version := range []int{2, 6} { + t.Run(fmt.Sprint(version), func(t *testing.T) { + path := schemaSevenFixture(t, version) + before := schemaSevenFile(t, path) + s := openTestStore(t, path) + schema := schemaSevenSchema(t, s) + if err := s.Rename(context.Background(), "root", "Nested"); err != ErrNameTaken { + t.Fatalf("expected name conflict: %v", err) + } + if userVersion(t, s) != version || !reflect.DeepEqual(schemaSevenFile(t, path), before) || !reflect.DeepEqual(schemaSevenSchema(t, s), schema) { + t.Fatal("failed write migrated predecessor") + } + }) + } +} + +// Contract (c): future versions and foreign application IDs are refused without +// modifying the file, including a file that changed after this handle opened. +func TestSchemaSevenRefusesUnknownDatabases(t *testing.T) { + for _, pragma := range []string{"PRAGMA user_version=8", "PRAGMA application_id=12345"} { + t.Run(pragma, func(t *testing.T) { + path := schemaSevenFixture(t, 6) + s := openTestStore(t, path) + if _, err := s.db.Exec(pragma); err != nil { + t.Fatal(err) + } + before := schemaSevenFile(t, path) + opened, err := Open(path) + if opened != nil { + opened.Close() + } + if err == nil || !strings.Contains(err.Error(), "unsupported collections database") { + t.Fatalf("open accepted unknown database: %v", err) + } + if _, err := s.Collections(context.Background()); err == nil || !strings.Contains(err.Error(), "unsupported collections database") { + t.Fatalf("read accepted unknown database: %v", err) + } + if err := s.Rename(context.Background(), "root", "Changed"); err == nil || !strings.Contains(err.Error(), "unsupported collections database") { + t.Fatalf("write accepted unknown database: %v", err) + } + if !reflect.DeepEqual(schemaSevenFile(t, path), before) { + t.Fatal("refusal changed file") + } + }) + } +} + +// Contract (c): schemas 3–6 are recognized by the generic tables alone. +func TestSchemaSevenReadsCleanPredecessorShapes(t *testing.T) { + for _, version := range []int{3, 4, 5, 6} { + t.Run(fmt.Sprint(version), func(t *testing.T) { + path := filepath.Join(t.TempDir(), "clean.db") + db, err := sql.Open("sqlite", path) + if err != nil { + t.Fatal(err) + } + if _, err := db.Exec(schemaSevenCoreDDL + schemaSevenRows + fmt.Sprintf("PRAGMA application_id=%d; PRAGMA user_version=%d;", applicationID, version)); err != nil { + t.Fatal(err) + } + if err := db.Close(); err != nil { + t.Fatal(err) + } + before := schemaSevenFile(t, path) + s := openTestStore(t, path) + if got, err := s.Members(context.Background(), "root"); err != nil || len(got) != 3 { + t.Fatalf("clean predecessor: %v, %v", got, err) + } + if !reflect.DeepEqual(schemaSevenFile(t, path), before) { + t.Fatal("read changed clean predecessor") + } + if err := s.Rename(context.Background(), "root", "Root"); err != nil { + t.Fatal(err) + } + if userVersion(t, s) != 7 { + t.Fatal("clean predecessor not upgraded") + } + }) + } +} diff --git a/internal/workspace/store.go b/internal/workspace/store.go index 6319953420..5c28fe4372 100644 --- a/internal/workspace/store.go +++ b/internal/workspace/store.go @@ -20,13 +20,10 @@ import ( sqlite3 "modernc.org/sqlite/lib" ) -// schemaVersion is the schema this build writes. Schema 2 added when a -// collection was made and when each membership was added, the crew and -// process kinds, and deletes that cascade. legacySchemaVersion is the one -// older schema this build still reads as it is and migrates on its first -// write; anything else claiming to be ours is refused, never misread. +// schemaVersion is the schema this build writes. Earlier schemas remain +// readable without migration; the first successful write upgrades them. const ( - schemaVersion = 2 + schemaVersion = 7 legacySchemaVersion = 1 ) const applicationID = 0x4146434c // AFCL distinguishes this store from optional memory. @@ -40,19 +37,17 @@ const busyTimeout = 10 * time.Second type schemaState uint8 const ( - schemaBlank schemaState = iota - schemaReady - schemaLegacy - schemaRefused + schemaBlank schemaState = 0 + schemaLegacy schemaState = 1 + schemaTwo schemaState = 2 + schemaReady schemaState = schemaVersion + schemaRefused schemaState = schemaVersion + 1 ) -// version is the schema a state reads through: 0 for a blank file. +// version is the schema a state reads through: 0 for a blank or refused file. func (s schemaState) version() int { - switch s { - case schemaReady: - return schemaVersion - case schemaLegacy: - return legacySchemaVersion + if s < schemaRefused { + return int(s) } return 0 } @@ -66,8 +61,8 @@ func (s schemaState) version() int { // lock to do it — which is what put a plain read behind whichever peer happened // to be adding a membership. `version` is that question already answered (0 // while the file is blank), and `schemaMu` keeps two goroutines on one handle -// from both answering it. The same law covers migration: a schema-1 file is -// read as it stands and rebuilt only by the first write. +// from both answering it. The same law covers migration: older supported files +// are read as they stand and upgraded only by the first write. type Store struct { db *sql.DB schemaMu sync.Mutex @@ -161,7 +156,7 @@ func (s *Store) snapshotSchema(ctx context.Context) (schemaState, error) { } // inspectSchema decides whether the database on the other end of q is ours and -// built (at this schema or the legacy one), ours and still blank, or somebody +// built at a supported schema, ours and still blank, or somebody // else's. It only ever reads; the caller decides what view it reads through. func inspectSchema(ctx context.Context, q schemaQuerier) (schemaState, error) { if err := ctx.Err(); err != nil { @@ -174,17 +169,18 @@ func inspectSchema(ctx context.Context, q schemaQuerier) (schemaState, error) { if err := q.QueryRowContext(ctx, "PRAGMA user_version").Scan(&version); err != nil { return schemaBlank, err } - if app == applicationID && (version == schemaVersion || version == legacySchemaVersion) { - state := schemaReady + if app == applicationID && version >= legacySchemaVersion && version <= schemaVersion { + state := schemaState(version) collections := "SELECT seq,id,name,created_at FROM collections LIMIT 0" memberships := "SELECT seq,collection_id,kind,ref_id,session_id,target_collection,added_at FROM memberships LIMIT 0" + if version >= 3 { + memberships = "SELECT seq,collection_id,kind,ref_id,session_id,target_collection,added_at,factory FROM memberships LIMIT 0" + } if version == legacySchemaVersion { - state = schemaLegacy collections = "SELECT seq,id,name FROM collections LIMIT 0" memberships = "SELECT seq,collection_id,kind,ref_id,session_id,target_collection FROM memberships LIMIT 0" } - // Preparing the actual reads catches missing tables before reporting a - // successful open. We never recreate a damaged initialized schema. + // Only the generic tables define a supported file's readable shape. if _, err := q.ExecContext(ctx, collections); err != nil { return state, err } @@ -205,7 +201,7 @@ func inspectSchema(ctx context.Context, q schemaQuerier) (schemaState, error) { return schemaBlank, nil } -// schemaTables is schema 2's two tables under the names given, so the same +// schemaTables builds the collection tables under the names given, so the same // words build a fresh store and the copies a migration fills. // // EVERY DELETE CASCADES: a deleted collection takes its own memberships and @@ -222,25 +218,27 @@ CREATE TABLE %[1]s ( CREATE TABLE %[2]s ( seq INTEGER PRIMARY KEY AUTOINCREMENT, collection_id TEXT NOT NULL REFERENCES %[1]s(id) ON DELETE CASCADE, - kind TEXT NOT NULL CHECK(kind IN ('collection','conversation','task','standing','artifact','crew','process')), + kind TEXT NOT NULL CHECK(kind IN ('collection','conversation','task','standing','artifact','crew','process','factory_job')), ref_id TEXT NOT NULL, session_id TEXT NOT NULL, target_collection TEXT REFERENCES %[1]s(id) ON DELETE CASCADE, added_at INTEGER NOT NULL DEFAULT 0, + factory TEXT NOT NULL DEFAULT '', + CHECK ((kind='factory_job' AND length(factory)>0) OR (kind!='factory_job' AND factory='')), CHECK ((kind='collection' AND target_collection IS NOT NULL AND target_collection=ref_id) OR (kind!='collection' AND target_collection IS NULL)), - UNIQUE(collection_id,kind,ref_id,session_id) + UNIQUE(collection_id,kind,ref_id,session_id,factory) ); `, collections, memberships) } const schemaIndexes = ` -CREATE INDEX memberships_reference ON memberships(kind,ref_id,session_id); +CREATE INDEX memberships_reference ON memberships(kind,ref_id,session_id,factory); CREATE INDEX memberships_target ON memberships(target_collection); ` -// migrateLegacy rebuilds a schema-1 file as schema 2 inside the caller's -// immediate transaction: either every row arrives in the new tables, in the +// migrateLegacy rebuilds a schema-1 or schema-2 file in the current generic +// shape inside the caller's immediate transaction: every row arrives in the // same order under the same sequence numbers, or the file is left exactly as // it was. A row from schema 1 says nothing of when it was made, so its // created_at and added_at are 0, which every reader takes as unknown. @@ -248,68 +246,116 @@ CREATE INDEX memberships_target ON memberships(target_collection); // THE TABLES ARE COPIED, NOT ALTERED, because SQLite cannot change a CHECK or // a foreign key in place. The old memberships go first so dropping the old // collections deletes nothing anything still points at, and renaming the copy -// carries the foreign keys that name it along. -func migrateLegacy(ctx context.Context, tx *sql.Tx) error { - _, err := tx.ExecContext(ctx, schemaTables("collections_v2", "memberships_v2")+` -INSERT INTO collections_v2(seq,id,name,created_at) SELECT seq,id,name,0 FROM collections ORDER BY seq; -INSERT INTO memberships_v2(seq,collection_id,kind,ref_id,session_id,target_collection,added_at) - SELECT seq,collection_id,kind,ref_id,session_id,target_collection,0 FROM memberships ORDER BY seq; +// carries the foreign keys that name it along. Sequence high-water marks also +// survive deleted rows so migration cannot reuse a sequence named by an older +// page cursor. +func migrateLegacy(ctx context.Context, tx *sql.Tx, version int) error { + created, added := "0", "0" + if version >= 2 { + created, added = "created_at", "added_at" + } + _, err := tx.ExecContext(ctx, schemaTables("collections_v3", "memberships_v3")+fmt.Sprintf(` +INSERT INTO collections_v3(seq,id,name,created_at) SELECT seq,id,name,%s FROM collections ORDER BY seq; +INSERT INTO memberships_v3(seq,collection_id,kind,ref_id,session_id,target_collection,added_at) + SELECT seq,collection_id,kind,ref_id,session_id,target_collection,%s FROM memberships ORDER BY seq; +UPDATE sqlite_sequence SET seq=max(seq,coalesce((SELECT seq FROM sqlite_sequence WHERE name='collections'),0)) + WHERE name='collections_v3'; +INSERT INTO sqlite_sequence(name,seq) SELECT 'collections_v3',seq FROM sqlite_sequence + WHERE name='collections' AND NOT EXISTS(SELECT 1 FROM sqlite_sequence WHERE name='collections_v3'); +UPDATE sqlite_sequence SET seq=max(seq,coalesce((SELECT seq FROM sqlite_sequence WHERE name='memberships'),0)) + WHERE name='memberships_v3'; +INSERT INTO sqlite_sequence(name,seq) SELECT 'memberships_v3',seq FROM sqlite_sequence + WHERE name='memberships' AND NOT EXISTS(SELECT 1 FROM sqlite_sequence WHERE name='memberships_v3'); DROP TABLE memberships; DROP TABLE collections; -ALTER TABLE collections_v2 RENAME TO collections; -ALTER TABLE memberships_v2 RENAME TO memberships; -`+schemaIndexes) +ALTER TABLE collections_v3 RENAME TO collections; +ALTER TABLE memberships_v3 RENAME TO memberships; +`, created, added)+schemaIndexes) return err } -// ensureSchema serializes initialization on one handle and then takes SQLite's -// immediate transaction so separate processes also agree which one builds it, -// or which one migrates a schema-1 file to this one. -func (s *Store) ensureSchema(ctx context.Context) error { - if err := ctx.Err(); err != nil { - return storeError(err) - } - s.schemaMu.Lock() - defer s.schemaMu.Unlock() - if s.version == schemaVersion { - return nil +// beginWrite ensures schema and data changes share one immediate transaction. +// A failed first write rolls back the migration as well as the requested change. +func (s *Store) beginWrite(ctx context.Context, create bool) (*sql.Tx, error) { + if !create { + version, err := s.readyForRead(ctx) + if err != nil { + return nil, err + } + if version == 0 { + return nil, ErrNotFound + } } tx, err := s.db.BeginTx(ctx, nil) if err != nil { - return storeError(err) + return nil, storeError(err) } - defer tx.Rollback() state, err := inspectSchema(ctx, tx) + if err == nil { + switch { + case state == schemaReady: + // A current file needs no DDL. + case state == schemaLegacy || state == schemaTwo: + err = migrateLegacy(ctx, tx, state.version()) + case state >= 3 && state < schemaReady: + _, err = tx.ExecContext(ctx, schemaSevenCleanup) + case state == schemaBlank: + if !create { + err = ErrNotFound + } else { + _, err = tx.ExecContext(ctx, schemaTables("collections", "memberships")+schemaIndexes) + } + } + } + if err == nil && state != schemaReady { + _, err = tx.ExecContext(ctx, fmt.Sprintf("PRAGMA application_id=%d; PRAGMA user_version=%d", applicationID, schemaVersion)) + } if err != nil { - return storeError(err) + _ = tx.Rollback() + return nil, storeError(err) } - switch state { - case schemaReady: - if err := tx.Commit(); err != nil { - return storeError(err) - } - s.version = schemaVersion - return nil - case schemaLegacy: - err = migrateLegacy(ctx, tx) - default: - _, err = tx.ExecContext(ctx, schemaTables("collections", "memberships")+schemaIndexes) + return tx, nil +} + +// ensureSchema is used by bulk fixtures that initialize without adding rows. +func (s *Store) ensureSchema(ctx context.Context) error { + tx, err := s.beginWrite(ctx, true) + if err != nil { + return err } + defer tx.Rollback() + return storeError(tx.Commit()) +} + +// Read runs fn against a consistent read-only snapshot. Version 0 means blank. +// Reads never initialize or migrate the database. +func (s *Store) Read(ctx context.Context, fn func(tx *sql.Tx, version int) error) error { + tx, version, err := s.beginRead(ctx) if err != nil { - return storeError(err) + return err } - if _, err := tx.ExecContext(ctx, fmt.Sprintf("PRAGMA application_id=%d; PRAGMA user_version=%d", applicationID, schemaVersion)); err != nil { - return storeError(err) + defer tx.Rollback() + return storeError(fn(tx, version)) +} + +// beginRead keeps the schema decision and projected rows in one snapshot. A +// peer may migrate between separate transactions, adding fields an old projection +// cannot supply (notably factory on a new job ref). +func (s *Store) beginRead(ctx context.Context) (*sql.Tx, int, error) { + tx, err := s.db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true}) + if err != nil { + return nil, 0, storeError(err) } - if err := tx.Commit(); err != nil { - return storeError(err) + state, err := inspectSchema(ctx, tx) + if err != nil { + _ = tx.Rollback() + return nil, 0, storeError(err) } - s.version = schemaVersion - return nil + return tx, state.version(), nil } // readyForRead answers the schema a read goes through, and 0 for a blank -// file. A handle that saw a blank or a schema-1 file looks again, because +// file. A handle that saw a blank or an older schema looks again, because // another process may have initialized or migrated it since. The look is // still only plain reads. func (s *Store) readyForRead(ctx context.Context) (int, error) { @@ -329,26 +375,10 @@ func (s *Store) readyForRead(ctx context.Context) (int, error) { return s.version, nil } -// readyForWrite is readyForRead for a change to a collection that must -// already exist: false on a blank file, which such a change cannot find -// anything in and must leave blank, and a schema-1 file migrated first. -func (s *Store) readyForWrite(ctx context.Context) (bool, error) { - version, err := s.readyForRead(ctx) - if err != nil || version == 0 { - return false, err - } - if version != schemaVersion { - if err := s.ensureSchema(ctx); err != nil { - return false, err - } - } - return true, nil -} - // columns are the expressions a read uses for created_at and added_at: the // columns themselves at schema 2, and the unknown 0 on a schema-1 file. func columns(version int) (created, added string) { - if version >= schemaVersion { + if version >= 2 { return "c.created_at", "m.added_at" } return "0", "0" @@ -409,10 +439,7 @@ func (s *Store) Create(ctx context.Context, name string) (Collection, error) { if err != nil { return Collection{}, storeError(err) } - if err := s.ensureSchema(ctx); err != nil { - return Collection{}, storeError(err) - } - tx, err := s.db.BeginTx(ctx, nil) + tx, err := s.beginWrite(ctx, true) if err != nil { return Collection{}, storeError(err) } @@ -434,14 +461,7 @@ func (s *Store) Rename(ctx context.Context, id, name string) error { if err := ValidateName(name); err != nil { return storeError(err) } - ready, err := s.readyForWrite(ctx) - if err != nil { - return storeError(err) - } - if !ready { - return storeError(ErrNotFound) - } - tx, err := s.db.BeginTx(ctx, nil) + tx, err := s.beginWrite(ctx, false) if err != nil { return storeError(err) } @@ -463,14 +483,7 @@ func (s *Store) Rename(ctx context.Context, id, name string) error { // first, because each of them changed too. The records any of it referenced // are untouched. func (s *Store) Delete(ctx context.Context, id string) ([]string, error) { - ready, err := s.readyForWrite(ctx) - if err != nil { - return nil, storeError(err) - } - if !ready { - return nil, storeError(ErrNotFound) - } - tx, err := s.db.BeginTx(ctx, nil) + tx, err := s.beginWrite(ctx, false) if err != nil { return nil, storeError(err) } @@ -511,18 +524,11 @@ func (s *Store) Duplicate(ctx context.Context, id, name string) (Collection, err if err := ValidateName(name); err != nil { return Collection{}, storeError(err) } - ready, err := s.readyForWrite(ctx) - if err != nil { - return Collection{}, storeError(err) - } - if !ready { - return Collection{}, storeError(ErrNotFound) - } copyID, err := newID() if err != nil { return Collection{}, storeError(err) } - tx, err := s.db.BeginTx(ctx, nil) + tx, err := s.beginWrite(ctx, false) if err != nil { return Collection{}, storeError(err) } @@ -536,8 +542,8 @@ func (s *Store) Duplicate(ctx context.Context, id, name string) (Collection, err if _, err := tx.ExecContext(ctx, "INSERT INTO collections(id,name,created_at) VALUES (?,?,?)", copyID, name, s.millis()); err != nil { return Collection{}, storeError(err) } - if _, err := tx.ExecContext(ctx, `INSERT INTO memberships(collection_id,kind,ref_id,session_id,target_collection,added_at) - SELECT ?,kind,ref_id,session_id,target_collection,added_at FROM memberships WHERE collection_id=? ORDER BY seq`, copyID, id); err != nil { + if _, err := tx.ExecContext(ctx, `INSERT INTO memberships(collection_id,kind,ref_id,session_id,target_collection,added_at,factory) + SELECT ?,kind,ref_id,session_id,target_collection,added_at,factory FROM memberships WHERE collection_id=? ORDER BY seq`, copyID, id); err != nil { return Collection{}, storeError(err) } if err := tx.Commit(); err != nil { @@ -547,20 +553,23 @@ func (s *Store) Duplicate(ctx context.Context, id, name string) (Collection, err } func (s *Store) Collections(ctx context.Context) ([]Collection, error) { - version, err := s.readyForRead(ctx) - if err != nil { - return nil, storeError(err) - } - if version == 0 { - return make([]Collection, 0), nil - } - rows, err := s.db.QueryContext(ctx, "SELECT id,name FROM collections ORDER BY seq") + out := make([]Collection, 0) + err := s.Read(ctx, func(tx *sql.Tx, version int) error { + if version == 0 { + return nil + } + rows, err := tx.QueryContext(ctx, "SELECT c.id,c.name FROM collections c ORDER BY seq") + if err != nil { + return err + } + defer rows.Close() + out, err = readCollections(rows) + return err + }) if err != nil { - return nil, storeError(err) + return nil, err } - defer rows.Close() - result, err := readCollections(rows) - return result, storeError(err) + return out, nil } func readCollections(rows *sql.Rows) ([]Collection, error) { @@ -610,14 +619,7 @@ func (s *Store) AddRefs(ctx context.Context, id string, refs []Addition) ([]Memb return nil, nil, storeError(fmt.Errorf("%w: added_at cannot be negative", ErrInvalid)) } } - ready, err := s.readyForWrite(ctx) - if err != nil { - return nil, nil, storeError(err) - } - if !ready { - return nil, nil, storeError(ErrNotFound) - } - tx, err := s.db.BeginTx(ctx, nil) + tx, err := s.beginWrite(ctx, false) if err != nil { return nil, nil, storeError(err) } @@ -625,6 +627,7 @@ func (s *Store) AddRefs(ctx context.Context, id string, refs []Addition) ([]Memb if err := requireCollection(ctx, tx, id); err != nil { return nil, nil, storeError(err) } + now := s.millis() added, already := make([]Member, 0, len(refs)), make([]Ref, 0) for _, ref := range refs { @@ -639,8 +642,8 @@ func (s *Store) AddRefs(ctx context.Context, id string, refs []Addition) ([]Memb if at == 0 { at = now } - result, err := tx.ExecContext(ctx, `INSERT INTO memberships(collection_id,kind,ref_id,session_id,target_collection,added_at) - VALUES (?,?,?,?,?,?) ON CONFLICT(collection_id,kind,ref_id,session_id) DO NOTHING`, id, ref.Kind, ref.ID, ref.SessionID, target, at) + result, err := tx.ExecContext(ctx, `INSERT INTO memberships(collection_id,kind,ref_id,session_id,target_collection,added_at,factory) + VALUES (?,?,?,?,?,?,?) ON CONFLICT(collection_id,kind,ref_id,session_id,factory) DO NOTHING`, id, ref.Kind, ref.ID, ref.SessionID, target, at, ref.Factory) if err != nil { return nil, nil, storeError(err) } @@ -701,14 +704,7 @@ func (s *Store) Relink(ctx context.Context, id string, from, to Ref) (Member, bo if err := to.Validate(); err != nil { return Member{}, false, storeError(err) } - ready, err := s.readyForWrite(ctx) - if err != nil { - return Member{}, false, storeError(err) - } - if !ready { - return Member{}, false, storeError(ErrNotFound) - } - tx, err := s.db.BeginTx(ctx, nil) + tx, err := s.beginWrite(ctx, false) if err != nil { return Member{}, false, storeError(err) } @@ -717,8 +713,8 @@ func (s *Store) Relink(ctx context.Context, id string, from, to Ref) (Member, bo return Member{}, false, storeError(err) } membership := func(ref Ref) (seq, at int64, err error) { - err = tx.QueryRowContext(ctx, "SELECT seq,added_at FROM memberships WHERE collection_id=? AND kind=? AND ref_id=? AND session_id=?", - id, ref.Kind, ref.ID, ref.SessionID).Scan(&seq, &at) + err = tx.QueryRowContext(ctx, "SELECT seq,added_at FROM memberships WHERE collection_id=? AND kind=? AND ref_id=? AND session_id=? AND factory=?", + id, ref.Kind, ref.ID, ref.SessionID, ref.Factory).Scan(&seq, &at) return seq, at, err } seq, at, err := membership(from) @@ -749,8 +745,8 @@ func (s *Store) Relink(ctx context.Context, id string, from, to Ref) (Member, bo } target = to.ID } - if _, err := tx.ExecContext(ctx, "UPDATE memberships SET kind=?,ref_id=?,session_id=?,target_collection=? WHERE seq=?", - to.Kind, to.ID, to.SessionID, target, seq); err != nil { + if _, err := tx.ExecContext(ctx, "UPDATE memberships SET kind=?,ref_id=?,session_id=?,target_collection=?,factory=? WHERE seq=?", + to.Kind, to.ID, to.SessionID, target, to.Factory, seq); err != nil { return Member{}, false, storeError(err) } if err := tx.Commit(); err != nil { @@ -774,14 +770,7 @@ func (s *Store) RemoveRefs(ctx context.Context, id string, refs []Ref) ([]Member return nil, storeError(err) } } - ready, err := s.readyForWrite(ctx) - if err != nil { - return nil, storeError(err) - } - if !ready { - return nil, storeError(ErrNotFound) - } - tx, err := s.db.BeginTx(ctx, nil) + tx, err := s.beginWrite(ctx, false) if err != nil { return nil, storeError(err) } @@ -792,8 +781,8 @@ func (s *Store) RemoveRefs(ctx context.Context, id string, refs []Ref) ([]Member removed := make([]Member, 0, len(refs)) for _, ref := range refs { var at int64 - err := tx.QueryRowContext(ctx, "DELETE FROM memberships WHERE collection_id=? AND kind=? AND ref_id=? AND session_id=? RETURNING added_at", - id, ref.Kind, ref.ID, ref.SessionID).Scan(&at) + err := tx.QueryRowContext(ctx, "DELETE FROM memberships WHERE collection_id=? AND kind=? AND ref_id=? AND session_id=? AND factory=? RETURNING added_at", + id, ref.Kind, ref.ID, ref.SessionID, ref.Factory).Scan(&at) if errors.Is(err, sql.ErrNoRows) { continue } @@ -809,55 +798,56 @@ func (s *Store) RemoveRefs(ctx context.Context, id string, refs []Ref) ([]Member } func (s *Store) Members(ctx context.Context, id string) ([]Ref, error) { - version, err := s.readyForRead(ctx) - if err != nil { - return nil, storeError(err) - } - if version == 0 { - return nil, storeError(ErrNotFound) - } - var exists bool - if err := s.db.QueryRowContext(ctx, "SELECT EXISTS(SELECT 1 FROM collections WHERE id=?)", id).Scan(&exists); err != nil { - return nil, storeError(err) - } - if !exists { - return nil, storeError(ErrNotFound) - } - rows, err := s.db.QueryContext(ctx, "SELECT kind,ref_id,session_id FROM memberships WHERE collection_id=? ORDER BY seq", id) - if err != nil { - return nil, storeError(err) - } - defer rows.Close() - result := make([]Ref, 0) - for rows.Next() { - var ref Ref - if err := rows.Scan(&ref.Kind, &ref.ID, &ref.SessionID); err != nil { - return nil, storeError(err) + out := make([]Ref, 0) + err := s.Read(ctx, func(tx *sql.Tx, version int) error { + if version == 0 { + return ErrNotFound } - result = append(result, ref) + if err := requireCollection(ctx, tx, id); err != nil { + return err + } + rows, err := tx.QueryContext(ctx, "SELECT kind,ref_id,session_id,"+factoryColumn(version)+" FROM memberships WHERE collection_id=? ORDER BY seq", id) + if err != nil { + return err + } + defer rows.Close() + for rows.Next() { + var ref Ref + if err := rows.Scan(&ref.Kind, &ref.ID, &ref.SessionID, &ref.Factory); err != nil { + return err + } + out = append(out, ref) + } + return rows.Err() + }) + if err != nil { + return nil, err } - return result, storeError(rows.Err()) + return out, nil } func (s *Store) CollectionsFor(ctx context.Context, ref Ref) ([]Collection, error) { if err := ref.Validate(); err != nil { return nil, storeError(err) } - version, err := s.readyForRead(ctx) - if err != nil { - return nil, storeError(err) - } - if version == 0 { - return make([]Collection, 0), nil - } - rows, err := s.db.QueryContext(ctx, `SELECT c.id,c.name FROM collections c JOIN memberships m ON m.collection_id=c.id - WHERE m.kind=? AND m.ref_id=? AND m.session_id=? ORDER BY c.seq`, ref.Kind, ref.ID, ref.SessionID) + out := make([]Collection, 0) + err := s.Read(ctx, func(tx *sql.Tx, version int) error { + if version == 0 { + return nil + } + rows, err := tx.QueryContext(ctx, `SELECT c.id,c.name FROM collections c JOIN memberships m ON m.collection_id=c.id + WHERE m.kind=? AND m.ref_id=? AND m.session_id=? AND `+factoryColumn(version)+`=? ORDER BY c.seq`, ref.Kind, ref.ID, ref.SessionID, ref.Factory) + if err != nil { + return err + } + defer rows.Close() + out, err = readCollections(rows) + return err + }) if err != nil { - return nil, storeError(err) + return nil, err } - defer rows.Close() - result, err := readCollections(rows) - return result, storeError(err) + return out, nil } // storeError replaces SQLite's lock account with the stable sentence exposed by @@ -942,3 +932,10 @@ func pathReason(err error) error { } return err } + +func factoryColumn(version int) string { + if version >= 3 { + return "factory" + } + return "''" +} diff --git a/internal/workspace/views.go b/internal/workspace/views.go index 1f235ad2c8..3ca1c469c8 100644 --- a/internal/workspace/views.go +++ b/internal/workspace/views.go @@ -136,18 +136,14 @@ func (s *Store) ListPage(ctx context.Context, holds *Ref, after string, limit in } from = c.S } - version, err := s.readyForRead(ctx) + tx, version, err := s.beginRead(ctx) if err != nil { return Page{}, storeError(err) } + defer tx.Rollback() if version == 0 { return Page{Collections: make([]Summary, 0)}, nil } - tx, err := s.db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true}) - if err != nil { - return Page{}, storeError(err) - } - defer tx.Rollback() var page Page if err := tx.QueryRowContext(ctx, "SELECT count(*) FROM collections").Scan(&page.Total); err != nil { return Page{}, storeError(err) @@ -183,18 +179,14 @@ func (s *Store) List(ctx context.Context, holds *Ref) ([]Summary, error) { // Describe is one collection as List says it. func (s *Store) Describe(ctx context.Context, id string) (Summary, error) { - version, err := s.readyForRead(ctx) + tx, version, err := s.beginRead(ctx) if err != nil { return Summary{}, storeError(err) } + defer tx.Rollback() if version == 0 { return Summary{}, storeError(ErrNotFound) } - tx, err := s.db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true}) - if err != nil { - return Summary{}, storeError(err) - } - defer tx.Rollback() out, err := summaries(ctx, tx, version, nil, id, 0, 1) if err != nil { return Summary{}, storeError(err) @@ -212,8 +204,8 @@ func summaries(ctx context.Context, tx *sql.Tx, version int, holds *Ref, only st held := "0" args := []any{} if holds != nil { - held = "EXISTS(SELECT 1 FROM memberships h WHERE h.collection_id=c.id AND h.kind=? AND h.ref_id=? AND h.session_id=?)" - args = append(args, holds.Kind, holds.ID, holds.SessionID) + held = "EXISTS(SELECT 1 FROM memberships h WHERE h.collection_id=c.id AND h.kind=? AND h.ref_id=? AND h.session_id=? AND " + factoryColumn(version) + "=?)" + args = append(args, holds.Kind, holds.ID, holds.SessionID, holds.Factory) } var where []string if only != "" { @@ -320,18 +312,14 @@ func (s *Store) Tree(ctx context.Context, id string, depth, limit int, after str } start = &levelAfter{added: c.A, seq: c.S} } - version, err := s.readyForRead(ctx) + tx, version, err := s.beginRead(ctx) if err != nil { return Tree{}, storeError(err) } + defer tx.Rollback() if version == 0 { return Tree{}, storeError(ErrNotFound) } - tx, err := s.db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true}) - if err != nil { - return Tree{}, storeError(err) - } - defer tx.Rollback() top, err := summaries(ctx, tx, version, nil, id, 0, 1) if err != nil { return Tree{}, storeError(err) @@ -415,14 +403,14 @@ func levelOf(ctx context.Context, tx *sql.Tx, version int, id string, limit int, // A schema-1 file knows no moments, so its order is insertion alone; a // constant in ORDER BY would be read as a column number. order := "m.seq DESC" - if version >= schemaVersion { + if version >= 2 { order = "m.added_at DESC, m.seq DESC" } args = append(args, limit+1) - rows, err := tx.QueryContext(ctx, fmt.Sprintf(`SELECT m.seq,m.kind,m.ref_id,m.session_id,%s,coalesce(c.name,''),c.id IS NOT NULL, + rows, err := tx.QueryContext(ctx, fmt.Sprintf(`SELECT m.seq,m.kind,m.ref_id,m.session_id,%s,%s,coalesce(c.name,''),c.id IS NOT NULL, CASE WHEN m.kind='collection' THEN (SELECT count(*) FROM memberships x WHERE x.collection_id=m.ref_id) ELSE 0 END FROM memberships m LEFT JOIN collections c ON m.kind='collection' AND c.id=m.ref_id -WHERE m.collection_id=?%s ORDER BY %s LIMIT ?`, added, from, order), args...) +WHERE m.collection_id=?%s ORDER BY %s LIMIT ?`, added, factoryColumn(version), from, order), args...) if err != nil { return nil, false, err } @@ -434,7 +422,7 @@ WHERE m.collection_id=?%s ORDER BY %s LIMIT ?`, added, from, order), args...) break } var n Node - if err := rows.Scan(&n.seq, &n.Kind, &n.ID, &n.SessionID, &n.AddedAt, &n.Name, &n.Found, &n.Count); err != nil { + if err := rows.Scan(&n.seq, &n.Kind, &n.ID, &n.SessionID, &n.AddedAt, &n.Factory, &n.Name, &n.Found, &n.Count); err != nil { return nil, false, err } nodes = append(nodes, n) @@ -467,18 +455,14 @@ type Expansion struct { // always say what it is not showing. func (s *Store) Expand(ctx context.Context, id string, depth int) (Expansion, error) { depth = max(1, min(depth, ExpandDepthMax)) - version, err := s.readyForRead(ctx) + tx, version, err := s.beginRead(ctx) if err != nil { return Expansion{}, storeError(err) } + defer tx.Rollback() if version == 0 { return Expansion{}, storeError(ErrNotFound) } - tx, err := s.db.BeginTx(ctx, &sql.TxOptions{ReadOnly: true}) - if err != nil { - return Expansion{}, storeError(err) - } - defer tx.Rollback() top, err := summaries(ctx, tx, version, nil, id, 0, 1) if err != nil { return Expansion{}, storeError(err) diff --git a/internal/workspace/workspace.go b/internal/workspace/workspace.go index 647c004f46..0414537657 100644 --- a/internal/workspace/workspace.go +++ b/internal/workspace/workspace.go @@ -22,8 +22,9 @@ const ( // CrewKind names a crew by its id; ProcessKind names a process a // conversation started, by its id, and like a task carries that // conversation as its session. Both arrived with schema 2. - CrewKind Kind = "crew" - ProcessKind Kind = "process" + CrewKind Kind = "crew" + ProcessKind Kind = "process" + FactoryJobKind Kind = "factory_job" // StandingKind is an older build's reference to a standing item. Standing // items are gone (automations replaced them), so the collections command // refuses a new one, but a row that already names one is still a row: it @@ -57,6 +58,7 @@ type Ref struct { Kind Kind `json:"kind"` ID string `json:"id"` SessionID string `json:"session_id,omitempty"` + Factory string `json:"factory,omitempty"` } type Collection struct { @@ -71,12 +73,17 @@ type Member struct { AddedAt int64 `json:"added_at,omitempty"` } -// key is a reference's identity, the three columns the uniqueness rule reads. -func (r Ref) key() string { return string(r.Kind) + "\x00" + r.ID + "\x00" + r.SessionID } +// key includes the factory because job numbers are local to one factory. +func (r Ref) key() string { + return string(r.Kind) + "\x00" + r.ID + "\x00" + r.SessionID + "\x00" + r.Factory +} // Validate checks the reference shape without asking an execution owner to // open it. A temporarily unavailable record must remain discoverable here. func (r Ref) Validate() error { + if r.Kind != FactoryJobKind && r.Factory != "" { + return fmt.Errorf("%w: only a factory_job reference has a factory", ErrInvalid) + } if !validText(r.ID, 4096) { return fmt.Errorf("%w: record id is empty, too long or contains control characters", ErrInvalid) } @@ -84,6 +91,11 @@ func (r Ref) Validate() error { return fmt.Errorf("%w: only a task or a process reference has a session id", ErrInvalid) } switch r.Kind { + case FactoryJobKind: + n, err := strconv.ParseUint(r.ID, 10, 63) + if err != nil || n == 0 || strconv.FormatUint(n, 10) != r.ID || !validText(r.Factory, 4096) { + return fmt.Errorf("%w: a factory_job needs a factory and a positive canonical job number", ErrInvalid) + } case CollectionKind, ConversationKind, StandingKind, CrewKind: case ProcessKind: if !validText(r.SessionID, 1024) {