Skip to content

fix(session): admit a conversation append on the stream port's capability - #572

Merged
M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/append-checks-stream-port-capability
Oct 23, 2026
Merged

M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/append-checks-stream-port-capability

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

A conversation append on a SQLite-backed DriverConversationStore is refused with unsupported capability conditional_stream_append before anything is written.

Root cause. admit_append (threads/driver/append.rs) required both ValidatedBatch and ConditionalStreamAppend from 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) lists ValidatedBatch but not ConditionalStreamAppend.
  • SqliteStreams::capabilities() (sqlite/streams/mod.rs) lists ConditionalStreamAppend.

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: ValidatedBatch on the documents (the intent and metadata batch) and ConditionalStreamAppend on the streams (the message write). The probe and append steps after admission are unchanged.

Evidence.

  • On openhuman main, with tinyagents at 1f24dbe and tinystoragedrivers at f05fbd2, the embed profiles example on storage-sqlite fails at threads_message_append with unsupported capability conditional_stream_append. That's a public RPC path: openhuman's threads/ops/crud.rs append goes through DriverConversationStore over the default sqlite: backend.
  • A direct SQLite repro in openhuman is pending on the shared CI host. A follow-up openhuman pin PR will keep it as a regression test.

Why tests missed it. The sqlite_* thread tests in driver_schema_tests.rs (sqlite_explicit_admission_refuses_missing_declaration_before_read, sqlite_prepared_actor_retains_original_native_fence_and_scope) construct MemoryStorage, not SQLite. Their tempdir is unused, and memory documents advertise ConditionalStreamAppend.

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 ConditionalStreamAppend is still refused, with the same error.

Tests

The append fixtures gain two capability-splitting fault actions (DocsLackStreamAppend, StreamsLackStreamAppend), and there are two new tests in driver_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: still Unsupported(ConditionalStreamAppend), and nothing is stored.

  • cargo fmt --check: CI

  • cargo clippy --all-targets -- -D warnings: CI

  • cargo clippy --all-targets --all-features -- -D warnings: CI

  • cargo build --all-targets: CI

  • cargo build --all-targets --all-features: CI

  • cargo test: CI

  • cargo test --all-features: CI

Documentation

The rule is stated in a code comment in admit_append.

Summary by CodeRabbit

  • Bug Fixes
    • Append operations now check conditional stream-append support on the stream storage port, rather than requiring that capability from the document storage port.
    • When the stream storage port lacks conditional stream-append support, append operations are rejected with an unsupported-capability error, and no messages are published.
    • Append operations can now succeed when the stream storage port supports conditional appends, even if the document storage port does not.

…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.
@tinysweeper

tinysweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: 751a2d786c576073e1e946c75f60ab2cdb5aca02

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 3 of 3 files.
Pull request purpose (title): fix(session): admit…

Diagnostics and supporting details

Full summary

Pull 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

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 2 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

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

  • Modified — Per-port capability admission for conversation append: Appends are admitted when the document port carries `ValidatedBatch` and the stream port carries `ConditionalStreamAppend`, matching the port each capability is actually used on, so SQLite-shaped backends no longer fail admission; a stream port lacking the capability is still refused with `Unsupported(ConditionalStreamAppend)`. (crates/tinyagents-session/src/threads/driver/append.rs impl DriverConversationStore {)
  • Added — Capability-splitting fault actions in append fixtures: Test fixtures can withdraw `ConditionalStreamAppend` from the document port only or the stream port only, enabling tests of the split-port shape. (crates/tinyagents-session/src/threads/driver_append_fixtures.rs pub(super) enum Action {, crates/tinyagents-session/src/threads/driver_append_fixtures.rs struct Documents {, crates/tinyagents-session/src/threads/driver_append_fixtures.rs struct Streams {)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Answered lane reported no defect introduced by this file; it noted the two new tests exercise the port-split admission in `admit_append`, one confirming admission when the stream port carries `ConditionalStreamAppend` and the document port does not, the other confirming refusal with the native `Unsupported` capability preserved, and that both anchor on fixtures whose `capabilities()` overrides match the actions used.
  • Lane summary: The two new tests exercise the port-split capability admission in `admit_append`: one confirms a stream port that carries `ConditionalStreamAppend` admits the append when the document port does not, the other confirms refusal when the stream port lacks it, with the native `Unsupported` capability preserved. Both anchor on fixtures whose `capabilities()` overrides match the actions used, and the assertions match the behaviour of `admit_append`. I found no defect introduced by this file. _The code index is behind this pull request (indexed at `8c06369521a8`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Answered lane reported no findings: the added tests use existing `actor`/`faulted`/`Fault`/`Action` fixtures, introduce no untrusted input, network, subprocess, credential handling, or widened permissions, and nothing in the diff is a security or behavioral defect.
  • Lane summary: The diff adds two unit tests to the driver append test module: one asserts that an append still succeeds when the document port does not advertise ConditionalStreamAppend but the stream port does, and the other asserts a typed `Unsupported(ConditionalStreamAppend)` refusal when the stream port lacks the capability. Both use the existing `actor`/`faulted`/`Fault`/`Action::DocsLackStreamAppend`/`Action::StreamsLackStreamAppend` fixtures, carry no `#[cfg(test)]` of their own, and match the repository's sibling-test-module layout. No untrusted input, network, subprocess, or credential handling is introduced, and nothing in the diff is a security or behavioural defect. _The code index is behind this pull request (indexed at `8c06369521a8`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Answered lane reported that the change moves the `ConditionalStreamAppend` check from the document port to the stream port and that the two new tests pin both directions, with assertions that are real (a one-message committed prefix and the exact capability in the error kind), so the behavior change is covered.
  • Lane summary: The change moves the ConditionalStreamAppend capability check from the document port to the stream port, and the two new tests pin both directions: append succeeds when only the stream port advertises it, and is refused with a typed Unsupported error when the stream port lacks it, with nothing written. I checked the fixtures and append path in the diff; the assertions are real (one asserts a one-message committed prefix, the other matches the exact capability in the error kind), so the behaviour change is covered. _The code index is behind this pull request (indexed at `8c06369521a8`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Answered lane reported no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `8c06369521a8`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Answered lane reported that the description accurately describes the driver change, the two new fixture fault actions, and both named new tests including the stream-port refusal test, and that nothing in the diff contradicts it; it did not confirm the tinystoragedrivers SQLite capability claims because that submodule is not checked out in the review tree.
  • Lane summary: The PR description accurately describes the driver change and the fixtures' two new fault actions; the newly added tests in driver_append_tests.rs are also described (both named in the body, including the stream-port refusal test). I did not confirm the tinystoragedrivers SQLite capability claims cited in the body because that submodule is not checked out in the review tree, but nothing in the diff contradicts the description. _The code index is behind this pull request (indexed at `8c06369521a8`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000264
  • Tokens: 164100 input · 3423 output · 37376 cached · 222 embedding
  • Updated: 2026-10-23T23:13:46Z
Head State Pass summary
8c06369521a8 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T23:08:09Z)
751a2d786c57 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T23:13:46Z)

tinysweeper 0.1.2

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 60c1bb6b-b6a2-4f6c-94f5-24c3aaf7ad8e
📥 Commits

Reviewing files that changed from the base of the PR and between 8c06369 and 751a2d7.

📒 Files selected for processing: 1
  • crates/tinyagents-session/src/threads/driver_append_tests.rs
🚧 Files skipped from review as they are similar to previous changes: 1
  • crates/tinyagents-session/src/threads/driver_append_tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Append admission checks ValidatedBatch on the document store and ConditionalStreamAppend on the stream store. Fixture modes and tests cover append outcomes when the stream store provides or lacks that capability.

Changes: Append capability validation

Layer / File(s) Summary
Port-specific capability checks
crates/tinyagents-session/src/threads/driver/append.rs, crates/tinyagents-session/src/threads/driver_append_fixtures.rs, crates/tinyagents-session/src/threads/driver_append_tests.rs
Fixture modes control which port advertises ConditionalStreamAppend. admit_append checks that capability on the stream port. Tests confirm append succeeds when the document port lacks it and fails without publishing messages when the stream port lacks it.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

Suggested reviewers: senamakel

Merge Risk: ⚪ Minimal · up to 751a2

No actionable merge-blocking risk is established for this change; complete the normal test checks before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: append admission now uses the stream port's conditional append capability.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the stream’s small sign,
While documents keep their own in line.
If stream append is present, messages show;
If absent, no messages flow.
The tests confirm each path is right,
Then hop away beneath the moonlight.

Comment @coderabbitai help to get the list of available commands.

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0003

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 23, 2026
@M3gA-Mind
M3gA-Mind merged commit 540997d into tinyhumansai:main Oct 23, 2026
18 checks passed
M3gA-Mind added a commit to tinyhumansai/openhuman that referenced this pull request Oct 23, 2026
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant