Skip to content

Complete Bun writes and bound fresh-sync memory - #1361

Open
mariusvniekerk wants to merge 83 commits into
t3code/unify-databases-with-bun-duckdbfrom
t3code/bun-sync-writer-unification
Open

Complete Bun writes and bound fresh-sync memory#1361
mariusvniekerk wants to merge 83 commits into
t3code/unify-databases-with-bun-duckdbfrom
t3code/bun-sync-writer-unification

Conversation

@mariusvniekerk

@mariusvniekerk mariusvniekerk commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Archive sync and repairs now keep Bun execution and transaction guards through full-session writes, incremental updates, attached scratch publication, usage caches, and PostgreSQL metadata operations. A staged import commits its session, tool results, usage, signals, and parser checkpoint together. SQLite-only result identity metadata survives replacements without entering portable mirror schemas.

Usage queries stream ordered rows into Go aggregation instead of retaining the complete input. Token interpretation, pricing, snapshot selection, and billing remain in Go. Shared queries retain date overlap and Unicode search behavior across backends. SQLite string formatting preserves internal raw-byte identities and copies text between quotes without rune-by-rune conversion.

Fresh-sync writes use a 1 MiB estimated payload per statement while preserving transaction boundaries and whole oversized rows. This limits transient formatted SQL buffers. The local corpus comparison and remaining allocation tradeoff are recorded in docs/internal/bun-usage-performance.md; external BenchDB measurements remain the acceptance check for its distinct machine and corpus.

The combined stack preserves upstream reasoning effort, archive-content policies, image retention, installation identity adoption, Chinese full-text search, activity categories, and Codex streaming and checkpoint behavior and advances archive data version to 108. Review the shared session-batch transaction, scratch publication, canonical result metadata, and SQLite literal formatter. Existing archives retain the rebuild and recovery procedure in docs/internal/storage-upgrade.md.

DuckDB pushes share a per-mirror process lock through probing, checkpointing, and replacement, preventing a concurrent push from leaving an old WAL for the rebuilt mirror. Session upserts reuse cached archive-column metadata throughout the write path.

@mariusvniekerk mariusvniekerk changed the title t3code/bun sync writer unification Consolidate archive sync writers on Bun transactions Aug 7, 2026
@roborev-ci

roborev-ci Bot commented Aug 7, 2026

Copy link
Copy Markdown

roborev: Combined Review (005ad77)

Changes requested: one medium-severity reliability issue could leave an incorrect parent unrepaired.

Medium

  • internal/sync/engine.go:14328 — A successfully written result is no longer queued for its own parent repair. If a later member fails before the final link, or linking itself fails, that session can retain an incorrect parent with no durable retry. After each successful atomic write, queue resultIDs[i], set repairQueued, and then queue its children.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 8m59s

@mariusvniekerk
mariusvniekerk force-pushed the t3code/bun-sync-writer-unification branch from 005ad77 to a37d0df Compare August 9, 2026 03:46
@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (a37d0df)

The transaction consolidation is generally sound, but one medium-severity partial-failure issue remains.

Medium

  • internal/sync/engine.go:14346 — After a member write succeeds, only its spawned children are queued. If a later member fails, the final scoped link at internal/sync/engine.go:14350 is skipped, so the successful member may retain an incorrect parser-derived parent despite an existing spawn edge pointing to it.
    • Fix: Durably queue each successfully written resultIDs[i] before continuing, or link each successful member immediately. Add a regression test where an earlier child write succeeds before a later member fails.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 9m27s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (935b4e0)

The transactional improvements are sound overall, but one medium-severity parent-repair regression remains.

Medium

  • internal/sync/engine.go:14346 — Successfully committed result sessions are no longer queued for parent repair. If a later member or the final scoped link fails, the function returns before linking those sessions. Because queueWrittenChildren queues only outgoing children, a committed session that is itself a child may retain an incorrect parser-derived parent indefinitely.

    Fix: After each successful write, durably queue the result session ID for parent repair and set repairQueued, alongside queuing its children.


Reviewers: 2 done | Synthesis: codex, 23s | Total: 10m2s

@mariusvniekerk
mariusvniekerk force-pushed the t3code/bun-sync-writer-unification branch from 935b4e0 to 178a8ff Compare August 9, 2026 04:08
@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (178a8ff)

Medium-severity issues remain in canonical event indexing and multi-session parent repair.

Medium

  • Secret findings use incorrect event coordinatesinternal/db/messages.go:566, internal/db/session_batch.go:451
    Canonical persistence preserves unique, non-positional tool-result EventIndex values, but secret scanning records slice positions. For events indexed 5 and 9, findings record 0 or 1, so SecretFindingSource cannot locate the persisted event and secret reveal fails.
    Fix: Derive finding coordinates using the same unique-index/positional-fallback rule as CanonicalMessageRows, and add a persisted scan/reveal regression for non-positional event indices.

  • Earlier committed sessions can miss authoritative parent repairinternal/sync/engine.go:14332
    During a multi-session, single-source sync, successfully committed sessions are not queued for their own parent repair; only their outgoing children are queued. If a later member fails, the early return skips LinkSubagentSessionsForSessions, potentially leaving an earlier committed child with parser-derived rather than authoritative edge-derived parentage.
    Fix: Queue each session ID after its atomic write commits, or defer linking all successfully committed IDs. Add coverage for an earlier successful child followed by a failing member.


Reviewers: 2 done | Synthesis: codex, 15s | Total: 9m48s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (e370642)

Medium-severity issue found: secret findings can reference the wrong result event.

Medium

  • internal/db/bun_write.go:295scanSecretsFromMessages records each result event’s slice position, while canonical writes preserve unique explicit EventIndex values. For example, an event with EventIndex: 7 can produce a finding for index 0, preventing SecretFindingSource from locating or revealing the matched content.
    • Fix: Reuse the canonical event-index normalization logic in the secret scanner: preserve unique explicit indices and fall back to slice positions only when indices collide. Add an end-to-end scan/reveal test covering a non-positional index.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 11m25s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (466bcaf)

Review identified one medium-severity durability issue in deferred spawn-link repair.

Medium

  • internal/sync/engine.go:14346 — Successful session writes queue only newly spawned children. If the subsequent scoped linking pass fails, a rewritten session whose incoming spawn edge must restore its parent remains committed with parser-derived parentage and no durable repair entry; unrelated future syncs will not repair it.
    • Fix: After each successful atomic write, also queue the written result ID and enable the deferred repair pass before scoped linking.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 18m31s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (e09df48)

The storage refactor is generally sound, but one medium-severity recall-provenance issue remains.

Medium

  • Recall evidence is not reconciled during append-only writesinternal/db/session_batch.go:464, internal/db/messages.go:644

    Newly appended messages or tool calls can duplicate an evidence endpoint, making it ambiguous while the corresponding recall entry incorrectly remains ProvenanceOK.

    Suggested fix: Reconcile recall evidence within the same transaction whenever messages are appended, including incremental writes. Add a regression test covering an appended duplicate endpoint.


Reviewers: 2 done | Synthesis: codex, 10s | Total: 19m19s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (96a0984)

Medium-severity issue found in secret-finding index consistency.

Medium

  • internal/db/messages.go:599 — The canonical archive path preserves unique non-positional result-event indexes, while secret scanning records slice positions. Because SecretFindingSource queries the persisted index exactly, findings for events indexed like 5 or 9 cannot be revealed or relocated.
    • Fix: Share canonical event-index normalization between persistence and scanning, and add a write/scan/reveal test covering distinct non-positional indexes.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 16m45s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (83d74c2)

Changes need revision due to one medium-severity recall-evidence consistency issue.

Medium

  • internal/db/messages.go:670 — Recall evidence is reconciled only when new messages are appended. An incremental write may instead change a tool call’s result content or subagent ID through SubagentLinks; because these fields contribute to the recall-evidence digest, link-only writes can leave stale evidence marked as trusted. Reconcile whenever transcriptChanged is true, and add a regression test for a link-only update to a tool call included in trusted evidence.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 12m21s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (7b4ce00)

Medium-severity risk found in bulk persistence; no security issues identified.

Medium

  • internal/db/bun_write.go:409 — Canonical message, tool, usage, finding, and repair-ordinal writes pass unbounded slices to single Bun inserts. Large transcripts can produce enormous SQL statements and memory allocations, potentially causing full sync or resync failures. Use bounded row- or byte-sized inserts within the existing transaction.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 12m37s

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (43af3ab)

One medium-severity issue found in snapshot timestamp ordering.

Medium

  • internal/db/bun_usage.go:1403dailyUsageProjectionSnapshotTimestamp omits the SessionCreatedAt fallback used by dailyUsageProjectionTime. Claude rows lacking usage or session-start timestamps can lose chronological ordering during snapshot selection, potentially selecting or attributing usage to the wrong session based on lexical tie-breakers. Fall back to SessionCreatedAt (or format dailyUsageProjectionTime(row)) and add coverage for cross-session snapshots where only creation timestamps establish order.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 11m14s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (00f413d)

The change is generally sound, but one medium-severity timestamp-ranking inconsistency can produce incorrect recent-edit results.

Medium

  • internal/db/bun_recent_edits.go:53 — SQLite ranks and paginates edits using julianday, which accepts legacy values such as date-only timestamps that bunmodel.ParseTimestamp later rejects. An invalid timestamp can therefore displace a valid recent edit or select the wrong session, while the returned timestamp is blank.
    • Suggested fix: Apply the same strict timestamp-validity rules in the SQL ranking expression, and add a multi-row ordering/pagination test containing both canonical and unsupported timestamps.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 14m16s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (7dc816d)

Medium-severity batching issue found; no security vulnerabilities were identified.

Medium

  • internal/db/bun_identity_write.go:328 — Worktree-mapping tombstones are rendered into a single unbounded tuple IN clause. A large publication delta could produce an oversized query and repeatedly block PostgreSQL or DuckDB synchronization. Execute deletions in bounded chunks using canonicalIdentityWriteBatchSize or the canonical byte-budget batching helper.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 11m44s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (c58a4b6)

Medium-severity performance issue found in the streaming usage path; no security vulnerabilities identified.

Medium

  • internal/db/bun_usage.go:852 — The streaming usage path counts and preallocates for every raw candidate before applying filters and deduplicating Claude rows. Large archives or narrowly filtered queries may allocate excessive memory and execute two unnecessary full count queries, undermining the streaming design.
    • Suggested fix: Remove count-based preallocation or cap it conservatively. Grow results only for accepted rows and buffer only candidates needed for deduplication.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 19m22s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (11a0f2a)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 35m45s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (a305777)

Medium-severity issue found in tombstone batching; no Critical or High findings.

Medium

  • internal/db/bun_identity_write.go:328 — Removing the previous 100-key chunking causes every tombstone to be formatted into one unbounded bun.Tuple. Large publication windows may produce excessive SQL and memory use, causing PostgreSQL or DuckDB pushes to fail.
    • Fix: Retain bounded batching, preferably using the new payload-budget helper, before constructing each tuple list.

Reviewers: 2 done | Synthesis: codex, 9s | Total: 27m32s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (15f20bb)

Medium-severity issue found in SQLite usage timestamp handling.

Medium

  • internal/db/bun_write.go:428, internal/db/bun_usage.go:707 — Archive writes preserve unsupported message timestamps, but SQLite usage queries still validate them with julianday. SQLite accepts values such as 2026-08-09 or 2451545, which bunmodel.Timestamp.Scan then rejects, causing the entire usage report to fail instead of falling back to the session timestamp.
    • Fix: Apply the canonical timestamp-availability expression to message usage selection, bounds, and ordering so unsupported values become NULL before typed scanning. Add coverage for a usage-bearing message with a SQLite-valid but noncanonical timestamp.

Reviewers: 2 done | Synthesis: codex, 18s | Total: 14m32s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (f1b0bc9)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 24m40s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (cc7b5be)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 26m55s

@roborev-ci

roborev-ci Bot commented Aug 10, 2026

Copy link
Copy Markdown

roborev: Combined Review (1c707c4)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 22m14s

@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (82615ca)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 13m15s

@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (e02f880)

No Medium, High, or Critical findings; the reviewers found the change clean at the requested severity threshold.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 20m23s

@mariusvniekerk
mariusvniekerk force-pushed the t3code/bun-sync-writer-unification branch from e02f880 to 0ee0579 Compare August 13, 2026 13:26
@roborev-ci

roborev-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown

roborev: Combined Review (0ee0579)

Changes requested: one medium-severity data-version gating issue can leave malformed timestamps unrepaired.

Medium

  • internal/db/orphaned.go:2104 — Canonical timestamp repair is gated at data version 84 even though version 86 already exists. Archives stamped 84–86 may skip both full resync and orphaned/trashed-row repair, leaving malformed timestamps that strict read paths reject. Bump dataVersion and canonicalTimestampDataVersion to 87 so all previously valid archives are rebuilt and copied rows are canonicalized.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 18m48s

Daily usage retained every streamed Claude snapshot before selecting winners.
Keep only each request group's winning projection in reusable arena blocks,
while preserving earliest attribution, billed searches and winner ordering.

Pricing catalog scans repeatedly normalized the same model and rule text.
Normalize immutable substring patterns at load time and model text once per
lookup, preserving Unicode equality, regex case sensitivity and price selection.

Keep reproducible backend, repeated-snapshot and concurrent-heap measurements
with the change so the remaining allocation gap can guide further tuning.
Metadata recovery could acquire a reopened writer while compaction still held
its maintenance barrier, allowing user-managed names to be changed before the
candidate archive was committed. Use the guarded connection acquisition path
so these writes remain rejected until the barrier is released.

The vector-table startup probe also retained a driver-native placeholder after
moving to Bun. Format the generation argument through Bun so the probe can
report both missing and existing chunk tables.

Clarify the distinction between schema migration and parser resync,
require PostgreSQL integration verification, and document archive backup
and restoration that preserve WAL contents alongside the stack's separate acceptance boundaries.
The bulk watch fixture must use Bun transactions and stamp archive identity
just like normal session writes. Preserve the fixture equivalence check
introduced on main so the large-archive tests exercise representative rows.
Daily usage retained selection state for every Claude request and resolved
computed prices repeatedly. Process one identity group at a time, retain only
survivors in the arena, and reuse the charge lookup for pricing provenance.
Preserve source order when attribution makes winners share an ordering key.

The year-range tools report crossed the performance gate because SQLite
repeatedly called Go for UTC conversion. Keep UTC work in SQL and retain one
timezone per connection for other reports. Preserve timestamp precision and
last-used ordering.

Record the current-main comparison and recommend keeping the consolidation
with an explicit transient-memory tradeoff for concurrent live usage reads.
A Windows rebuild check observed a one-microsecond discrepancy. Check the
archive revision before copying and the reopened mirror version before its
pricing value, so the failure identifies the stage that lost precision or
returned an old file. Keep the exact microsecond assertion intact.
The latency gate does not establish memory parity. Record allocation profiles
and query plans showing where the shared usage reader retains full rows,
grows backing arrays, and incurs database sorting costs. Keep the measured
cardinality and concurrency limits explicit so future changes cannot use a
green benchmark gate as evidence of bounded process memory.
Retaining every selected usage row made concurrent reports hold large arrays
and pool them after completion. Stream ordered candidates into the reducers
so completed Claude groups no longer consume retained row or identity storage.
Keep token interpretation and pricing in Go, and preserve chronological
selection across usage events and Cursor charges through a shared query.
Fresh archive sync spends substantial time validating already-clean text.
Check UTF-8 and control characters in one pass before the existing repair
path, preserving normalization across archive writes and mirror fingerprints.

Extend the full-rebuild benchmark with configurable long replies so sync
measurements can exercise text volume as well as message count.
Rounded SQL timestamps could admit a session just outside an activity
cutoff and let it suppress an eligible duplicate. Apply the exact time
filter before deduplication while keeping unfiltered reads on the fast path.
Completed parser results dominated live heap during archive rebuilds.
Flush smaller pending batches while preserving active worker admission,
so parsed content can be released sooner without adding more pools.

Validated session batches also repeated full-body sanitization during
canonical conversion. Reuse their cleaned content while keeping direct
writes and mirror conversion on the sanitizing path.
SQLite rounds microsecond timestamps during SQL comparisons, dropping
eligible active and stale sessions before the exact usage filter runs.
Keep boundary candidates in SQL and apply precise filtering before
usage deduplication and matching-session counts.
Resolving tool-call parents selected only IDs and ordinals but allocated
full message structs for every row. Scan into the two-field projection
while retaining the canonical model query and parent mapping.

Paired SQLite measurements reduce allocation bytes by 52-74% in this
lookup. Full-sync impact still requires the external benchmark.
Full sync can run in a daemon worker while CLI profiling records only
the waiting client. Let isolated profiling runs capture CPU and memory
in the worker through inherited environment settings, with runtime
tracing enabled separately to control output size.

Keep profiling opt-in and best-effort so invalid output settings do
not interrupt archive work. Give each worker a unique private output
directory without including source identities in filenames.
Keep the current streaming importer and checkpoint guarantees when archive
writes move through Bun. Staged content, usage, signals, and completion
must commit together, and local result identity metadata must survive
replacement without becoming part of mirror schemas.

Large formatted insert statements increase transient memory during fresh
sync. Use smaller statement payloads within the same transactions and
copy SQLite literal spans directly. Local full-corpus comparisons bring
peak anonymous memory close to baseline while cumulative allocation
remains higher. Preserve raw internal key bytes and canonical timestamps.

Repair date-search integration and isolate CLI fixtures from inherited
agent homes so local validation measures fixture behavior.
The profiling subprocess fixture replaced its environment with Unix-only
home and temporary-directory variables. Windows could not load config
without USERPROFILE, so every profiling case stopped before running sync.
Supply the platform-specific variables using the same isolated fixtures.

Record the final small-statement allocation measurement separately from
the earlier formatter-only experiment.
The year-range tool report exceeded the benchmark gate after moving to
shared Bun aggregates. Computing week boundaries for every tool call
repeated work already owned by the Go response builder.

Aggregate local dates in SQL and extract canonical UTC dates directly
on SQLite. Keep timezone conversion and weekly response semantics while
avoiding repeated timestamp normalization in the scan.
The Windows sync package exhausted its 20-minute process budget after
1,482 top-level tests passed. Run complementary name filters in separate
jobs so every test still runs once without extending the timeout.
A retained destination WAL can replay old schema metadata and pricing
rows onto a freshly rebuilt mirror. Reopening then returns the old data
even though the replacement file passed validation.

Checkpoint that WAL into its original mirror before the file swap, and
keep the old mirror if checkpointing cannot finish. Exercise replacement
with an intentionally retained WAL so the failure reproduces locally.
Forced garbage collection and heap serialization are profiling shutdown
work, but they ran while the sync CPU profile and trace were still active.
Stop streaming profiles before collecting the final heap snapshot so
short sync captures describe the measured operation.
Full archive rebuilds spend substantial CPU time checking transcript text.
Skip printable ASCII in eight-byte blocks while retaining the existing
Unicode and control repair behavior. Corpus comparisons preserve stored
message and tool-result content while reducing local rebuild time.

Reuse registry-derived archive session columns and conflict SQL to avoid
rebuilding the same metadata for each upsert.
Upstream added archive content policies, image retention, and reasoning
effort after the shared Bun implementation diverged. Carry those behaviors
through the canonical projections and transactions so rebasing does not
silently restore discarded text or lose message metadata.

Keep restricted staged imports on projected rows without checkpoints,
preserve automation and PostgreSQL policy markers, and repair copied
version-106 timestamps before strict canonical reads.
A rebuild leaves the destination available while constructing its replacement.
Another push could write the old mirror during that interval and leave a WAL
for the replacement to replay. Hold one process lock through the entire push,
including the final checkpoint and swap.

Reuse the archive column cache when resetting or preserving session state so
each ingestion upsert avoids rebuilding and sorting the same metadata.
The current main branch adds image migration and qualified model pricing
coverage. Keep those checks executable after the storage cutover by using
Bun transaction callbacks and exercising DuckDB pricing through the shared
usage API instead of its removed SQL builder.
Long sessions made each hourly export group, sort, and parse the same
activity events again. Reuse the daily pairs so digest generation avoids
most of those allocations while preserving cross-hour intervals and
model inheritance from earlier messages.
Installation adoption must move canonical worktree rules and project
observations with session ownership. Keep archive boundaries when resolving
collisions and retain the newest observation at canonical timestamp precision.

Route machine metadata, raw parse jobs, and Chinese search through Bun, and
carry the sync engine image policy and activity subagent categories through
shared writes and reports. Repair archives from the new upstream data version
without discarding orphaned history.
@mariusvniekerk
mariusvniekerk force-pushed the t3code/bun-sync-writer-unification branch from ea08459 to a424fbf Compare September 11, 2026 17:32
@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (a424fbf)

Verdict: Changes require fixes for 1 finding.

Medium

  • internal/duckdb/rebuild.go:507: The reader-held fallback rebuild is documented to avoid opening the destination, but checkpointReplacedMirror calls Open(path) read-write whenever .wal exists. A serving read-only handle then prevents this open, so an explicit push fails instead of atomically replacing the mirror. A leftover WAL is an intended supported state, as shown by the existing pricing-precision test. Perform WAL recovery before readers can attach, or recover/checkpoint a separate copy without opening the reader-held destination. Add a regression test combining an existing WAL with a held read-only mirror.

    Reported by: codex


Reviewers: 2 done | Synthesis: codex, 8s | Total: 43m49s

Quoting queries in BunStore made ordinary Chinese searches look like explicit
phrases before SQLite could segment them. Service and MCP searches therefore
missed messages whose matching words were separated by other text.

Leave lexical preparation to the database adapter while retaining shared
whitespace trimming and pagination. Explicit phrase queries keep their meaning.
@roborev-ci

roborev-ci Bot commented Sep 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (67aacbf)

Verdict: Changes require fixes for 2 findings.

Medium

  • internal/duckdb/rebuild.go:507: checkpointReplacedMirror opens the live destination read-write when a WAL exists, but a concurrent OpenReadOnly handle can prevent that open and cause the rebuild to fail before atomically swapping in the temporary mirror. Recover or checkpoint the old WAL without requiring a write-open of the reader-held destination, or coordinate WAL recovery before readers attach; add a regression test with an existing WAL and a held read-only handle.

    Reported by: codex

  • internal/postgres/push.go:1407, 2215-2218: Narrowing the archive-content policy changes the replication fingerprint and triggers an incremental push, but pushSession still copies dependent messages, tool payloads, and secret findings from the raw snapshot. The PostgreSQL mirror can therefore retain content excluded by the new policy. Apply archive-content projection to replication snapshots and secret findings before fingerprinting and writing the mirror, including removal of previously published excluded rows, or block pushes until a full resync/reprojection completes; add a regression test for narrowing policy with stale full-content rows.

    Reported by: codex (security)


Reviewers: 2 done | Synthesis: codex, 10s | Total: 54m12s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant