feat: Ollama/gemma4 compat — /init flow, stream filter, safety fixes
provider/openai: - Fix doubled tool call args (argsComplete flag): Ollama sends complete args in the first streaming chunk then repeats them as delta, causing doubled JSON and 400 errors in elfs - Handle fs: prefix (gemma4 uses fs:grep instead of fs.grep) - Add Reasoning field support for Ollama thinking output cmd/gnoma: - Early TTY detection so logger is created with correct destination before any component gets a reference to it (fixes slog WARN bleed into TUI textarea) permission: - Exempt spawn_elfs and agent tools from safety scanner: elf prompt text may legitimately mention .env/.ssh/credentials patterns and should not be blocked tui/app: - /init retry chain: no-tool-calls → spawn_elfs nudge → write nudge (ask for plain text output) → TUI fallback write from streamBuf - looksLikeAgentsMD + extractMarkdownDoc: validate and clean fallback content before writing (reject refusals, strip narrative preambles) - Collapse thinking output to 3 lines; ctrl+o to expand (live stream and committed messages) - Stream-level filter for model pseudo-tool-call blocks: suppresses <<tool_code>>...</tool_code>> and <<function_call>>...<tool_call|> from entering streamBuf across chunk boundaries - sanitizeAssistantText regex covers both block formats - Reset streamFilterClose at every turn start
This commit is contained in:
@@ -3,6 +3,7 @@ package elf
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
@@ -73,13 +74,16 @@ func nextID(prefix string) string {
|
||||
|
||||
// BackgroundElf runs on its own goroutine with an independent engine.
|
||||
type BackgroundElf struct {
|
||||
id string
|
||||
eng *engine.Engine
|
||||
events chan stream.Event
|
||||
result chan Result
|
||||
cancel context.CancelFunc
|
||||
status atomic.Int32
|
||||
startAt time.Time
|
||||
id string
|
||||
eng *engine.Engine
|
||||
events chan stream.Event
|
||||
result chan Result
|
||||
cancel context.CancelFunc
|
||||
status atomic.Int32
|
||||
startAt time.Time
|
||||
cachedResult Result
|
||||
resultOnce sync.Once
|
||||
eventsClose sync.Once
|
||||
}
|
||||
|
||||
// SpawnBackground creates and starts a background elf.
|
||||
@@ -102,6 +106,22 @@ func SpawnBackground(eng *engine.Engine, prompt string) *BackgroundElf {
|
||||
}
|
||||
|
||||
func (e *BackgroundElf) run(ctx context.Context, prompt string) {
|
||||
closeEvents := func() { e.eventsClose.Do(func() { close(e.events) }) }
|
||||
|
||||
defer func() {
|
||||
if r := recover(); r != nil {
|
||||
closeEvents()
|
||||
res := Result{
|
||||
ID: e.id,
|
||||
Status: StatusFailed,
|
||||
Error: fmt.Errorf("elf panicked: %v", r),
|
||||
Duration: time.Since(e.startAt),
|
||||
}
|
||||
e.status.Store(int32(StatusFailed))
|
||||
e.result <- res
|
||||
}
|
||||
}()
|
||||
|
||||
cb := func(evt stream.Event) {
|
||||
select {
|
||||
case e.events <- evt:
|
||||
@@ -111,7 +131,7 @@ func (e *BackgroundElf) run(ctx context.Context, prompt string) {
|
||||
|
||||
turn, err := e.eng.Submit(ctx, prompt, cb)
|
||||
|
||||
close(e.events)
|
||||
closeEvents()
|
||||
|
||||
r := Result{
|
||||
ID: e.id,
|
||||
@@ -149,5 +169,8 @@ func (e *BackgroundElf) Events() <-chan stream.Event { return e.events }
|
||||
func (e *BackgroundElf) Cancel() { e.cancel() }
|
||||
|
||||
func (e *BackgroundElf) Wait() Result {
|
||||
return <-e.result
|
||||
e.resultOnce.Do(func() {
|
||||
e.cachedResult = <-e.result
|
||||
})
|
||||
return e.cachedResult
|
||||
}
|
||||
|
||||
@@ -222,6 +222,94 @@ func TestManager_WaitAll(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestBackgroundElf_WaitIdempotent(t *testing.T) {
|
||||
mp := &mockProvider{
|
||||
name: "test",
|
||||
streams: []stream.Stream{newEventStream("hello")},
|
||||
}
|
||||
eng, _ := engine.New(engine.Config{Provider: mp, Tools: tool.NewRegistry()})
|
||||
elf := SpawnBackground(eng, "do something")
|
||||
|
||||
r1 := elf.Wait()
|
||||
r2 := elf.Wait() // must not deadlock
|
||||
|
||||
if r1.Status != r2.Status {
|
||||
t.Errorf("Wait() returned different statuses: %s vs %s", r1.Status, r2.Status)
|
||||
}
|
||||
if r1.Output != r2.Output {
|
||||
t.Errorf("Wait() returned different outputs: %q vs %q", r1.Output, r2.Output)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBackgroundElf_PanicRecovery(t *testing.T) {
|
||||
// A provider that panics on Stream() — simulates an engine crash
|
||||
panicProvider := &panicOnStreamProvider{}
|
||||
eng, _ := engine.New(engine.Config{Provider: panicProvider, Tools: tool.NewRegistry()})
|
||||
elf := SpawnBackground(eng, "do something")
|
||||
|
||||
result := elf.Wait() // must not hang
|
||||
|
||||
if result.Status != StatusFailed {
|
||||
t.Errorf("status = %s, want failed", result.Status)
|
||||
}
|
||||
if result.Error == nil {
|
||||
t.Error("error should be non-nil after panic recovery")
|
||||
}
|
||||
}
|
||||
|
||||
type panicOnStreamProvider struct{}
|
||||
|
||||
func (p *panicOnStreamProvider) Name() string { return "panic" }
|
||||
func (p *panicOnStreamProvider) DefaultModel() string { return "panic" }
|
||||
func (p *panicOnStreamProvider) Models(_ context.Context) ([]provider.ModelInfo, error) {
|
||||
return nil, nil
|
||||
}
|
||||
func (p *panicOnStreamProvider) Stream(_ context.Context, _ provider.Request) (stream.Stream, error) {
|
||||
panic("intentional test panic")
|
||||
}
|
||||
|
||||
func TestManager_CleanupRemovesMeta(t *testing.T) {
|
||||
mp := &mockProvider{
|
||||
name: "test",
|
||||
streams: []stream.Stream{newEventStream("result")},
|
||||
}
|
||||
|
||||
rtr := router.New(router.Config{})
|
||||
rtr.RegisterArm(&router.Arm{
|
||||
ID: "test/mock", Provider: mp, ModelName: "mock",
|
||||
Capabilities: provider.Capabilities{ToolUse: true},
|
||||
})
|
||||
|
||||
mgr := NewManager(ManagerConfig{Router: rtr, Tools: tool.NewRegistry()})
|
||||
e, _ := mgr.Spawn(context.Background(), router.TaskGeneration, "task", "", 30)
|
||||
e.Wait()
|
||||
|
||||
// Before cleanup: elf and meta both present
|
||||
mgr.mu.RLock()
|
||||
_, elfExists := mgr.elfs[e.ID()]
|
||||
_, metaExists := mgr.meta[e.ID()]
|
||||
mgr.mu.RUnlock()
|
||||
|
||||
if !elfExists || !metaExists {
|
||||
t.Fatal("elf and meta should exist before cleanup")
|
||||
}
|
||||
|
||||
mgr.Cleanup()
|
||||
|
||||
// After cleanup: both removed
|
||||
mgr.mu.RLock()
|
||||
_, elfExists = mgr.elfs[e.ID()]
|
||||
_, metaExists = mgr.meta[e.ID()]
|
||||
mgr.mu.RUnlock()
|
||||
|
||||
if elfExists {
|
||||
t.Error("elf should be removed after cleanup")
|
||||
}
|
||||
if metaExists {
|
||||
t.Error("meta should be removed after cleanup (was leaking)")
|
||||
}
|
||||
}
|
||||
|
||||
// slowEventStream blocks until context cancelled
|
||||
type slowEventStream struct {
|
||||
done bool
|
||||
|
||||
@@ -7,31 +7,38 @@ import (
|
||||
"sync"
|
||||
|
||||
"somegit.dev/Owlibou/gnoma/internal/engine"
|
||||
"somegit.dev/Owlibou/gnoma/internal/permission"
|
||||
"somegit.dev/Owlibou/gnoma/internal/provider"
|
||||
"somegit.dev/Owlibou/gnoma/internal/router"
|
||||
"somegit.dev/Owlibou/gnoma/internal/security"
|
||||
"somegit.dev/Owlibou/gnoma/internal/tool"
|
||||
)
|
||||
|
||||
// elfMeta tracks routing metadata for quality feedback.
|
||||
// elfMeta tracks routing metadata and pool reservations for quality feedback.
|
||||
type elfMeta struct {
|
||||
armID router.ArmID
|
||||
taskType router.TaskType
|
||||
decision router.RoutingDecision // holds pool reservations until elf completes
|
||||
}
|
||||
|
||||
// Manager spawns, tracks, and manages elfs.
|
||||
type Manager struct {
|
||||
mu sync.RWMutex
|
||||
elfs map[string]Elf
|
||||
meta map[string]elfMeta // routing metadata per elf ID
|
||||
router *router.Router
|
||||
tools *tool.Registry
|
||||
logger *slog.Logger
|
||||
mu sync.RWMutex
|
||||
elfs map[string]Elf
|
||||
meta map[string]elfMeta // routing metadata per elf ID
|
||||
router *router.Router
|
||||
tools *tool.Registry
|
||||
permissions *permission.Checker
|
||||
firewall *security.Firewall
|
||||
logger *slog.Logger
|
||||
}
|
||||
|
||||
type ManagerConfig struct {
|
||||
Router *router.Router
|
||||
Tools *tool.Registry
|
||||
Logger *slog.Logger
|
||||
Router *router.Router
|
||||
Tools *tool.Registry
|
||||
Permissions *permission.Checker // nil = allow all (unsafe; prefer passing parent checker)
|
||||
Firewall *security.Firewall // nil = no scanning
|
||||
Logger *slog.Logger
|
||||
}
|
||||
|
||||
func NewManager(cfg ManagerConfig) *Manager {
|
||||
@@ -40,11 +47,13 @@ func NewManager(cfg ManagerConfig) *Manager {
|
||||
logger = slog.Default()
|
||||
}
|
||||
return &Manager{
|
||||
elfs: make(map[string]Elf),
|
||||
meta: make(map[string]elfMeta),
|
||||
router: cfg.Router,
|
||||
tools: cfg.Tools,
|
||||
logger: logger,
|
||||
elfs: make(map[string]Elf),
|
||||
meta: make(map[string]elfMeta),
|
||||
router: cfg.Router,
|
||||
tools: cfg.Tools,
|
||||
permissions: cfg.Permissions,
|
||||
firewall: cfg.Firewall,
|
||||
logger: logger,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -71,16 +80,26 @@ func (m *Manager) Spawn(ctx context.Context, taskType router.TaskType, prompt, s
|
||||
"model", arm.ModelName,
|
||||
)
|
||||
|
||||
// Resolve permissions for this elf: inherit parent mode but never prompt
|
||||
// (no TUI in elf context — prompting would deadlock).
|
||||
elfPerms := m.permissions
|
||||
if elfPerms != nil {
|
||||
elfPerms = elfPerms.WithDenyPrompt()
|
||||
}
|
||||
|
||||
// Create independent engine for the elf
|
||||
eng, err := engine.New(engine.Config{
|
||||
Provider: arm.Provider,
|
||||
Tools: m.tools,
|
||||
System: systemPrompt,
|
||||
Model: arm.ModelName,
|
||||
MaxTurns: maxTurns,
|
||||
Logger: m.logger,
|
||||
Provider: arm.Provider,
|
||||
Tools: m.tools,
|
||||
Permissions: elfPerms,
|
||||
Firewall: m.firewall,
|
||||
System: systemPrompt,
|
||||
Model: arm.ModelName,
|
||||
MaxTurns: maxTurns,
|
||||
Logger: m.logger,
|
||||
})
|
||||
if err != nil {
|
||||
decision.Rollback()
|
||||
return nil, fmt.Errorf("create elf engine: %w", err)
|
||||
}
|
||||
|
||||
@@ -88,14 +107,14 @@ func (m *Manager) Spawn(ctx context.Context, taskType router.TaskType, prompt, s
|
||||
|
||||
m.mu.Lock()
|
||||
m.elfs[elf.ID()] = elf
|
||||
m.meta[elf.ID()] = elfMeta{armID: arm.ID, taskType: taskType}
|
||||
m.meta[elf.ID()] = elfMeta{armID: arm.ID, taskType: taskType, decision: decision}
|
||||
m.mu.Unlock()
|
||||
|
||||
m.logger.Info("elf spawned", "id", elf.ID(), "arm", arm.ID)
|
||||
return elf, nil
|
||||
}
|
||||
|
||||
// ReportResult reports an elf's outcome to the router for quality feedback.
|
||||
// ReportResult commits pool reservations and reports an elf's outcome to the router.
|
||||
func (m *Manager) ReportResult(result Result) {
|
||||
m.mu.RLock()
|
||||
meta, ok := m.meta[result.ID]
|
||||
@@ -105,6 +124,11 @@ func (m *Manager) ReportResult(result Result) {
|
||||
return
|
||||
}
|
||||
|
||||
// Commit pool reservations with actual token consumption.
|
||||
// Cancelled/failed elfs still commit what they consumed; a zero commit is
|
||||
// safe — it just moves reserved tokens to used at rate 0.
|
||||
meta.decision.Commit(int(result.Usage.TotalTokens()))
|
||||
|
||||
m.router.ReportOutcome(router.Outcome{
|
||||
ArmID: meta.armID,
|
||||
TaskType: meta.taskType,
|
||||
@@ -116,13 +140,19 @@ func (m *Manager) ReportResult(result Result) {
|
||||
|
||||
// SpawnWithProvider creates an elf using a specific provider (bypasses router).
|
||||
func (m *Manager) SpawnWithProvider(prov provider.Provider, model, prompt, systemPrompt string, maxTurns int) (Elf, error) {
|
||||
elfPerms := m.permissions
|
||||
if elfPerms != nil {
|
||||
elfPerms = elfPerms.WithDenyPrompt()
|
||||
}
|
||||
eng, err := engine.New(engine.Config{
|
||||
Provider: prov,
|
||||
Tools: m.tools,
|
||||
System: systemPrompt,
|
||||
Model: model,
|
||||
MaxTurns: maxTurns,
|
||||
Logger: m.logger,
|
||||
Provider: prov,
|
||||
Tools: m.tools,
|
||||
Permissions: elfPerms,
|
||||
Firewall: m.firewall,
|
||||
System: systemPrompt,
|
||||
Model: model,
|
||||
MaxTurns: maxTurns,
|
||||
Logger: m.logger,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("create elf engine: %w", err)
|
||||
@@ -207,6 +237,7 @@ func (m *Manager) Cleanup() {
|
||||
s := e.Status()
|
||||
if s == StatusCompleted || s == StatusFailed || s == StatusCancelled {
|
||||
delete(m.elfs, id)
|
||||
delete(m.meta, id)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user