feat(mcp,kkernel): events-daemon transport cutover (ADR-170, part 3) - #2202
Open
ohdearquant wants to merge 42 commits into
Open
feat(mcp,kkernel): events-daemon transport cutover (ADR-170, part 3)#2202ohdearquant wants to merge 42 commits into
ohdearquant wants to merge 42 commits into
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.
ohdearquant
force-pushed
the
feat/events-split-s2-config
branch
from
August 25, 2026 01:15
ed4455d to
a37cc01
Compare
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.
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
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
…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
…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
… 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
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 by itself.
Verdict on head 80fc2ee: REQUEST-CHANGES, 1 blocking finding. Finding details are delivered to the review's recipients rather than posted here. Do not merge this head while blocking findings are outstanding; a pipeline comment on a newer head supersedes this one.
…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.
The events sidecar path is derived from the canonicalized main-database path, never user-chosen, so a pre-existing symlink at the sidecar path or its -wal/-shm companions is a planted redirect: permission hardening and the SQLite open would follow it and tighten or write event rows through to the link's target (CWE-59). Admission now refuses symlinks at these paths before any open, on both the embedded backend and the daemon create path. Tests cover a symlink to an existing file (target proven untouched), a dangling symlink (no file minted at its target), a planted -wal symlink, and a regular-file control that must proceed.
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 by itself.
Verdict on head b0d454f: REQUEST-CHANGES, 2 blocking findings. Finding details are delivered to the review's recipients rather than posted here. Do not merge this head while blocking findings are outstanding; a pipeline comment on a newer head supersedes this one.
The wire PageRequest.limit is client-supplied u32 and the daemon materializes the full page as a Vec<Event> before serializing it into one response frame, so an unbounded limit is attacker-controlled memory and serialization work. Over-cap requests now get a typed non-retryable refusal naming the cap and the requested value; requests at or under the cap proceed unchanged. Refusal over silent clamping: a clamped short page would read as end-of-data to a paginating caller. The cap also bounds the prefix window (offset + limit) the split client's merged read can demand in one request.
Couples the symlink defense to its uses instead of racing them. The hardening step now opens each target with O_NOFOLLOW and chmods the returned handle, so no path re-lookup exists between validation and the permission change. SQLite's own open is path-based and cannot be pinned from here, so both the embedded and daemon arms now require the events directory to pass the same traversal-trust walk the daemon socket uses: owner-or-root components, no group/other-writable directories, sticky ancestors per the existing rule. The dangling-alias resolver's hop bound rises to the traversal guard's 40 so a 33-40-link chain the kernel would resolve derives the target's sidecar instead of an intermediate name. Each defense carries an arm that fails when that defense alone is removed.
Merges the QueryEvents page cap from the module slice. Socket-directory trust now runs before any lock-path operation; the lock open pins its final component with O_NOFOLLOW and tightens permissions on the returned descriptor, failing closed. Regressions cover a symlinked lock entry and exhaustive event pagination across multiple timestamp groups per page.
The events daemon transport rejects query pages larger than MAX_QUERY_EVENTS_PAGE_ROWS, and the split store forwards offset+limit as a single daemon page, so any consumer requesting more rows than the cap in one call fails outright. Brain's window and exhaustive fetches requested up to 50k/10k rows per page. - Export MAX_QUERY_EVENTS_PAGE_ROWS (now pub, platform-independent) with docs requiring consumers to cursor-walk in <=cap pages. - Add collect_events_cursor_walk in khive-pack-brain: before-cursor pagination with boundary dedup, bounded widening capped at the transport page limit, a typed error for dense timestamp ties, and a final truncate so the row budget is never exceeded. - Route both fetch_event_counts_window arms and fetch_event_counts_window_exhaustive through the shared walk; truncation is judged from count_events totals versus collected rows. - Regression test: exhaustive fetch terminates across multiple timestamp groups per page with exact-once delivery.
Two cursor-walk edge cases in the capped event pagination: - A timestamp tie that exactly fills a transport-cap page was reported as unpageable even though it was fully collected. When an at-the-cap page comes back all-duplicates, the walk now proves the tie complete against count_events (at-or-above the boundary versus rows collected, which are all at-or-above by construction) and steps the strict before-bound to the boundary itself; only a run genuinely wider than the cap keeps the typed dense-tie error. The at-or-above count uses the strict after-bound as boundary - 1. - Advancing the cursor with saturating_add(1) at created_at == i64::MAX left the strict bound at i64::MAX, silently excluding uncollected rows at that timestamp. The walk now holds the cursor and re-reads: dedup drops re-admitted rows and the completeness check advances past the boundary once the group is fully collected. Regressions: an exactly-cap tie (4096 rows plus one older) pages past and reaches the older row while one extra tied row restores the typed error; a max-timestamp group is collected exactly once with no rows skipped. Both verified load-bearing by mutation controls.
The walk's page reads and at-the-cap completeness count are independent reads of a live event plane with no snapshot spanning them, so a row appended concurrently with the walk can be excluded from that walk's view — exactly as it would be by a single bounded read predating it. It is never duplicated and never an error. State that contract on the walk and on the event_counts verb doc (bound until in the past for a closed population), and pin it with a regression: a delegating store injects a boundary-timestamp append between the completeness count and the next page query; the walk completes with the pre-append population, no duplicate, no error, and the test proves the injection actually fired.
…cutover # Conflicts: # crates/khive-pack-brain/src/handlers.rs # crates/khive-pack-brain/src/tests.rs # crates/khive-runtime/src/events_split.rs
…exact Residual wording outlived the read-consistency contract: the exhaustive parameter description and the response-schema comment still promised an exact aggregate, the walk doc called the result a point-in-time view although an independent live read can also INCLUDE a row appended mid-walk (one landing below the cursor's standing boundary), and the verb doc called window_event_total exact without naming it an independent read. All sites now state one contract: a non-sampled best-effort live-window view in which concurrent appends may be included or excluded, the total is counted at its own read instant, and a past until bound closes the population only because the event plane stamps rows at append time rather than backdating them.
The events store now opens beside the main database and its parent directory passes the same trust walk as the daemon socket directory, which refuses world-writable parents including sticky /tmp. The contract harness placed each test database bare in the system temp directory, so every server refused to serve events. Give each test its own private mkdtemp working directory holding the database and config, removed on exit.
…age cap train_preference requested MAX_TRAINING_EVENTS+1 (50,001) judgment rows in a single query_events call, which the events daemon refuses outright above its 4,096-row page cap. Cursor-walk the read in transport-cap-sized pages instead, matching the technique the events-split store already uses internally, so training keeps working once the events daemon is in the loop.
Both events-forwarder shutdown paths broke on ADR-170's visibility contract: the queued-batch arm and the mid-delivery arm both exited without touching dropped_batches/dropped_events or logging, even though the code comment claimed queued batches were a counted drop. Drain and count every queued batch on shutdown (one summary log line, not one per batch), and count/log the in-flight batch separately when shutdown cancels mid-delivery.
brain.event_counts moved from single-snapshot to cursor-walked, best-effort live-view aggregation so its reads stay under the events daemon's page cap (khive-pack-brain#collect_events_cursor_walk). The handler doc and tests already carried the relaxed contract; this adds the governing decision record so the change has ADR cover instead of standing on code comments alone.
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 3 of the events-daemon split series (ADR-170). Builds on #2195.
Wires the transport cutover and the operational surfaces:
kkernel mcpdaemon mode supervises the events daemon when a split is configured; a read-only deployment skips supervision entirely (the runtime guard from part 2 already keeps its reads on the direct lane).created_at, which previously made any window containing such a burst permanently error.<main-file-name>.events.db), plus the exec/audit-batch plumbing.