Skip to content

feat(sqlite): clock-injectable blob expiry and sweep - #41

Merged
senamakel merged 3 commits into
mainfrom
storage-7781-tinystoragedrivers-1
Oct 14, 2026
Merged

senamakel merged 3 commits into
mainfrom
storage-7781-tinystoragedrivers-1

Conversation

@senamakel

@senamakel senamakel commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Part of tinyhumansai/openhuman#7781, tinyhumansai/openhuman#7522

SQLite blob expiry (get, get_range, head, delete, list, put_expiring and sweep_expired) read SystemTime::now() directly. SqliteBlobs now takes the storage's existing Clock (the one documents and fencing already use), so SqliteStorage::with_clock drives blob expiry too.

  • The file driver already routes blob expiry through its injected clock (Db::now), so no change was needed there.
  • tests/blob_sweep.rs no longer sleeps; it advances a manual clock. New test pins the exact expiry boundary (live at ttl-1ms, gone at ttl) for get/head/list.
  • fmt, clippy -D warnings and cargo test -p tinystoragedrivers-sqlite (incl. conformance) pass.

Deferred: the other items in openhuman#7781 (tinybus move, tinyagents pin, module re-pins, profile-scope blob sweep) live in other repos.

Summary by CodeRabbit

  • Bug Fixes
    • Blob expiration checks and cleanup now follow the configured clock, keeping availability consistent when retrieving, inspecting, listing, or cleaning up blobs. Expiring blobs remain available until their expiration time and are treated as expired afterward, aligning blob expiration behavior with document expiration.

SqliteBlobs read SystemTime directly, so blob expiry tests had to sleep.
It now takes the storage's Clock (the one documents already use), set via
SqliteStorage::with_clock. The file driver already routes blob expiry
through its injected clock.

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@tinysweeper

tinysweeper Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Tiny Sweeper reviewed this change across 5 lane(s) and found 0 active actionable finding(s). SqliteBlobs now reads every expiry decision through the storage's injected Clock instead of SystemTime::now(), so SqliteStorage::with_clock drives blob lifetimes exactly as it already drives documents. The delta since the last review is test-only. The code index is behind this pull request (indexed at 23bad66), so retrieved context may be out of date.

State: Incomplete
Priority: none
Reviewed head: d3d7c920c3a4
Updated: 2026-10-14T04:27:11Z

Review snapshot

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

Completeness: Incomplete
Test assessment: Test coverage is assessed from changed tests and lane evidence; execution is not claimed without trusted check data.

What changed

SqliteBlobs gained a clock field populated by SqliteStorage when constructing scope blob stores, and all internal expiry decisions now read the injected clock rather than the wall clock. Tests updated to drive time through a manual clock instead of sleeping.

Features

None identified with supported citations.

Tests

  • modification — The backend sweep hook test now installs a manual clock, advances it by 150 ms via an atomic instead of sleeping, and still asserts that only lapsed blobs are reclaimed while young and forever keys survive.: Deterministic replacement of a real-time sleep; preserves what is asserted while removing timing flakiness. (crates/tinystoragedrivers-sqlite/tests/blob_sweep.rs#async fn the_backend_sweep_hook_reclaims_only_lapsed_blobs() {)
  • addition — Pins the expiry boundary against the injected clock: with a 10 s TTL the blob is present via get, head, and list at ttl-1ms, and absent via all three at ttl.: Boundary assertion matches the LIVE predicate (expires_at_ms > now) and pins that reads follow the injected clock rather than real time. (crates/tinystoragedrivers-sqlite/tests/blob_sweep.rs#async fn the_backend_sweep_hook_reclaims_only_lapsed_blobs() {)
  • modification — The S3 sweeper test replaces Duration::from_millis(1_000) with the equivalent Duration::from_secs(1) on a blob that must survive a 150 ms advance.: Semantically identical duration; only widens the margin and does not change what is asserted. (crates/tinystoragedrivers-s3/src/blobs/mod_tests.rs#async fn the_sweeper_deletes_only_lapsed_objects_of_its_scope() {)
  • Unreviewed: tinysweeper/tests

Findings

No active actionable findings.

Could not review: tinysweeper/tests

Before merge

  • Complete the tests review for tinysweeper/tests.

How this fits together

flowchart LR
  n0["..._deletes_only_lapsed_objects_of_its_scope<br/>changed"]:::changed
  n1["SqliteStorage<br/>changed"]:::changed
  n2["...end_sweep_hook_reclaims_only_lapsed_blobs<br/>changed"]:::changed
  n3["handle"]:::impacted
  n4["Result"]:::impacted
  n5["StorageBackend"]:::impacted
  n6["get"]:::impacted
  n7["now_ms"]:::impacted
  n8["head"]:::impacted
  n0 -->|calls| n3
  n0 -->|tests| n3
  n1 -->|implements| n5
  n2 -->|uses| n5
  n6 -->|uses| n4
  n6 -->|calls| n7
  n8 -->|uses| n4
  n8 -->|calls| n6
  n8 -->|calls| n7
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 4 files; 0 findings. _The code index is behind this pull request (indexed at `23bad66e4429`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: Reviewed 4 files; 0 findings. _The code index is behind this pull request (indexed at `23bad66e4429`), so retrieved context may be out of date._

tests

  • Conclusion: Neutral
  • Scope reviewed: incomplete; unanswered: tinysweeper/tests
  • Lane summary: No reviewer produced a usable response.

commits

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: Nothing sensitive found in what this pull request commits.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Lane summary: The change threads the storage's existing `Clock` into `SqliteBlobs`, replacing the free `now_ms()` system-clock helper with `self.now_ms()`, and rewrites the blob sweep test to advance a manual clock instead of sleeping. The wiring in `storage/mod.rs` passes `Arc::clone(&self.clock)` correctly, and the new boundary test matches the `LIVE` predicate (`expires_at_ms > ?`). The description matches the diff; I found nothing reportable. _The code index is behind this pull request (indexed at `23bad66e4429`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000910
  • Tokens: 412210 input · 11359 output · 65408 cached · 319 embedding
Head State Pass summary
cc6d68d92431 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-14T04:13:08Z)
aa4826229cd8 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-14T04:21:11Z)
d3d7c920c3a4 incomplete 0 active finding(s), 0 resolved finding(s) (at 2026-10-14T04:27:11Z)

tinysweeper 0.1.2

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-14T04:23:59.541965Z d3d7c92 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

  • Run on-demand review

This review includes 2 billable files and costs up to $0.50.

Or wait 45 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b814a01a-88a5-4385-bbb8-48048e035a26
📥 Commits

Reviewing files that changed from the base of the PR and between cc6d68d and d3d7c92.

⛔ Files ignored due to path filters: 1
  • proof-workspaces/document-callback/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing: 2
  • crates/tinystoragedrivers-s3/src/blobs/mod_tests.rs
  • crates/tinystoragedrivers-sqlite/tests/blob_sweep.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b352b549-903f-47a4-8ec7-e4399dc5edbd
📥 Commits

Reviewing files that changed from the base of the PR and between 44745d6 and cc6d68d.

📒 Files selected for processing: 3
  • crates/tinystoragedrivers-sqlite/src/blobs/mod.rs
  • crates/tinystoragedrivers-sqlite/src/storage/mod.rs
  • crates/tinystoragedrivers-sqlite/tests/blob_sweep.rs

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

📝 Walkthrough

SQLite blob expiry now uses the clock injected into storage. Blob timestamps, expiry checks, and sweeps use that clock. Tests use a manual clock to verify sweep behavior and expiry boundaries.

Changes: SQLite blob expiry clock

Layer / File(s) Summary
Inject and store the expiry clock
crates/tinystoragedrivers-sqlite/src/blobs/mod.rs, crates/tinystoragedrivers-sqlite/src/storage/mod.rs
SqliteBlobs stores a Clock and converts its readings to SQLite milliseconds. SqliteStorage passes its clock to SqliteBlobs, and the with_clock documentation describes its use for blob expiry.
Use the clock for expiry operations
crates/tinystoragedrivers-sqlite/src/blobs/mod.rs, crates/tinystoragedrivers-sqlite/tests/blob_sweep.rs
Blob storage, reads, range reads, metadata reads, deletion, listing, and sweeps use the injected clock. Tests advance a manual clock and check behavior before and at the expiry time.

Priority: ⬇️ Low

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

Change: Feature

Merge Risk: ⚪ Minimal · up to cc6d6

SQLite blob expiry now follows the configured clock consistently across storage and expiry checks. No actionable merge risk remains in the reviewed changes.

Security architecture risk: 🔵 Low · up to cc6d6

This is a bounded expiry-timekeeping change. Existing scope isolation and transactional controls remain intact. Custom clocks must remain compatible across independently opened handles and database reopen, but that deployment assumption is not established here.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — Clock authority belongs to the storage-configuring caller and is inherited by subsequently constructed scoped blob handles and selected databases. Its effect can span those handles, but each blob query and sweep remains restricted to its scope. Deployment tenant mappings and whether any untrusted caller can influence clock configuration remain unverified.

Trust Boundaries and Controls

  • observed — SqliteBlobs construction remains crate-private and storage-owned. Blob keys, TTLs, ranges, and list prefixes do not select the clock. The base-to-head comparison preserves key validation, bound SQL parameters, scope predicates, and public operation reachability; it does not establish a new attacker-controlled route to clock authority.

Hardening Proposals

  • proposed — Document that custom-clock users sharing persisted data must maintain compatible time semantics across reopen and independent storage instances, and that replacing the storage clock does not update already-created handles.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 35.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 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: injected clock support for SQLite blob expiry and sweep operations.
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.
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

A rabbit checks the ticking clock,
Then hops past blobs that meet the lock.
At fourteen-nine, they’re still in sight,
At fifteen thousand, gone outright.
No waiting sleeps; time moves on,
The sweep runs when the clock has drawn.

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.

             $0.0040 · 423,041 in / 47,889 out · 22,656 cached (5%) · ladder/vectors-oai3, deepseek-v4.1-flash · 282 embedded
critique:    $0.0016 · 170,747 in / 18,530 out · 10,624 cached (6%) · deepseek-v4.1-flash
security:    $0.0008 · 151,296 in / 6,237 out  · 8,960 cached (6%)  · deepseek-v4.1-flash
tests:       $0.0009 · 62,950 in  / 11,589 out · 1,024 cached (2%)  · deepseek-v4.1-flash
description: $0.0006 · 33,413 in  / 8,819 out  · 2,048 cached (6%)  · deepseek-v4.1-flash

Co-authored-by: Medulla <medulla@tinyhumans.ai>
tinysweeper[bot]
tinysweeper Bot previously approved these changes Oct 14, 2026

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

             $0.0042 · 272,387 in / 35,868 out · 47,104 cached (17%) · ladder/vectors-oai3, deepseek-v4.1-flash · 319 embedded
critique:    $0.0015 · 106,194 in / 13,442 out · 24,064 cached (23%) · deepseek-v4.1-flash
security:    $0.0013 · 69,345 in  / 6,089 out  · 19,328 cached (28%) · deepseek-v4.1-flash
tests:       $0.0004 · 59,143 in  / 3,351 out  · 1,920 cached (3%)   · deepseek-v4.1-flash
description: $0.0009 · 32,683 in  / 12,114 out · 1,792 cached (5%)   · deepseek-v4.1-flash

The Release v0.10.0 commit on main left this lockfile at 0.9.0, so the --locked proof build fails on main too.

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@tinysweeper
tinysweeper Bot dismissed their stale review October 14, 2026 04:27

tinysweeper could not review the latest push, so its earlier approval no longer speaks for this pull request.

@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, but could not review everything, so this is not an approval: tinysweeper/tests.

             $0.0009 · 412,210 in / 11,359 out · 65,408 cached (16%) · ladder/vectors-oai3, deepseek-v4.1-flash · 319 embedded
critique:    $0.0002 · 192,343 in / 2,584 out  · 24,704 cached (13%) · deepseek-v4.1-flash
security:    $0.0002 · 145,242 in / 2,397 out  · 15,872 cached (11%) · deepseek-v4.1-flash
tests:       $0.0003 · 34,717 in  / 4,869 out  · 14,336 cached (41%) · deepseek-v4.1-flash
description: $0.0000 · 29,367 in  / 638 out    · 6,272 cached (21%)  · deepseek-v4.1-flash

@senamakel
senamakel merged commit ec0aee3 into main Oct 14, 2026
27 checks passed
@senamakel
senamakel deleted the storage-7781-tinystoragedrivers-1 branch October 24, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant