Repository navigation
fix(session): admit a conversation append on the stream port's capability - #572
Conversation
…lity `admit_append` required ValidatedBatch and ConditionalStreamAppend from the document port. The append writes messages through the stream port, and a backend may advertise the two on separate ports: SQLite's document store lists ValidatedBatch but not ConditionalStreamAppend, which only its stream store does. So every append on a SQLite-backed conversation store was refused with "unsupported capability conditional_stream_append" before writing anything, while memory-backed tests (whose documents list everything) passed. Check each capability on the port that uses it: ValidatedBatch on the documents, ConditionalStreamAppend on the streams. Tests cover both shapes with the append fixtures: a document port without the stream capability plus a stream port with it appends, and a stream port without it is still refused.
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): fix(session): admit a conversation append on the stream port's capability. Tiny Sweeper completed its review; deterministic results follow. Review snapshot
Completeness: Complete What changed`DriverConversationStore` append admission in `crates/tinyagents-session/src/threads/driver/append.rs` now checks `ValidatedBatch` against `self.docs.capabilities()` and `ConditionalStreamAppend` against `self.streams.capabilities()` rather than requiring both from the document port; the probe and append steps after admission are unchanged. The append fixtures gain `DocsLackStreamAppend` and `StreamsLackStreamAppend` actions that withdraw `ConditionalStreamAppend` from the respective port's `capabilities()`. Two new tests in `driver_append_tests.rs` pin the admitting case (document port lacks the capability, stream port has it, append succeeds and the message is stored) and the refusing case (stream port lacks it, `Unsupported(ConditionalStreamAppend)` typed error, nothing stored). Behaviorally, appends on backends that split the capability across ports such as SQLite are now admitted, while a stream port lacking `ConditionalStreamAppend` is still refused with the same error. Features
TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
Every thread message append on the default SQLite backend was refused with "unsupported capability conditional_stream_append": TinyAgents' admit_append required that capability from the document port, while SQLite advertises it only on its stream port (the one the append uses). tinyhumansai/tinyagents#572 checks each capability on the port that uses it. Move vendor/tinyagents 1f24dbe -> 540997d (#572's merge, tinyagents main; a fast-forward of #572's commits only, no manifest change) and keep the reproduction as a regression test: a plain message appends on a workspace opened on the production default backend kind (SQLite).
Summary
A conversation append on a SQLite-backed
DriverConversationStoreis refused withunsupported capability conditional_stream_appendbefore anything is written.Root cause.
admit_append(threads/driver/append.rs) required bothValidatedBatchandConditionalStreamAppendfrom the document port:self.docs.capabilities(). The append writes its messages through the stream port (self.streams). A backend may advertise the two on separate ports, and tinystoragedrivers' SQLite driver does:SqliteDocuments::capabilities()(tinystoragedrivers-document/src/sqlite/documents/mod.rs) listsValidatedBatchbut notConditionalStreamAppend.SqliteStreams::capabilities()(sqlite/streams/mod.rs) listsConditionalStreamAppend.So every SQLite append failed admission, while memory-backed stores passed, because their documents list everything.
Fix. Each capability is checked on the port that uses it:
ValidatedBatchon the documents (the intent and metadata batch) andConditionalStreamAppendon the streams (the message write). The probe and append steps after admission are unchanged.Evidence.
1f24dbeand tinystoragedrivers atf05fbd2, the embedprofilesexample onstorage-sqlitefails atthreads_message_appendwithunsupported capability conditional_stream_append. That's a public RPC path: openhuman'sthreads/ops/crud.rsappend goes throughDriverConversationStoreover the defaultsqlite:backend.Why tests missed it. The
sqlite_*thread tests indriver_schema_tests.rs(sqlite_explicit_admission_refuses_missing_declaration_before_read,sqlite_prepared_actor_retains_original_native_fence_and_scope) constructMemoryStorage, not SQLite. Their tempdir is unused, and memory documents advertiseConditionalStreamAppend.API Or Behavior Changes
Appends on backends that split the capability across ports, such as SQLite, are now admitted. A backend whose stream port lacks
ConditionalStreamAppendis still refused, with the same error.Tests
The append fixtures gain two capability-splitting fault actions (
DocsLackStreamAppend,StreamsLackStreamAppend), and there are two new tests indriver_append_tests.rs:an_append_admits_a_stream_capability_its_document_port_does_not_list: a document port without the capability plus a stream port with it, as in SQLite. The append succeeds and the message is stored. This fails before the fix.an_append_is_refused_when_the_stream_port_lacks_conditional_appends: stillUnsupported(ConditionalStreamAppend), and nothing is stored.cargo fmt --check: CIcargo clippy --all-targets -- -D warnings: CIcargo clippy --all-targets --all-features -- -D warnings: CIcargo build --all-targets: CIcargo build --all-targets --all-features: CIcargo test: CIcargo test --all-features: CIDocumentation
The rule is stated in a code comment in
admit_append.Summary by CodeRabbit