flush_pending can be deduplicated away by an in-flight flush, leaving fresh buffers for the seven-day force flush

[YellowSnnowmann]

Correction (2026-09-03). The "Suggested fixes" section below says option 2 is "a contract addition, so it needs a release plus a host re-pin". That is wrong. FlushSourceTree has been a MemoryTree contract member for some time (crates/tinymemory-bus/src/names.rs FLUSH_SOURCE_TREE, in METHODS; MemoryTree::flush_source_tree(&self, source_scope: &str) -> Result<u64, MemoryError>), the pinned module serves it, and openhuman already calls it from flush_source_tree_rpc. So the host can close this without any engine change: tinyhumansai/openhuman#6015 replaces the post-sync flush_pending() with flush_source_tree(&format!("{toolkit}:{connection_id}")), which bypasses the dedupe queue (no race) and seals one scope instead of the whole workspace. Option 1 (record a rerun on a suppressed enqueue) remains a valid engine-side hardening for other flush_pending callers, but is no longer needed for the sync path. The original text is kept below unedited for the record.

Raised from a CodeRabbit finding on tinyhumansai/openhuman#6011, which added a post-sync flush_pending() call. The finding is valid, and the durable fix is engine-side — a host cannot close it.

The race

MemoryMaintenance::flush_pending enqueues a flush_stale job keyed date + hour/3:

let date_iso = now.format("%Y-%m-%d").to_string();
let hour_block = chrono::Timelike::hour(&now) / 3;
let job = NewJob::flush_stale(&payload, &date_iso, hour_block)?;
let enqueued = queue::store::enqueue(config, &job)?.is_some();

enqueue is INSERT OR IGNORE against the partial unique index

CREATE UNIQUE INDEX idx_mem_tree_jobs_dedupe_active
    ON mem_tree_jobs(dedupe_key)
    WHERE dedupe_key IS NOT NULL AND status IN ('ready', 'running');

so a request is suppressed while a flush for that window is ready or running. That is correct for "spamming the button is safe", but it conflates two different situations:

• suppressed against a ready job — harmless, that job has not scanned yet and will pick the new buffers up;
• suppressed against a running job — the scan may already have walked past the buffer the caller just wrote, and nothing remains queued for it.

Why it bites now

Before openhuman#6011 nothing on the sync path asked for a seal at all, so this was invisible. With a post-sync flush it becomes the residual gap, and the fallback is poor: the periodic scheduler does not rescue those records. It enqueues FlushStalePayload::default(), whose max_age_secs is None, and the handler resolves that with

let age_secs = payload.max_age_secs.unwrap_or(L0_DEFAULT_FLUSH_AGE_SECS);

L0_DEFAULT_FLUSH_AGE_SECS is 604800 — seven days. So the three-hourly tick only sweeps genuinely stale buffers and steps over a buffer written ten minutes ago. The affected records wait for the next force flush, which in practice means the next connector sync, and absent one, seven days.

Why the host cannot fix it

Both halves live in the engine. flush_pending takes no arguments, so a caller has no way to express "rerun after the in-flight scan", and the dedupe key is built inside the driver. The obvious host-side placements were considered on openhuman#6011 and each has a defect:

• Flush once after the multi-page loop instead of per page. Avoids self-suppression by an earlier page's job, but the loop has three exits and two of them (next_pass_budget exhausted, MAX_PASSES reached) leave with more_pending: true. Gating on batch completeness therefore skips the seal for precisely the runs that wrote the most records.
• Poll until the in-flight job settles, then re-enqueue. Puts a queue-state wait on a user-facing sync path for a best-effort secondary index.

Suggested fixes

Either would close it; the first is smaller.

1. Record a rerun. When enqueue is suppressed against a running row, set a flag on that job (or insert a successor with a distinct key) so the worker re-scans once on completion. This is CodeRabbit's phrasing — "make the maintenance queue record a rerun" — and it keeps the button-spam property, since the rerun collapses to one regardless of how many requests arrived.
2. Put force_flush_tree on the contract. The engine already has a per-tree force seal (tree::tree::flush::force_flush_tree), which is what a connector sync actually wants — it knows its own path_scope and has no business sealing every unrelated tree. Exposing it as a Maintenance member would also fix the second-order complaint on openhuman#6011: flush_pending is workspace-wide, so a Gmail sync currently also seals whatever a folder or github_repo source left pending.

Option 2 is the better shape but is a contract addition, so it needs a release plus a host re-pin before any caller can use it.

Not urgent

Strictly better than the pre-openhuman#6011 behaviour, where every small connector sync waited seven days. The race needs a flush to be mid-scan at the moment a pass commits, and the next sync clears it. Filing so it is not rediscovered as a bug report.

[tinysweeper]

flush_pending can be deduplicated away by an in-flight flush, leaving fresh buffers waiting up to seven days for the next force flush. The engine-side fix is clearly described and non-trivial; workaround exists (next connector sync clears it) but the race is real.

Labelled priority: p2.

[YellowSnnowmann]

Correcting the severity in my own report, downward.

The "Not urgent" section says the affected records wait for "the next connector sync, and absent one, seven days". The second half is true but misleading as written, because a next sync is the default rather than the exception: DEFAULT_MEMORY_SYNC_INTERVAL_SECS is 86_400 (crates/tinymemory-api/src/host/mod.rs:91), so the periodic Composio loop re-syncs each connection every 24 hours, and every sync pass asks for a flush once openhuman#6011 is in.

So the realistic exposure is:

• manual sync — cleared immediately on the next click;
• default periodic sync — cleared within ~24h;
• seven days — only where periodic sync is off (memory_sync_interval_secs = 0, the "Manual only" setting) and the user never syncs by hand.

That makes this a latency wart rather than a data-visibility bug, and argues for fixing it as option 2 (force_flush_tree on the contract) whenever the next contract-touching release happens, rather than shipping option 1 as a standalone patch that option 2 would then delete. Option 2 is also the one that fixes the second-order complaint from openhuman#6011 review — flush_pending being workspace-wide, so a Gmail sync currently also seals whatever a folder or github_repo source left pending.

No change to the analysis of the race itself, only to how much it costs when it hits.

[YellowSnnowmann]

Correcting a factual error in the report, and it changes the conclusion.

The "Suggested fixes" section says option 2 — a per-tree force seal — is "a contract addition, so it needs a release plus a host re-pin before any caller can use it". That is wrong. FlushSourceTree is already a contract member and has been for some time:

• crates/tinymemory-bus/src/names.rs: pub const FLUSH_SOURCE_TREE: &str = "FlushSourceTree";, and it is in METHODS
• crates/tinymemory-api/src/provider/content.rs: MemoryTree::flush_source_tree(&self, source_scope: &str) -> Result<u64, MemoryError>
• served by the module, and OpenHuman already calls it through flush_source_tree_rpc

So the good fix needs no release, no re-pin, and no change in this repository at all.

What that makes of the race

It removes it rather than mitigating it. flush_source_tree bypasses the job queue — it opens (or creates) the scope's tree and seals it directly. No queue means no dedupe_key, and no dedupe key means there is nothing for an in-flight flush to suppress. The whole failure mode described above only exists because the caller went through flush_pending, which enqueues.

It also settles the second-order complaint recorded here: flush_pending is workspace-wide, so a Gmail sync currently seals whatever a folder or github_repo source left pending. A per-scope seal touches one tree.

And it is safe to call unconditionally — the contract already promises that a scope with nothing buffered is Ok(0) rather than an error, and that an unknown scope is Ok(0) too because the driver creates the tree, so it cannot be used to probe which scopes exist.

Consequences for this issue

• Option 1 ("record a rerun" in the maintenance queue) is no longer worth building. It would add machinery to a queue that the fixed call path does not touch.
• The remaining work is host-side, in run_sync_pass: call flush_source_tree("{toolkit}:{connection_id}") instead of flush_pending(). That is the same scope the connector items are treed under, so it is the tree that actually needs sealing.
• This issue is therefore mis-filed here. Tracking and fixing it in feat(memory): backfill connector memories into the memory tree openhuman#6015; closing this one in favour of that.

The analysis of the race itself stands, as does the earlier correction that the periodic tick does not rescue fresh buffers (it enqueues the default seven-day age). What was wrong was the cost estimate of the fix — and it was wrong in the direction that mattered, because it argued for deferring work that turns out to be a ten-line change already available on the pinned contract.

[tinysweeper]

Race condition where flush_pending is deduplicated away by an in-flight flush, leaving fresh buffers waiting. However, comments establish the fix is host-side (openhuman#6015) using the already-available flush_source_tree contract member which bypasses the queue entirely, so no engine change is needed in this repo. Severity downgraded to a latency wart with ~24h default clearance via periodic sync.

Labelled priority: p3.