fix: resolve 18 correctness, durability, and honesty gaps - #47
Merged
Conversation
Agent core: - EventStream.Wait() now returns the run's terminal error; ErrorDelta and the stream close error are emitted from a single point and always agree - sub-agent failures mark the parent tool result as errored, and child agents inherit the parent's StepRunner (namespaced steps) for durability - provider fallback now also triggers on mid-stream errors that arrive before any forwarded content; later errors propagate without duplication - ConfigContent.Model is honored via the new types.ModelSwitcher seam, implemented by all four provider adapters Durability: - every tree mutation (archive/restore/checkpoint/rewind/branch/compact/ set-active) writes WAL ops in a single transaction - new agent/store/filewal: production append-only JSONL WAL with fsync on commit and torn-tail repair; agent/store/walrecover heals a store from committed-unapplied transactions; RecoverAndLoadTree wires it at startup - checkpoints round-trip: Store.ListCheckpoints (pgstore + memstore) feeds LoadTreeFromStore so rewind works after restore - DBOS adapter docs now describe the real sequential durable execution Providers & CLI: - Generate(ctx, prompt) implemented on anthropic/openai/google adapters via shared types.GenerateText (previously ollama-only seam) - MarkedTool implements RichTool: approval-wrapped rich results keep blocks - PDFs map to native document/file parts in anthropic and openai adapters, so ContentSupport claims are now truthful for every provider - new --embed-provider / SAIGE_EMBED_PROVIDER decouples the CLI embedding provider from the LLM provider RAG: - document writes are transactional (pgstore tx; sections/variants now actually persisted); replace keeps the prior doc on mid-ingest failure - metadata filters push down into SQL, removing the limit*3 over-fetch - partial retriever failure returns surviving hits + ErrPartialSearch; RRF k is configurable via WithFusionK - WithGraph registers the graph retriever and deletes graph episodes on document delete/replace (knowledge gains DeleteEpisodes end to end) Knowledge graph: - GroupID is real tenant isolation: group-scoped entity uniqueness, dedup, relations, and search (schema migration included; empty group = legacy) - search errors propagate (ErrPartialSearch for partial fan-out failures) and episode metadata persists to a JSONB column and rounds back Eval: - scorer/metric errors record per-case and are excluded from aggregates instead of aborting the suite or silently scoring zero
- bump Go to 1.25.12 (GO-2026-5856 crypto/tls) and goldmark to v1.7.17 (GO-2026-5320 XSS) - extract embedVariants/writeDocument/enrichGraph from pipeline Ingest and retrieveAll/fuseHits from Search to bring cyclomatic complexity under the gocyclo threshold; no behavior change
- rag/pgstore: transactional document-tree persistence, atomic replace with mid-transaction rollback, metadata filter pushdown past the old limit*3 window, JSONB filter semantics, vector ordering (7 tests) - knowledge/pgstore: group isolation for entities/relations/search, episode metadata round-trip, DeleteEpisodes cascade, in-place migration upgrade from the historical unscoped schema (5 tests) - integration/: opt-in E2E suite (Ollama/Postgres/DBOS-gated, hermetic skips) covering the full agent loop, handoffs, structured output, RAG+KG round trip with graph cleanup on delete, WAL crash recovery via RecoverAndLoadTree, DBOS durable runs and idempotent replay; includes docker-compose infra and Justfile recipes - dbos: register RunInput/RunOutput/StepResult with gob — the serializer encodes them as interface values, so unregistered types fail at runtime - tree.FromStore/LoadTreeFromStore accept tree.Option and RecoverAndLoadTree re-attaches the WAL, so recovered sessions keep write-ahead protection - FallbackError.Error() now includes the underlying provider errors
Migrations (data stranding): - backfill kg_entity/kg_relation group_id from episode mentions where the group is unambiguous; cross-tenant-merged entities deliberately stay in the default group rather than leak between tenants - backfill agent_branch/agent_checkpoint conversation_id from each row's root node via recursive CTE; orphans stay in the legacy namespace Agent core: - fallback relay: UsageDelta (emitted at message_start before content) no longer latches the no-fallback gate, and every relay return path drains an abandoned source stream so provider goroutines can't leak - RunDurable uses stream.Wait() (authoritative close error) instead of a racy in-band ErrorDelta, and an empty response under a dead context is reported as ErrStreamCanceled, so cancelled runs can't masquerade as success - ConfigContent.Model now propagates through fallback and retry wrappers (both implement ModelSwitcher); an unswitchable provider logs a warning instead of silently ignoring the requested model - new Agent.Checkpoint persists checkpoints through the Store so they round-trip without WAL recovery; Tree.Checkpoint docs state the contract Durability: - filewal rolls back partial writes by truncating to the pre-write offset (self-disabling if rollback fails) so mid-log corruption is impossible - filewal.Compact atomically rewrites the log keeping only unapplied transactions; RecoverWAL compacts after a successful pass, bounding growth to one session RAG: - Pipeline.Delete removes the store document before graph episodes, so a failed delete can no longer destroy graph facts for a live document
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Single hardening sweep resolving 18 audited issues across the agent core, durability layer, providers, CLI, RAG, knowledge graph, and eval packages — plus DB-backed/E2E integration suites and fixes for all 10 findings from a high-effort adversarial review of the diff.
Agent core
EventStream.Wait()now returns the run's terminal error.ErrorDeltaand the stream close error are emitted from a single point and always agree;RunDurableconsults the authoritative close error, and a cancelled run can never masquerade as success.IsError, and child agents inherit the parent'sStepRunnerwithsub-<toolCallID>-namespaced step names.fallback.Providerrelays the stream and retries the next provider when an error arrives before any content-bearing delta (usage deltas don't latch the gate, so Anthropic's message_start usage can't disable fallback); abandoned streams are always drained so adapter goroutines can't leak.types.ModelSwitcher(WithModel) on all four adapters and the fallback/retry wrappers; unswitchable providers log a warning instead of silently ignoring the model.Durability
agent/store/filewal(append-only JSONL, fsync on commit, torn-tail repair, partial-write rollback, atomicCompact) andwalrecover.RecoverWAL;RecoverAndLoadTreeheals the store at startup, re-attaches the WAL, and compacts the log so growth is bounded per session.types.Store.ListCheckpoints(pgstore + memstore),LoadTreeFromStorerestores checkpoints, and the newAgent.Checkpointpersists them without requiring WAL recovery.ListCheckpointsis breaking for external Store implementers (v0.x).RunInput/RunOutput/StepResultregistered with gob (they travel as interface values — unregistered types failed at runtime on real DBOS).Providers & CLI
Generate(ctx, prompt)on anthropic/openai/google adapters via sharedtypes.GenerateText(was ollama-only).MarkedToolimplementsRichTool— approval-wrapped rich results keep their blocks.ContentSupportclaim truthful.--embed-provider/SAIGE_EMBED_PROVIDERdecouples CLI embeddings from the LLM provider.RAG
ReplaceDocumentkeeps the prior doc on any mid-ingest failure; KG-stage failure surfaces asErrPartialIngestwith the doc committed;Deleteremoves the document before its graph facts so a failed delete can't strand a live doc without provenance.limit×3over-fetch is gone.ErrPartialSearch+ surviving hits otherwise); RRF k configurable viaWithFusionK.WithGraphregisters the graph retriever, and document delete/replace removes derived graph episodes via the newknowledgeDeleteEpisodes(groupID).Knowledge graph
group_idon entities/relations with group-scoped uniqueness, dedup, and search. Migration backfills legacy rows from episode mentions where unambiguous; entities the old cross-tenant merge bug already fused stay in the default group deliberately (assigning either tenant would leak data).ErrPartialSearchfor partial fan-out failure); episode metadata persists and rounds back through provenance.Eval
Migrations & compatibility notes
agent_branch/agent_checkpointare conversation-scoped; the migration backfillsconversation_idfrom each row's root node (recursive CTE). Orphans stay reachable via the legacy""namespace.ctxthroughTree.Archive/Restore/Checkpoint/Rewind/SetActive.Evaluation performed
Test plan
go build ./...,go vet ./...clean;gofmtclean on all changed filesgo test ./...green (49 packages);go test -race ./agent/...greengolangci-lint run --new-from-rev=main— 0 issues; CI lint/vuln failures fixed (Go 1.25.12, goldmark 1.7.17, gocyclo refactors)