Repository navigation
feat(storage): devices, notifications and task sources on the storage ports - #7181
Merged
Merged
Conversation
Reworked the document storage code to reduce duplication and make the read and write paths easier to follow. Behaviour is unchanged. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Register the documents storage module so its API is reachable from the storage crate root. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Device store operations now delegate to the document port when a storage backend is configured, falling back to the existing SQLite path otherwise. This lets paired-device persistence work under document storage without changing callers. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat the device store and document storage modules and their tests to satisfy rustfmt, wrapping long call chains and assertions. No behavioural change. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a store_documents module to the desktop notifications layer so notification documents can be persisted and retrieved. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Every notification store function now delegates to the document port when a storage backend is configured, falling back to the existing SQLite path otherwise. The document list filter was also rebuilt from optional clauses so provider and score constraints compose correctly. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the store_documents module to the notifications module tree so its contents are compiled and available to the rest of the crate. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat the received_ms document insert in the notification store to multi-line form, matching the surrounding style. No behaviour change. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `set` helper now requires its edit closure to be `Sync` in addition to `Send`, so callers can share the closure across threads when applying notification document edits. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replace the default version with the explicit first version constant in the unparseable raw payload test so the fixture states the version it intends rather than relying on the default. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…hrough store Split the patch logic out of update_source into a shared apply_patch helper so both the SQLite and document stores can reuse it. update_source now delegates to the document store when one is active, falling back to the existing SQLite path otherwise. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The implementation note about the three SQLite connections and the TOCTOU window was attached to the wrong function, so it now sits on update_source where it applies. The note also records that the document store applies the patch under compare-and-swap and therefore has no such window. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a task source that reads tasks from stored documents, letting the integration layer surface document-backed work items alongside existing sources. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Dropped the private TaskSourceStorageError marker and its From<StorageError> impl, which were no longer referenced since storage errors surface through Repo::run. Also trimmed the now-unused StorageError import. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Each store function now checks for a configured storage backend and delegates to the document port when one is present, falling back to the existing SQLite path otherwise. This lets task sources and the ingested ledger be served from the document store without changing callers. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the store_documents module declaration so the new submodule is compiled as part of task_sources. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted the task source document store and its tests to satisfy rustfmt, wrapping long signatures, calls, and assertions. The Notion filter spec in the tests also gained the new assigned_to_me and status fields so it matches the current type. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The provider slug parse error was not convertible into the anyhow error returned by `to_source`, so the `?` operator failed to compile. The error is now explicitly mapped into an anyhow error before propagation. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Moved the notification row-to-struct conversion helpers out of store.rs into a dedicated store_rows submodule, leaving the store to import rows_to_notifications. Also qualified ScopedStorage paths inline in the document stores to drop now-unused imports. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Moved the task source row mapping, timestamp parsing, and SQL conversion helpers out of store.rs into a dedicated store_rows submodule, matching the layout already used by the notifications store. Also applied rustfmt to the notification row helper signature. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed the unused DateTime import from the notification store, leaving only Utc which is still in use. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a dedicated test target for the storage domains end-to-end suite so it runs in its own binary, matching storage_approvals_e2e, since it installs a storage backend into the process-wide slot. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
… and devices Added README sections describing how each module's store functions switch to the tinystoragedrivers document port when a storage backend is configured, covering collections, scoping, and the compare-and-swap semantics that replace the SQLite transactions. The desktop default of using the local database files is noted in each case. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds the scope accessors, the block_on runtime bridge and the documents::Repo and compare_and_swap primitives to the storage README, and lists the domain stores that switch to the document port when a backend is installed. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (24)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe change adds shared document-storage utilities and document-backed implementations for notifications, task sources, and paired devices. Each domain selects the document backend when configured and retains its SQLite path otherwise. Tests cover storage operations, concurrency, and scope isolation. ChangesScoped storage foundation
Notification storage
Task-source storage
Paired-device storage
Cross-domain verification
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Suggested reviewers:
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 57.42% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 155 functions across 19 files. (5 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely identifies the main change: routing device, notification, and task-source storage through storage ports. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
Explanation
Docstring coverage is 57.42% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 155 functions across 19 files. (5 skipped: 5 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the scoped-store trail,
While documents hop where backends prevail.
Devices, tasks, and notices land,
With SQLite waiting close at hand.
The tests nibble through each case,
And leave clean tracks in every place.
Comment @coderabbitai help to get the list of available commands.
senamakel
changed the title
senamakel
marked this pull request as ready for review
October 9, 2026 10:37
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 20 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Changes requested Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Previously reported and still active
Resolved this pass
Pending checks: Rust E2E (mock backend), Build Playwright E2E Artifact, E2E (Playwright / web lane), Desktop E2E (full suite, 3 OS) Before merge
How this fits togetherflowchart LR
n0["handle_ingest<br/>changed"]:::changed
n1["exists_recent<br/>changed<br/>2 findings"]:::blocking
n2["get_settings<br/>changed<br/>2 findings"]:::blocking
n3["insert<br/>changed<br/>2 findings"]:::blocking
n4["list<br/>changed<br/>2 findings"]:::blocking
n5["list_core_notifications<br/>changed<br/>2 findings"]:::blocking
n6["mark_acted<br/>changed<br/>2 findings"]:::blocking
n7["with_connection"]:::impacted
n8["with_connection"]:::impacted
n9["prepare"]:::impacted
n10["format"]:::impacted
n0 -->|calls| n10
n1 -->|calls| n7
n2 -->|calls| n7
n2 -->|calls| n9
n3 -->|calls| n7
n4 -->|calls| n7
n4 -->|calls| n9
n4 -->|calls| n10
n5 -->|calls| n7
n5 -->|calls| n9
n6 -->|calls| n7
n7 -->|calls| n10
n8 -->|calls| n10
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
Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 628e1e0687
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0302 · 1,350,172 in / 62,174 out · 98,558 cached (7%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0225 · 687,884 in / 37,165 out · 47,677 cached (7%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0061 · 496,794 in / 21,358 out · 50,881 cached (10%) · gpt-5.6-luna
tests: $0.0004 · 39,295 in / 111 out · 0 cached (0%) · glm-5.3-flash
description: $0.0004 · 40,129 in / 82 out · 0 cached (0%) · glm-5.3-flash
e2e: $0.0004 · 42,749 in / 207 out · 0 cached (0%) · glm-5.3-flash
tinysweeper
Bot
added
the
priority: p0
11 of 12 tasks
Reformatted a multi-line assert! call in the storage domains end-to-end test into a single line. No behaviour change. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a store_documents module to the desktop notifications subsystem so notification documents can be persisted and retrieved. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…tore Moved the document persistence logic into its own module so the notification store no longer mixes document handling with its other responsibilities. Behaviour is unchanged. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds tests exercising the notification store document handling to lock in current behaviour and guard against regressions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds coverage for the document store notification RPC handlers, exercising the store-backed paths that previously had no tests. This makes regressions in document notification delivery easier to catch. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a store_documents module and wires it into the task sources store so documents can be persisted alongside tasks. This gives task sources a place to keep document data without overloading the existing task storage. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a store for persisting task source documents, along with tests covering its behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted the notification store and its tests, plus a task source test, to satisfy rustfmt line-width rules. No behaviour changed. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed a redundant Ok wrapper and trailing ? in the ingested-documents query, returning the collected result directly. No behaviour change. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 512275f47d
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| /// The document store for this call, when the host configured one. | ||
| pub(super) fn current() -> Result<Option<Docs>> { | ||
| Ok(Repo::current(DOMAIN, collections)?.map(Docs)) |
There was a problem hiding this comment.
When a storage backend is used with derived agents, changing handle_ingest to spawn_scoped does not cover the process-wide NotificationBridgeSubscriber::handle or cron::scheduler::push_cron_alert, both of which enter this helper without the originating agent's task-local CoreContext. Their writes therefore go to local in non-SaaS mode or fail with "no acting agent" in SaaS, while list/mark RPCs read the agent bucket, so core and cron alerts are not durably visible to that agent; these producers need to carry an explicit owner scope.
AGENTS.md reference: AGENTS.md:L401-L408
Useful? React with 👍 / 👎.
|
|
||
| /// The document store for this call, when the host configured one. | ||
| pub(super) fn current() -> Result<Option<Docs>> { | ||
| Ok(Repo::current(DOMAIN, collections)?.map(Docs)) |
There was a problem hiding this comment.
When devices_create_pairing runs under a derived agent, the actual insert happens later in DeviceTunnelSubscriber::handle_tunnel_frame after a process-wide socket/bus event, and PairingSession retains no CoreContext or storage scope. Resolving current() here consequently writes the pairing to local in non-SaaS mode or errors in SaaS, so the cryptographic handshake can complete while devices_list for the requesting agent never sees a persisted device; retain the requester's scope with the pending pairing and use it for the insert.
AGENTS.md reference: AGENTS.md:L401-L408
Useful? React with 👍 / 👎.
|
|
||
| /// The document store for this call, when the host configured one. | ||
| pub(super) fn current() -> Result<Option<Docs>> { | ||
| Ok(Repo::current(DOMAIN, collections)?.map(Docs)) |
There was a problem hiding this comment.
When task sources are created under a derived agent with a backend configured, the process-wide periodic poller and TaskSourcesConnectionSubscriber both call store::list_sources without that agent's scoped context (and the latter's child is also a bare tokio::spawn). This helper therefore enumerates only local outside SaaS or returns the no-acting-agent error in SaaS, so the agent's source records are never considered by either automatic path; schedule these consumers per agent or pass the owning scope explicitly through listing and fetch execution.
AGENTS.md reference: AGENTS.md:L401-L408
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes: 2 lane(s) blocking, worst finding is critical.
Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.
$0.0422 · 1,132,412 in / 86,071 out · 77,495 cached (7%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0342 · 516,113 in / 39,326 out · 43,296 cached (8%) · gpt-5.6-luna, glm-5.3-flash
security: $0.0062 · 436,326 in / 30,796 out · 34,007 cached (8%) · gpt-5.6-luna
tests: $0.0004 · 42,787 in / 4,982 out · 64 cached (0%) · glm-5.3-flash
description: $0.0004 · 43,609 in / 3,941 out · 64 cached (0%) · glm-5.3-flash
e2e: $0.0004 · 46,241 in / 3,372 out · 64 cached (0%) · glm-5.3-flash
| fn sql_conv<E: std::fmt::Display>(err: E) -> rusqlite::Error { | ||
| rusqlite::Error::ToSqlConversionFailure(anyhow::anyhow!("{err}").into()) | ||
| } | ||
| mod store_rows; |
There was a problem hiding this comment.
Add the declared store_rows module
This declaration has no corresponding store_rows.rs module in the repository, so the crate fails to compile with a missing module file. Add the module source at the expected path or remove the declaration and restore the definitions here.
Additional security observation
Add the declared store_rows module
[RULE] missing-module-source
This declaration requires crates/openhuman-core/src/integrations/task_sources/store_rows.rs (or an inline module), but no such source is present in the provided pull-request diff. Unless that file is already present on the target branch, the crate will fail to compile because the module cannot be found. Add the module source containing these items or keep their definitions in this file.
[RULE] missing-module ·
| } | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
Add the declared test module source
The new file declares store_documents_tests.rs under #[cfg(test)], but that source file is not included in the supplied patch. If it does not already exist in the branch, any test build fails with a missing module error. Add the sibling test file or remove the declaration.
[RULE] missing-module-source ·
| let Some(stored) = docs.get(DEDUP, id).await? else { | ||
| return Ok(()); | ||
| }; | ||
| if stored.doc.get("last_ms").and_then(Value::as_i64) != Some(claimed_ms) { |
There was a problem hiding this comment.
Do not release another process's dedup claim
The rollback identifies ownership only by last_ms. Two callers can receive the same millisecond from Utc::now().timestamp_millis(); if the second caller claims the same content with skip_recent = false, the first caller's insert can fail and release_content will see the same timestamp and delete or restore the second caller's claim. A later retry can then be accepted as new even though the second notification was stored. Store a unique claim token/version in the dedup document and require that token when releasing the claim.
[RULE] atomic-claim-ownership ·
| let id = n.id.clone(); | ||
| let doc = to_doc(n); | ||
| let dedup = dedup_id(&n.provider, n.account_id.as_deref(), &n.title, &n.body); | ||
| let now_ms = Utc::now().timestamp_millis(); |
There was a problem hiding this comment.
Record the actual arrival time for deduplication
now_ms is captured before entering the document-store operation. Under executor or storage contention, the claim can be written substantially later, but its timestamp still reflects the earlier call time. This can make a notification appear older than it was and shorten the effective dedup window; capture the arrival/claim time immediately before the claim operation, or use the storage transaction's timestamp.
[RULE] stale-dedup-timestamp ·
|
|
||
| fn collections() -> Vec<CollectionSpec> { | ||
| vec![ | ||
| CollectionSpec::new(SOURCES).index(IndexSpec::new("by_created", ["created_ms"])), |
There was a problem hiding this comment.
Preserve full timestamp ordering when listing sources
created_ms truncates DateTime<Utc> to milliseconds. Two sources created within one millisecond are then ordered by _id, not by their actual creation time, so list_sources can violate insertion/creation order. Persist and sort by a lossless ordering key, or capture a deterministic insertion sequence for ties.
[RULE] lossy-ordering-key ·
| triage_action: text(doc, "triage_action").map(str::to_string), | ||
| triage_reason: text(doc, "triage_reason").map(str::to_string), | ||
| status: status_of(text(doc, "status")), | ||
| received_at: parse_time(text(doc, "received_at")).unwrap_or_else(Utc::now), |
There was a problem hiding this comment.
Do not replace invalid receipt times with the current time
A missing or malformed persisted received_at is replaced with the current time during reads. That changes historical notification metadata and can make old corrupt records appear newest, destabilizing listing and downstream processing. Propagate the parse error or explicitly omit/reject the malformed record.
Additional critique observation
Do not treat malformed notification timestamps as valid state
[RULE] malformed-data-propagation
A corrupt received_at is silently replaced with the current time, changing the notification's ordering and making the corruption invisible to callers. A corrupt scored_at is silently converted to None, so stats and downstream logic treat a malformed scored notification as unscored. Return a storage/data error for malformed required timestamps, or explicitly preserve and surface the invalid-record state instead of normalizing it.
Additional e2e observation
Do not replace invalid receipt times with the current time
[RULE] silent-parse-fallback
Still stands: an unparseable received_at becomes the current time, silently reordering and re-deduplicating against real arrivals. Surface the corruption rather than fabricating a timestamp.
[RULE] strict-timestamp-decoding ·
|
|
||
| pub(super) fn list_sources(&self) -> Result<Vec<TaskSource>> { | ||
| let stored = self.0.run(|docs| async move { | ||
| let query = Query::all() |
There was a problem hiding this comment.
Preserve full timestamp ordering when listing sources
Sources are persisted with only epoch milliseconds, so sources created within the same millisecond are ordered by _id rather than their actual DateTime<Utc> values. This can change the ordering promised by the SQLite implementation. Persist and sort by a precision-preserving timestamp key, or otherwise retain a deterministic full-precision ordering value.
[RULE] lossy-timestamp-ordering ·
| } | ||
|
|
||
| pub(super) fn list_ingested_refs(&self, source_id: &str) -> Result<Vec<IngestedTaskRef>> { | ||
| let query = Query::filter(Filter::eq("source_id", source_id)) |
There was a problem hiding this comment.
Preserve full timestamp ordering for ingested references
The reference listing sorts only the millisecond timestamp and then the external ID. Multiple ingestions can share that millisecond while having different actual arrival times, so the document backend can return a different order from SQLite. Store and sort on a precision-preserving timestamp.
[RULE] lossy-timestamp-ordering ·
| source_id: &str, | ||
| limit: usize, | ||
| ) -> Result<Vec<NormalizedTask>> { | ||
| let query = |
There was a problem hiding this comment.
Preserve full ingestion ordering when listing tasks
Recently ingested tasks are ordered only by epoch milliseconds. Tasks received in the same millisecond are therefore ordered by the backend's unspecified tie behavior, which can violate the newest-first contract. Use a precision-preserving ingestion timestamp and an explicit stable tie-breaker.
[RULE] lossy-timestamp-ordering ·
| triage_reason: text(doc, "triage_reason").map(str::to_string), | ||
| status: status_of(text(doc, "status")), | ||
| received_at: parse_time(text(doc, "received_at")).unwrap_or_else(Utc::now), | ||
| scored_at: parse_time(text(doc, "scored_at")), |
There was a problem hiding this comment.
Do not treat malformed scoring timestamps as unscored
When scored_at is present but malformed, parse_time returns None, making the notification look unscored even though it has persisted scoring fields. This creates incorrect triage state and statistics. Distinguish a missing field from an invalid timestamp and propagate or reject the invalid record.
Additional e2e observation
Do not treat malformed scoring times as unscored
[RULE] silent-parse-fallback
Still stands from the earlier revision and unchanged: a scored_at value that fails to parse makes the notification read as unscored, so the intelligence pipeline re-scores content it already scored. Return an error or preserve the raw string instead of silently dropping the field.
[RULE] strict-timestamp-decoding ·
12 tasks done
senamakel
added a commit
that referenced
this pull request
Oct 9, 2026
chore(ci): drop the saas-ambient baseline entry #7181 fixed
This was referenced Oct 9, 2026
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
OPENHUMAN_STORAGE_URL/[storage] url, feat(storage): storage backend by URL; session stores on tinystoragedrivers; bump tinyagents + tinyflows #7168), every function insecurity::devices::store,desktop::notifications::storeandintegrations::task_sources::storeis served from the backend's document store instead ofdevices.db,notifications.dbandsources.db. This follows the approvals pattern from feat(approval): approvals on the storage ports #7178.localon a single-user host, refused in SaaS mode with no acting agent.BEGIN IMMEDIATE; it is now a compare-and-swap on a per-content dedup document. Device touch/revoke and task-source updates are compare-and-swap too, which also closes the read-modify-write windowupdate_source's own comment called out.storage::documents.Repo(one domain's scoped handle: declare its collections, run a call from sync code) andcompare_and_swap(the guarded-UPDATEloop). The next domains build on these instead of copying the plumbing.Problem
rusqlitefiles. A multi-tenant deployment would write them to one shared local file, outside the configured backend and with no per-agent isolation.Solution
store_documents.rsbeside itsstore.rs. Every public store function first asksstore_documents::current()?and returns its answer when a backend is installed; otherwise it falls through to the existing SQLite code. Signatures are unchanged.paired_devicesdocument perchannel_id. Pairing replaces the document (the SQLINSERT OR REPLACE). Revoking an already revoked device still reports that it exists, as the SQLUPDATEdoes.integration_notifications,notification_dedup,notification_settingsandcore_notifications.null, so "unscored" is a missing field on every driver.received_msorders the list; RFC 3339 strings with differing fractional digits don't sort as instants.statsfolds its counts in the store, since the port has noGROUP BY. It is bounded by one user's notifications.Precondition::Absent).task_sourcesandingested_tasks(id(source_id, external_id), length-prefixed so ids can't collide).filter,targetand the task payload stay JSON strings.apply_patchis extracted, so both stores apply a patch the same way.store/store_rows.rsin notifications and task sources, keeping bothstore.rsfiles under the 750-line layout limit.config/workspace/state.rs). It records local file timestamps on this machine, so it belongs in local SQLite, not a shared backend.Submission Checklist
storage/documents_tests.rs: run, error prefix, CAS apply/decline/missing, eight concurrent swaps with one winner, scope isolation.devices/store_documents_tests.rs: round trip, re-pair clears revocation, touch only live devices, revoke semantics, ordering, scopes.notifications/store_documents_tests.rs:task_sources/store_documents_tests.rs:record_fetch;clear_all;tests/storage_domains_e2e.rs: the public functions of all three with a memory backend installed, then back on SQLite afterstorage::clear(). It is its own test binary, likestorage_approvals_e2e.memorydriver.Impact
Related
SecretStore;RuntimeBuilder::storage(url).AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
storage-domainsValidation Run
pnpm --filter openhuman-app format:check: N/A, no frontend changespnpm typecheck: N/A, no frontend changesRUST_MIN_STACK=16777216 cargo test -p openhuman --lib -- integrations::task_sources desktop::notifications security::devices storage:: security::approval(395 passed);cargo test -p openhuman-cli --test storage_domains_e2ecargo fmt,cargo clippy -p openhuman --lib --tests -- -D warnings,pnpm rust:layoutValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
INSERT OR REPLACE, update-matched-a-row booleans, the 60 s dedup window,limit.max(1), cascade on delete, corrupt rows skipped where SQL skipped them), and the tests assert them.Duplicate / Superseded PR Handling
Summary by CodeRabbit