test(runtime): events-daemon split test suite (ADR-170, part 4) - #2203
Merged
Conversation
The events_split module: daemon loop, forwarding client, split store, and the shared config/naming helpers. Nothing constructs it yet — runtime routing, transport wiring, and the full daemon test suite land in the following parts of this series. Two contracts here are deliberate corrections to the draft this series replaces: the events sidecar derives from the main database's full file name with a canonicalized parent (a stem-derived name silently shares one sidecar between a.db and a.sqlite, and path aliases would mint one sidecar per spelling), and wire retryability defers to StorageError::is_retryable() instead of re-enumerating variants (a hand-rolled subset turned transient writer contention into a terminal client error).
ohdearquant
marked this pull request as ready for review
August 24, 2026 23:27
…semantics
- Create the events database owner-only (0600) before SQLite opens it, and
tighten existing db/-wal/-shm sidecars fail-closed at daemon boot: the 0600
socket and peer-uid admission bound nothing if the database file beside
them is world-readable. Embedded writable mode creates the file 0600 too;
the guard lock file is created owner-only.
- Carry side_effects_unknown across the wire as its own Error field and
reconstruct WriterTaskTerminated{SideEffectsUnknown} client-side; admit
Pool/Timeout into the audit driver's bounded retry set so transport-level
transience keeps the same retry behavior as a direct store.
- Make the fire-and-forget forwarder observe the daemon shutdown token, so
drain() no longer waits its full timeout on a task pinned by the
process-global client registry.
- Absolutize the events db/socket paths at derivation and at daemon entry:
a bare relative spelling has an empty parent, which broke lock-file
parenting and socket-directory validation.
- Cache per-namespace stores in the daemon: events_for_namespace takes a
writer-lane checkout and re-runs DDL per call; pay it once per namespace.
- Document the O(offset+limit) materialization floor on merged pagination.
Tests: absolute-sidecar derivation, wire marker set/round-trip/older-frame
compat, client-side variant reconstruction.
RuntimeConfig gains the optional events_split section; KhiveRuntime::events routes by append class when it is set — the idempotent audit-batch lane to the events database, plain appends to the legacy store, reads merged across both. Remaining files are the mechanical field addition to existing RuntimeConfig literals. One correction over the draft this series replaces: read-only is decided before the transport question. A read-only runtime never forwards writes to the events daemon (previously the socket arm ignored read-only), never creates or schema-initializes the sidecar, and serves merged reads from a read-only open only when the sidecar already exists.
The two resident daemon hosts (khive-mcp serve under --daemon, kkernel mcp) upgrade the resolved event plane from direct mode to socket forwarding and supervise an events daemon at the derived socket. kkernel gains the events-daemon subcommand; the audit-batch flusher and brain.event_counts ride the split store. Two corrections over the draft this series replaces: - A read-only deployment never supervises an events daemon (the supervised process opens the sidecar writable); together with the runtime-side refusal to forward writes from a read-only backend, both halves of the read-only guarantee fail safe. - brain.event_counts no longer errors when one microsecond holds more rows than a page — an idempotent audit batch stamps every row in one transaction, so bursts routinely share a created_at. The keyset walk drains such a cluster with offset pagination scoped to exactly its microsecond (sound under the store's created_at,id ordering), then resumes below it.
ohdearquant
force-pushed
the
feat/events-split-s3-cutover
branch
from
August 25, 2026 01:15
7084801 to
ba9149a
Compare
…ning The base slice and this one each added the transient-transport arm to the audit retry classifier; the merge kept both and the second was unreachable. One arm remains.
Full daemon/forwarding/read-merge test coverage for the events_split module, plus the idempotent audit-batch integration case. The sidecar naming test from the original branch is superseded by the naming tests already in the module (full-file-name derivation, canonicalized parent).
ohdearquant
force-pushed
the
feat/events-split-s4-tests
branch
from
August 25, 2026 01:22
9c39b3f to
63c1cf1
Compare
ohdearquant
commented
Aug 25, 2026
ohdearquant
left a comment
ohdearquant
left a comment
Owner
Author
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge.
Verdict on current head: APPROVE, zero blocking findings. This is a comment, not an approval — a human reviewer decides whether to approve and merge.
Three defects in the ADR-170 split routing, plus the pagination change the last of them forces: - Idempotent audit batches consulted only the events lane, so a retry of a batch that landed on the legacy store before the cutover would insert a second copy of each id into the lane and merged reads would double-count it. The split store now probes the legacy store for the batch's ids and routes resident rows through the legacy store's own compare-without-reinsert machinery; only genuinely new rows reach the lane, with dispositions stitched back in input order. - kg's unfiltered by-id event lookup read only the legacy events table, reporting sidecar-only rows as not found. The runtime now exposes a read-only SQL handle on the sidecar (never creating one as a side effect of a read), and the lookup falls through to it on a miss. - The merged offset window materialized offset+limit rows from both stores with no bound, so one request with a pathological offset could buffer both stores wholesale. The window is now bounded at 100k rows with a typed refusal naming the cursor remedy; every in-tree caller pages at offset 0 within the bound. - brain.event_counts' exhaustive walk paged with a growing offset, which under the split re-materialized an ever-larger two-store prefix per page (quadratic) and would eventually hit the new bound. It now walks a descending strict before-cursor at offset 0 — linear, bounded, and tie-safe: the cursor steps past the boundary microsecond and re-read boundary rows are dropped by id, covered by a page-edge tie test.
…cutover # Conflicts: # crates/khive-pack-brain/src/handlers.rs
ohdearquant
commented
Aug 25, 2026
ohdearquant
left a comment
ohdearquant
left a comment
Owner
Author
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge.
Verdict on current head: APPROVE, zero blocking findings. This is a comment, not an approval — a human reviewer decides whether to approve and merge.
The prefix resolver scanned only the main store, so a sidecar-only event id could not be resolved through the public short-id path even though the by-id path already falls back. The sidecar scan merges into the same match set, keeping cross-file ambiguity detection. Also open the sidecar through the writable binding on writable runtimes: the read-only binding's frozen-snapshot guard refuses any sidecar with a live writer's -shm beside it, which is exactly the live deployment these reads serve; read-only runtimes keep the frozen-snapshot semantics. Regression test writes a lane-only event and resolves its prefix, with a pre-insert miss as control.
…o feat/events-split-s3-cutover
…to feat/events-split-s4-tests
…bedded db modes Four fixes to the events-split module: - Writer-task terminations now cross the socket with their request state verbatim (new optional writer_task_state wire field). NotStarted and TransactionRolledBack previously flattened into a generic non-retryable refusal and reconstructed as terminal InvalidInput on the client, breaking the audit-batch retry contract that classifies those states as safe replays. The older single-state side_effects_unknown field is kept for frame-skew tolerance. - The forwarder's per-batch delivery is now cancellable and bounded: a connected but non-responding daemon previously parked the tracked task inside write_frame/read_frame beyond the reach of the recv-side shutdown select, hanging daemon drain. Delivery now races the shutdown token and a delivery timeout; timeout poisons the connection. - direct_backend() in writable mode now applies the daemon's fail-closed sidecar hardening to pre-existing databases and -wal/-shm files instead of only pre-creating a missing file at 0600. - The daemon now caps concurrently served connections, applies a per-frame I/O deadline on served connections (a partial-frame or non-reading peer is closed instead of holding a task and descriptor indefinitely), and bounds the per-namespace store cache with trim-normalized keys matching the backend's own normalization. Regression tests cover each: wire round-trip of all three writer-task states with a non-writer-task control, hung-delivery abandonment with a never-responding listener, pre-existing 0644 db+wal tightened to 0600 with a pre-open mode control, and cache bound + trim normalization.
…o feat/events-split-s2-config
…o feat/events-split-s3-cutover
…to feat/events-split-s4-tests
…ngle wire carrier Three admission-control gaps on the events daemon serve path: - Request-frame buffers are now admitted against a shared 64 MiB byte budget before allocation. The per-frame cap bounds one buffer and the connection cap bounds task count, but their product allowed ~1 GiB of declared-length allocation across 128 connections. Admission waits inside the existing per-connection I/O deadline, so budget exhaustion degrades into connection timeouts, never daemon memory growth. - Wire namespaces are validated as real Namespace values (charset plus the 256-byte bound) before they can become cache keys, stores, or rows; oversized or malformed namespaces get a typed non-retryable refusal. The per-namespace store cache now evicts an arbitrary entry at its cap instead of permanently refusing admission, which also removes the per-request store-rebuild penalty for namespaces arriving after the cap filled. - writer_task_state is the single wire carrier of ADR-133 writer dispositions; the redundant side_effects_unknown field is removed. The protocol and the field are born in the same revision, so no deployed daemon emits the old shape, and cross-version frames never reach the client mapping because dispatch refuses a mismatched protocol version outright. Each fix carries a regression test with a control, and each test was verified to fail with its fix reverted.
… sidecars A final-component symlink alias of one database previously derived its sidecar and socket from the alias's own file name, while backend identity canonicalizes the whole path and treats the alias and its target as one database. A process opening one spelling could then write audit-batch rows to an event store a process opening the other spelling never reads. Canonicalize the whole path when the database file exists; keep the parent-only canonicalization for databases that do not exist yet. Adds a symlink-alias regression test with a distinct-file control.
Version 1 carried the side_effects_unknown error field and never reached a released ref; the writer_task_state carrier replaced it during development. Shipping as version 2 makes the version check the failure mode for any process built from an unreleased v1 head: a typed refusal instead of retryable writer states silently mapping to terminal InvalidInput.
…o feat/events-split-s2-config
…arquant/khive into feat/events-split-s3-cutover
…earquant/khive into feat/events-split-s4-tests
… to the version guarantee The client mapping's terminal arm (retryable: false, no writer_task_state) had no test: nothing pinned that a stateless non-retryable frame maps to InvalidInput rather than a reconstructed writer state or the retryable Pool arm. New test covers it with a with-state control proving the arm is selected by the field's absence. The two wire docstrings claiming cross-version frames never reach the mapping now say WHY that holds — the version refusal precedes any write, so a skewed peer has no writer state to lose — and name the obligation that keeps it true: any field-shape change bumps EVENTS_PROTOCOL_VERSION.
…arquant/khive into feat/events-split-s2-config
…arquant/khive into feat/events-split-s3-cutover
…earquant/khive into feat/events-split-s4-tests
…n cold start Event-path derivation runs before backend creation, so a symlink alias can be consulted while its target does not exist yet — and canonicalize refuses dangling links, so the previous fallback derived the sidecar from the alias's own name. An alias-first cold start and a later target-spelled process then used different event stores. Follow final-component links by hand on that arm (relative targets anchored at each link's parent, kernel-style chain bound): a dangling alias now derives the sidecar its target will use once the first open creates it. Regression test covers the dangling alias, a two-link chain, and a distinct-file control; no-opping the resolver reddens it.
…earquant/khive into feat/events-split-s4-tests
ohdearquant
commented
Aug 25, 2026
ohdearquant
left a comment
ohdearquant
left a comment
Owner
Author
There was a problem hiding this comment.
Automated review. Posted by this repository's automated pull-request review pipeline; this is not a human read and does not gate the merge.
Verdict on current head: APPROVE, zero blocking findings. This is a comment, not an approval — a human reviewer decides whether to approve and merge.
Resolves 24 conflict hunks in events_split.rs plus one each in khive-pack-brain/handlers.rs, khive-pack-brain/tests.rs, and khive-runtime/operations.rs. PR #2202 (878db6d, merged to main before this branch's conflicts were cut) fixed a real defect: the forwarder's backoff shutdown arm exited with a bare `break` and never drained queued fire-and-forget batches, silently losing accepted writes on shutdown. #2202 also added symlink/ directory-trust hardening around the events sidecar, a dangling-alias resolver, and a query-page-size cap. All of that is main's territory and wins here. This branch independently reimplemented several of the same fixes (narrower symlink dangling-alias resolution, a brain-pack cursor-walk pagination fix, a sidecar-aware prefix resolver) against the pre-#2202 code; main's versions are supersets and win those hunks too. The branch's own ADR-170 part 4 test suite (~500 lines of new tests in events_split.rs, none of which main has) is kept in full. See .khive/IMPL_REPORT_2203_merge.md for the per-hunk resolution table and gate results.
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.
Part 4 (final) of the events-daemon split series (ADR-170). Builds on the part-3 cutover.
Test-only: the full daemon/forwarding/read-merge suite for the
events_splitmodule (routing, raw-SQL legacy visibility, idempotent daemon round-trips, queue overflow accounting, dead-socket and protocol-skew refusals, read-only never-creates guarantee, direct mode), plus the idempotent audit-batch integration case. The original branch's sidecar naming test is superseded by the naming tests already in the module (full-file-name derivation, canonicalized parent).