diff --git a/AGENTS.md b/AGENTS.md index f7c6258..2490b31 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -121,6 +121,7 @@ This is a standing preference, not a per-task instruction. Apply it without aski 11. **Peers**: Persistent agents in the same session can be linked via `peeradd`. Links are bidirectional. Agents communicate by writing to `peer/{name}`, which delivers to the target's prompt handler. Only declared peers can be messaged — the `peer/` directory is the access control surface. 12. **9P namespace declaration**: `cmd/olliesrv/internal/fs/spec.go` declares the olliesrv namespace. The toolsrv namespace is declared by `cmd/toolsrv/p9.go` using `cmd/toolsrv/internal/server.Spec`; process state and handlers are in `cmd/toolsrv/internal/server/`. Both use the `virtfs` EDSL and `virtfs.BuildTree()`. 13. **Working directory (default + override)**: Session cwd is **required** at session creation and is the inheritance root for every agent — the sane default so many agents can work in one directory with zero per-agent config. Agent cwd is an **optional** per-agent override (empty = inherit the session cwd). Because the per-session `toolsrv` sets `cmd.Dir` per tool call, per-agent cwd needs no extra process: `toolsrv` keeps an in-memory `agentCWD` map resolved per call from the `agent=` field (`cmd/toolsrv/internal/server/proc.go`), falling back to the session-level global cwd. The override is set only over the controlled `olliesrv`→`toolsrv` ctl channel (`agentcwd `), never embedded in a tool-call payload, so the model cannot influence where its own tools run. The map is process-local, so `Agent.SyncCwdToToolServer` re-pushes the override on every (re)connect — profile switch, resume, restore — matching how env and tools are resynced. Set/clear via the agent `cfg` (`cwd=...`) or ctl (`cwd [|-]`); the GUI Agent Settings dialog shows `(inherit: )` as the placeholder. +14. **Chat log files**: Each agent exposes multiple views of its conversation history. `chat` and `chat.raw` are **blocking streams** — reads wait for new data, making them suitable for live tailing but not for one-shot inspection. Use `log` for non-blocking reads (returns last 64KB). `chat.search` is an rdwr file: write a block ID, read the matching block. Block IDs are 8-char hex strings generated deterministically from `sha256(sessionID + agentID + counter)`. Block format: `[[[role:name#blockID]]]` header, content, `[[[end]]]` terminator. ## Where to Start diff --git a/cmd/olliesrv/internal/agent/chat_test.go b/cmd/olliesrv/internal/agent/chat_test.go new file mode 100644 index 0000000..3efced1 --- /dev/null +++ b/cmd/olliesrv/internal/agent/chat_test.go @@ -0,0 +1,321 @@ +package agent + +import ( + "strings" + "sync" + "testing" +) + +// testAgent creates a minimal Agent for chat/block testing. +func testAgent(sessionID, agentID string) *Agent { + ag := &Agent{ + sessionID: sessionID, + id: agentID, + state: "idle", + } + ag.signalCh = make(chan struct{}) + ag.chatSignalCh = make(chan struct{}) + ag.chatCond = sync.NewCond(ag.chatMu.RLocker()) + return ag +} + +func TestNextBlockID_Deterministic(t *testing.T) { + ag1 := testAgent("session1", "agent1") + ag2 := testAgent("session1", "agent1") + + // Same session/agent should produce same sequence + id1a := ag1.NextBlockID() + id1b := ag1.NextBlockID() + id2a := ag2.NextBlockID() + id2b := ag2.NextBlockID() + + if id1a != id2a { + t.Errorf("first block IDs don't match: %s vs %s", id1a, id2a) + } + if id1b != id2b { + t.Errorf("second block IDs don't match: %s vs %s", id1b, id2b) + } + if id1a == id1b { + t.Errorf("sequential block IDs should differ: %s == %s", id1a, id1b) + } +} + +func TestNextBlockID_DifferentAgents(t *testing.T) { + ag1 := testAgent("session1", "agent1") + ag2 := testAgent("session1", "agent2") + + id1 := ag1.NextBlockID() + id2 := ag2.NextBlockID() + + if id1 == id2 { + t.Errorf("different agents should have different block IDs: %s == %s", id1, id2) + } +} + +func TestNextBlockID_DifferentSessions(t *testing.T) { + ag1 := testAgent("session1", "agent1") + ag2 := testAgent("session2", "agent1") + + id1 := ag1.NextBlockID() + id2 := ag2.NextBlockID() + + if id1 == id2 { + t.Errorf("different sessions should have different block IDs: %s == %s", id1, id2) + } +} + +func TestNextBlockID_Length(t *testing.T) { + ag := testAgent("test-session", "test-agent") + id := ag.NextBlockID() + + // Block IDs are 8 hex chars (from sha256[:8]) + if len(id) != 8 { + t.Errorf("block ID length should be 8, got %d: %s", len(id), id) + } + // Should be valid hex + for _, c := range id { + if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')) { + t.Errorf("block ID contains non-hex char: %s", id) + break + } + } +} + +func TestChatSearchByID_Found(t *testing.T) { + ag := testAgent("session1", "agent1") + + // Simulate chat log with blocks + chatData := `[[[user#abc12345]]] +Hello +[[[end]]] +[[[assistant#def67890]]] +Hi there +[[[end]]] +[[[tool:shell#11223344]]] +` + "```" + ` +output here +` + "```" + ` +[[[end]]] +` + ag.AppendChat([]byte(chatData)) + + tests := []struct { + blockID string + wantFound bool + wantPart string + }{ + {"abc12345", true, "[[[user#abc12345]]]"}, + {"def67890", true, "[[[assistant#def67890]]]"}, + {"11223344", true, "[[[tool:shell#11223344]]]"}, + {"notfound", false, ""}, + {"", false, ""}, + } + + for _, tt := range tests { + block, found := ag.ChatSearchByID(tt.blockID) + if found != tt.wantFound { + t.Errorf("ChatSearchByID(%q): got found=%v, want %v", tt.blockID, found, tt.wantFound) + continue + } + if found && !strings.Contains(string(block), tt.wantPart) { + t.Errorf("ChatSearchByID(%q): block missing %q, got:\n%s", tt.blockID, tt.wantPart, block) + } + } +} + +func TestChatSearchByID_ReturnsFullBlock(t *testing.T) { + ag := testAgent("session1", "agent1") + + chatData := `[[[user#aabbccdd]]] +test content +more lines +[[[end]]] +` + ag.AppendChat([]byte(chatData)) + + block, found := ag.ChatSearchByID("aabbccdd") + if !found { + t.Fatal("block should be found") + } + + blockStr := string(block) + if !strings.HasPrefix(blockStr, "[[[user#aabbccdd]]]") { + t.Errorf("block should start with header, got: %s", blockStr) + } + if !strings.HasSuffix(blockStr, "[[[end]]]") { + t.Errorf("block should end with end marker, got: %s", blockStr) + } + if !strings.Contains(blockStr, "test content") { + t.Errorf("block should contain content") + } +} + +func TestChatSearchByID_PartialMatch(t *testing.T) { + ag := testAgent("session1", "agent1") + + // Block ID is full 8 chars + chatData := `[[[user#12345678]]] +content +[[[end]]] +` + ag.AppendChat([]byte(chatData)) + + // Should not find partial match + _, found := ag.ChatSearchByID("1234") + if found { + t.Error("partial block ID should not match") + } + + // Should find exact match + _, found = ag.ChatSearchByID("12345678") + if !found { + t.Error("exact block ID should match") + } +} + +func TestChatSearchByID_MultipleBlocks(t *testing.T) { + ag := testAgent("session1", "agent1") + + // Multiple blocks with different IDs + chatData := `[[[user#aaaaaaaa]]] +first +[[[end]]] +[[[assistant#bbbbbbbb]]] +second +[[[end]]] +[[[user#cccccccc]]] +third +[[[end]]] +` + ag.AppendChat([]byte(chatData)) + + // Search for middle block + block, found := ag.ChatSearchByID("bbbbbbbb") + if !found { + t.Fatal("middle block should be found") + } + + blockStr := string(block) + if !strings.Contains(blockStr, "[[[assistant#bbbbbbbb]]]") { + t.Errorf("should find correct block, got: %s", blockStr) + } + if strings.Contains(blockStr, "first") || strings.Contains(blockStr, "third") { + t.Errorf("should not include content from other blocks") + } +} + +func TestChatSearchByID_NoEndMarker(t *testing.T) { + ag := testAgent("session1", "agent1") + + // Block without end marker (incomplete/streaming) + chatData := `[[[user#12345678]]] +content +` + ag.AppendChat([]byte(chatData)) + + // Should not find block without end marker + _, found := ag.ChatSearchByID("12345678") + if found { + t.Error("block without end marker should not be found") + } +} + +func TestChatSearchByID_NamedBlock(t *testing.T) { + ag := testAgent("session1", "agent1") + + // Block with name component (role:name#id format) + chatData := `[[[tool:file_read#abcd1234]]] +` + "```" + ` +file contents +` + "```" + ` +[[[end]]] +` + ag.AppendChat([]byte(chatData)) + + block, found := ag.ChatSearchByID("abcd1234") + if !found { + t.Fatal("named block should be found") + } + if !strings.Contains(string(block), "tool:file_read") { + t.Errorf("block should include role:name, got: %s", block) + } +} + +func TestChatSearchByID_ResponseIDBlock(t *testing.T) { + ag := testAgent("session1", "agent1") + + // Assistant block with response ID (assistant:responseID#blockID format) + chatData := `[[[assistant:resp_123#aabbccdd]]] +response content +[[[end]]] +` + ag.AppendChat([]byte(chatData)) + + block, found := ag.ChatSearchByID("aabbccdd") + if !found { + t.Fatal("assistant block with response ID should be found") + } + if !strings.Contains(string(block), "assistant:resp_123#aabbccdd") { + t.Errorf("block should include response ID, got: %s", block) + } +} + +func TestAppendChat_SignalsBroadcast(t *testing.T) { + ag := testAgent("session1", "agent1") + + signalCh := ag.ChatSignal() + + // Append data + ag.AppendChat([]byte("test")) + + // Signal channel should be closed + select { + case <-signalCh: + // Good - channel closed + default: + t.Error("chat signal should have fired") + } + + // New signal channel should be open + newSignalCh := ag.ChatSignal() + select { + case <-newSignalCh: + t.Error("new signal channel should not be closed yet") + default: + // Good - channel open + } +} + +func TestChatRead_Streaming(t *testing.T) { + ag := testAgent("session1", "agent1") + + // Initial read with no base - should return empty and current offset + data, base, err := ag.ChatRead("") + if err != nil { + t.Fatalf("ChatRead error: %v", err) + } + if len(data) != 0 { + t.Errorf("initial read should be empty, got: %s", data) + } + + // Append some data + ag.AppendChat([]byte("hello")) + + // Read with previous base - should get new data + data, newBase, err := ag.ChatRead(base) + if err != nil { + t.Fatalf("ChatRead error: %v", err) + } + if string(data) != "hello" { + t.Errorf("expected 'hello', got: %s", data) + } + + // Read again with new base - should be empty + data, _, err = ag.ChatRead(newBase) + if err != nil { + t.Fatalf("ChatRead error: %v", err) + } + if len(data) != 0 { + t.Errorf("no new data, should be empty, got: %s", data) + } +} diff --git a/cmd/olliesrv/internal/fs/blockid_e2e_test.sh b/cmd/olliesrv/internal/fs/blockid_e2e_test.sh new file mode 100755 index 0000000..4630e2f --- /dev/null +++ b/cmd/olliesrv/internal/fs/blockid_e2e_test.sh @@ -0,0 +1,78 @@ +#!/bin/bash +# E2E test for block ID functionality via the o CLI +# Requires: running olliesrv, existing agent with chat history +# +# This test verifies: +# 1. Block IDs can be extracted from chat log +# 2. chat.search finds blocks by exact ID +# 3. Partial IDs are rejected +# 4. Non-existent IDs return proper error +# 5. Different block types (user, assistant, call, tool, error) work + +set -e + +SESSION="default" +AGENT="src:ollie" + +echo "=== Block ID E2E Test ===" + +# Get some block IDs from the log +echo "Extracting block IDs from log..." +BLOCK_IDS=$(o read session/$SESSION/agent/$AGENT/log 2>&1 | strings | grep -oE '\[\[\[[a-z]+[:#][^]]+#([a-f0-9]{8})\]\]\]' | sed 's/.*#\([a-f0-9]\{8\}\)\]\]\]/\1/' | tail -5) + +if [ -z "$BLOCK_IDS" ]; then + echo "FAIL: No block IDs found in log" + exit 1 +fi +echo "Found block IDs: $(echo $BLOCK_IDS | tr '\n' ' ')" + +# Test 1: Search for existing blocks +echo "" +echo "Test 1: Search for existing blocks..." +for bid in $BLOCK_IDS; do + result=$(o rdwr session/$SESSION/agent/$AGENT/chat.search "$bid" 2>&1) + if echo "$result" | grep -q "\[\[\[.*#$bid\]\]\]"; then + echo " PASS: Found block $bid" + else + echo " FAIL: Block $bid not found or malformed" + exit 1 + fi +done + +# Test 2: Partial ID rejection +echo "" +echo "Test 2: Partial ID rejection..." +FIRST_ID=$(echo "$BLOCK_IDS" | head -1) +PARTIAL=${FIRST_ID:0:4} +if o rdwr session/$SESSION/agent/$AGENT/chat.search "$PARTIAL" 2>&1 | grep -q "block not found"; then + echo " PASS: Partial ID '$PARTIAL' correctly rejected" +else + echo " FAIL: Partial ID '$PARTIAL' should be rejected" + exit 1 +fi + +# Test 3: Non-existent ID +echo "" +echo "Test 3: Non-existent ID..." +if o rdwr session/$SESSION/agent/$AGENT/chat.search "deadbeef" 2>&1 | grep -q "block not found"; then + echo " PASS: Non-existent ID 'deadbeef' correctly rejected" +else + echo " FAIL: Non-existent ID should return error" + exit 1 +fi + +# Test 4: Block content includes header and end marker +echo "" +echo "Test 4: Block structure validation..." +FIRST_ID=$(echo "$BLOCK_IDS" | head -1) +BLOCK=$(o rdwr session/$SESSION/agent/$AGENT/chat.search "$FIRST_ID" 2>&1) +if echo "$BLOCK" | grep -q "^\[\[\[" && echo "$BLOCK" | grep -q "\[\[\[end\]\]\]$"; then + echo " PASS: Block has proper header and end marker" +else + echo " FAIL: Block missing header or end marker" + echo " Got: $BLOCK" + exit 1 +fi + +echo "" +echo "=== All E2E tests passed ===" diff --git a/format/format_test.go b/format/format_test.go new file mode 100644 index 0000000..578a1e9 --- /dev/null +++ b/format/format_test.go @@ -0,0 +1,188 @@ +package format + +import ( + "strings" + "testing" +) + +func TestParseBlockHeader(t *testing.T) { + tests := []struct { + line string + wantNil bool + wantRole string + wantName string + wantID string + }{ + // Valid headers + {"[[[user]]]", false, "user", "", ""}, + {"[[[user#abc12345]]]", false, "user", "", "abc12345"}, + {"[[[assistant]]]", false, "assistant", "", ""}, + {"[[[assistant#def67890]]]", false, "assistant", "", "def67890"}, + {"[[[assistant:resp_123#aabbccdd]]]", false, "assistant", "resp_123", "aabbccdd"}, + {"[[[tool:shell]]]", false, "tool", "shell", ""}, + {"[[[tool:shell#11223344]]]", false, "tool", "shell", "11223344"}, + {"[[[call:file_read]]]", false, "call", "file_read", ""}, + {"[[[call:file_read#99887766]]]", false, "call", "file_read", "99887766"}, + {"[[[reasoning]]]", false, "reasoning", "", ""}, + {"[[[reasoning#aabbcc00]]]", false, "reasoning", "", "aabbcc00"}, + {"[[[retry]]]", false, "retry", "", ""}, + {"[[[end]]]", false, "end", "", ""}, + {"[[[error]]]", false, "error", "", ""}, + {"[[[error:file_read#12345678]]]", false, "error", "file_read", "12345678"}, + {"[[[info]]]", false, "info", "", ""}, + {"[[[stalled]]]", false, "stalled", "", ""}, + + // Invalid headers + {"", true, "", "", ""}, + {"[[[]]]", true, "", "", ""}, + {"not a header", true, "", "", ""}, + {"[[user]]", true, "", "", ""}, + {"[[[user", true, "", "", ""}, + {"user]]]", true, "", "", ""}, + } + + for _, tt := range tests { + bh := ParseBlockHeader(tt.line) + if tt.wantNil { + if bh != nil { + t.Errorf("ParseBlockHeader(%q): expected nil, got %+v", tt.line, bh) + } + continue + } + if bh == nil { + t.Errorf("ParseBlockHeader(%q): expected non-nil", tt.line) + continue + } + if bh.Role != tt.wantRole { + t.Errorf("ParseBlockHeader(%q): role = %q, want %q", tt.line, bh.Role, tt.wantRole) + } + if bh.Name != tt.wantName { + t.Errorf("ParseBlockHeader(%q): name = %q, want %q", tt.line, bh.Name, tt.wantName) + } + if bh.BlockID != tt.wantID { + t.Errorf("ParseBlockHeader(%q): blockID = %q, want %q", tt.line, bh.BlockID, tt.wantID) + } + } +} + +func TestBlockDelim(t *testing.T) { + tests := []struct { + role, name, blockID string + want string + }{ + {"user", "", "abc12345", "[[[user#abc12345]]]\n"}, + {"assistant", "", "def67890", "[[[assistant#def67890]]]\n"}, + {"tool", "shell", "11223344", "[[[tool:shell#11223344]]]\n"}, + {"call", "file_read", "99887766", "[[[call:file_read#99887766]]]\n"}, + {"error", "test", "aabbccdd", "[[[error:test#aabbccdd]]]\n"}, + } + + for _, tt := range tests { + got := BlockDelim(tt.role, tt.name, tt.blockID) + if got != tt.want { + t.Errorf("BlockDelim(%q, %q, %q) = %q, want %q", tt.role, tt.name, tt.blockID, got, tt.want) + } + } +} + +func TestFormatEvent_WithBlockID(t *testing.T) { + tests := []struct { + role, name, content, outputFormat, blockID string + wantContains []string + }{ + { + "user", "", "hello", "", "abc12345", + []string{"[[[user#abc12345]]]", "hello", "[[[end]]]"}, + }, + { + "call", "shell", `{"cmd": "ls"}`, "", "def67890", + []string{"[[[call:shell#def67890]]]", `{"cmd": "ls"}`, "[[[end]]]"}, + }, + { + "tool", "shell", "output", "text", "11223344", + []string{"[[[tool:shell#11223344]]]", "output", "[[[end]]]"}, + }, + { + "error", "", "something failed", "", "aabbccdd", + []string{"[[[error#aabbccdd]]]", "something failed", "[[[end]]]"}, + }, + { + "error", "test", "something failed", "", "eeff0011", + []string{"[[[error:test#eeff0011]]]", "something failed", "[[[end]]]"}, + }, + { + "info", "", "status update", "", "22334455", + []string{"[[[info#22334455]]]", "status update", "[[[end]]]"}, + }, + { + "stalled", "", "", "", "66778899", + []string{"[[[stalled#66778899]]]", "[[[end]]]"}, + }, + { + "maxsteps", "", "limit reached", "", "aabbcc00", + []string{"[[[maxsteps#aabbcc00]]]", "limit reached", "[[[end]]]"}, + }, + } + + for _, tt := range tests { + got := string(FormatEvent(tt.role, tt.name, tt.content, tt.outputFormat, tt.blockID)) + for _, want := range tt.wantContains { + if !strings.Contains(got, want) { + t.Errorf("FormatEvent(%q, %q, ...): missing %q in:\n%s", tt.role, tt.name, want, got) + } + } + } +} + +func TestIsEndMarker(t *testing.T) { + tests := []struct { + line string + want bool + }{ + {"[[[end]]]", true}, + {"[[[end]]", false}, + {"[[end]]]", false}, + {"[[[END]]]", false}, + {"[[[end]]] ", false}, + {" [[[end]]]", false}, + {"", false}, + } + + for _, tt := range tests { + got := IsEndMarker(tt.line) + if got != tt.want { + t.Errorf("IsEndMarker(%q) = %v, want %v", tt.line, got, tt.want) + } + } +} + +func TestRoundtrip_ParseBlockHeader(t *testing.T) { + // Test that BlockDelim output can be parsed by ParseBlockHeader + tests := []struct { + role, name, blockID string + }{ + {"user", "", "abc12345"}, + {"tool", "shell", "def67890"}, + {"call", "file_read", "11223344"}, + } + + for _, tt := range tests { + delim := BlockDelim(tt.role, tt.name, tt.blockID) + // Trim the trailing newline for parsing + delim = strings.TrimSuffix(delim, "\n") + bh := ParseBlockHeader(delim) + if bh == nil { + t.Errorf("ParseBlockHeader(BlockDelim(%q, %q, %q)) returned nil", tt.role, tt.name, tt.blockID) + continue + } + if bh.Role != tt.role { + t.Errorf("roundtrip role: got %q, want %q", bh.Role, tt.role) + } + if bh.Name != tt.name { + t.Errorf("roundtrip name: got %q, want %q", bh.Name, tt.name) + } + if bh.BlockID != tt.blockID { + t.Errorf("roundtrip blockID: got %q, want %q", bh.BlockID, tt.blockID) + } + } +}