fix: harden gossip, HLC restarts, watcher races, MCP args, pagination

- gossip: validate push batches (4 MiB / 1000-row caps); reject rows
  claiming the local node id (vector-poisoning), empty ids, negative HCLs
- gossip: reconcile derived state after pulls (pulls only append to the
  observation log, so entry-count comparison could never trigger it)
- hlc: seek clock from persisted MAX(hcl) at Open so a restart with a
  regressed wall clock cannot reissue values (locator/cursor safety)
- db: serialize writers via BEGIN IMMEDIATE DSN, single conn per pool,
  and a per-KnoxDB mutex around RecordObservation's dedup
- watch: atomic ticker guards (was a cross-goroutine data race),
  trailing-edge per-path debounce, recursive directory watches,
  rename re-ingest, remove cancels pending ingests
- mcp: strict argument validation (no silent clamping), thread existence
  checks before writes, nil-safe golden-thread tool
- cli: --page 0 no longer panics; query/recent pagination actually pages
- tests: hlc SeekTo monotonicity, concurrent dedup race, push validation,
  batch caps, idempotency on observation counts
This commit is contained in:
2026-09-17 01:52:06 -07:00
parent 876d2aa45f
commit 6845975b7b
10 changed files with 733 additions and 116 deletions
+15 -2
View File
@@ -98,10 +98,16 @@ func nextCursor(rows []db.GossipObservation) int64 {
return rows[len(rows)-1].HCL
}
// maxBatchRows bounds the number of observations a peer may push in one POST.
// Pull already pages at 500 rows, so any larger batch is at best redundant and
// at worst a flood; capping keeps memory and insert work bounded.
const maxBatchRows = 1000
func (n *Node) handleBatch(w http.ResponseWriter, r *http.Request) {
r.Body = http.MaxBytesReader(w, r.Body, 4<<20) // 4 MiB
body, err := io.ReadAll(r.Body)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
http.Error(w, "batch too large or unreadable", http.StatusBadRequest)
return
}
var rows []db.GossipObservation
@@ -109,6 +115,10 @@ func (n *Node) handleBatch(w http.ResponseWriter, r *http.Request) {
http.Error(w, "bad json: "+err.Error(), http.StatusBadRequest)
return
}
if len(rows) > maxBatchRows {
http.Error(w, fmt.Sprintf("batch too large: %d rows (max %d)", len(rows), maxBatchRows), http.StatusBadRequest)
return
}
inserted, err := n.Kdb.PushObservations(rows)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
@@ -273,7 +283,7 @@ func (c *Client) Diff() (*DiffSummary, error) {
// node pulls/pushes directly with every other node it learns about.
//
// m, when non-nil, receives gossip event counters (nil for one-shot CLI runs).
func Run(kdb *db.KnoxDB, m *metrics.Metrics, static []string) {
func Run(kdb *db.KnoxDB, m *metrics.Metrics, static []string) int {
myID := kdb.NodeID()
// Seed the work queue with static config plus persisted discoveries.
@@ -284,6 +294,7 @@ func Run(kdb *db.KnoxDB, m *metrics.Metrics, static []string) {
seen := make(map[string]bool) // addr → handled (also suppresses self)
queue := 0
pulledTotal := 0
for queue < len(work) {
addr := strings.TrimSpace(work[queue])
queue++
@@ -338,6 +349,7 @@ func Run(kdb *db.KnoxDB, m *metrics.Metrics, static []string) {
pulled += n
}
}
pulledTotal += pulled
if m != nil {
m.IncrementPull(pulled)
}
@@ -356,6 +368,7 @@ func Run(kdb *db.KnoxDB, m *metrics.Metrics, static []string) {
log.Printf("[gossip] synced with %s (%s): in sync (pushed %d)", p.NodeID, addr, pushed)
}
}
return pulledTotal
}
func kdbVectorGet(kdb *db.KnoxDB, nodeID string) int64 {
+108 -4
View File
@@ -1,8 +1,12 @@
package watch
import (
"bytes"
"encoding/json"
"net/http"
"net/http/httptest"
"path/filepath"
"strings"
"testing"
"github.com/david/knox/internal/db"
@@ -228,16 +232,116 @@ func TestGossipIdempotent(t *testing.T) {
defer sb.Close()
Run(b, nil, []string{sa.URL})
before, err := b.EntryCount()
entryBefore, err := b.EntryCount()
if err != nil {
t.Fatal(err)
}
stats, err := b.Stats()
if err != nil {
t.Fatal(err)
}
obsBefore, _ := stats["total_observations"].(int)
Run(b, nil, []string{sa.URL})
after, err := b.EntryCount()
entryAfter, err := b.EntryCount()
if err != nil {
t.Fatal(err)
}
if before != after {
t.Errorf("second sweep changed entry count: %d -> %d", before, after)
if entryBefore != entryAfter {
t.Errorf("second sweep changed entry count: %d -> %d", entryBefore, entryAfter)
}
stats, err = b.Stats()
if err != nil {
t.Fatal(err)
}
obsAfter, _ := stats["total_observations"].(int)
if obsBefore != obsAfter {
t.Errorf("second sweep duplicated observations: %d -> %d", obsBefore, obsAfter)
}
}
// TestHandleBatchRejectsOversizedBatch: more than maxBatchRows in one POST
// must be refused up front, before any insert work.
func TestHandleBatchRejectsOversizedBatch(t *testing.T) {
b := tmpKnoxDB(t)
node := &Node{Kdb: b, Name: "B"}
sv := httptest.NewServer(node.Handler())
defer sv.Close()
rows := make([]db.GossipObservation, maxBatchRows+1)
for i := range rows {
rows[i] = db.GossipObservation{
NodeID: "0123456789abcdef0123456789abcdef", HCL: int64(i + 1),
Fingerprint: "fp:oversized", SourceID: "test", Title: "t", Summary: "s",
CollectedAt: "2026-08-29T00:00:00Z",
}
}
body, _ := json.Marshal(rows)
resp, err := http.Post(sv.URL+"/v1/obs/batch", "application/json", bytes.NewReader(body))
if err != nil {
t.Fatalf("post: %v", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusBadRequest {
t.Errorf("oversized batch: want 400, got %d", resp.StatusCode)
}
}
// TestHandleBatchRejectsHugeBody: an oversized body (beyond the 4 MiB cap)
// must be refused even when the row count is small.
func TestHandleBatchRejectsHugeBody(t *testing.T) {
b := tmpKnoxDB(t)
node := &Node{Kdb: b, Name: "B"}
sv := httptest.NewServer(node.Handler())
defer sv.Close()
rows := []db.GossipObservation{{
NodeID: "0123456789abcdef0123456789abcdef", HCL: 1,
Fingerprint: "fp:huge", SourceID: "test", Title: "t",
Summary: strings.Repeat("x", 5<<20), // 5 MiB summary
CollectedAt: "2026-08-29T00:00:00Z",
}}
body, _ := json.Marshal(rows)
resp, err := http.Post(sv.URL+"/v1/obs/batch", "application/json", bytes.NewReader(body))
if err != nil {
t.Fatalf("post: %v", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusBadRequest {
t.Errorf("huge body: want 400, got %d", resp.StatusCode)
}
}
// TestHandleBatchSkipsSelfRows: rows claiming the receiver's own node_id are
// dropped at the HTTP layer too (the poisoning vector), reported as conflicts.
func TestHandleBatchSkipsSelfRows(t *testing.T) {
b := tmpKnoxDB(t)
node := &Node{Kdb: b, Name: "B"}
sv := httptest.NewServer(node.Handler())
defer sv.Close()
rows := []db.GossipObservation{
{NodeID: b.NodeID(), HCL: 1, Fingerprint: "fp:self1", SourceID: "test", Title: "t", Summary: "s", CollectedAt: "2026-08-29T00:00:00Z"},
{NodeID: b.NodeID(), HCL: 2, Fingerprint: "fp:self2", SourceID: "test", Title: "t", Summary: "s", CollectedAt: "2026-08-29T00:00:00Z"},
}
body, _ := json.Marshal(rows)
resp, err := http.Post(sv.URL+"/v1/obs/batch", "application/json", bytes.NewReader(body))
if err != nil {
t.Fatalf("post: %v", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
t.Fatalf("want 200, got %d", resp.StatusCode)
}
var out struct {
Accepted int `json:"accepted"`
Conflict int `json:"conflict"`
}
if err := json.NewDecoder(resp.Body).Decode(&out); err != nil {
t.Fatalf("decode: %v", err)
}
if out.Accepted != 0 || out.Conflict != 2 {
t.Errorf("self rows: want accepted=0 conflict=2, got accepted=%d conflict=%d", out.Accepted, out.Conflict)
}
}
+136 -63
View File
@@ -1,11 +1,14 @@
package watch
import (
"io/fs"
"log"
"net/http"
"os"
"path/filepath"
"strings"
"sync"
"sync/atomic"
"time"
"github.com/david/knox/internal/db"
@@ -28,6 +31,44 @@ type Watcher struct {
metrics *metrics.Metrics
}
// fileDebouncer schedules one ingest per path a settle-window after the last
// relevant event (trailing edge): writers that emit bursts (multi-write appends,
// atomic save = temp-write + rename) settle before anything is read, so a
// partial file is never recorded as final state. Timers are removed once they
// fire, keeping the map bounded by recently-active paths.
type fileDebouncer struct {
mu sync.Mutex
timers map[string]*time.Timer
}
func newFileDebouncer() *fileDebouncer {
return &fileDebouncer{timers: make(map[string]*time.Timer)}
}
func (d *fileDebouncer) schedule(path string, delay time.Duration, fn func(string)) {
d.mu.Lock()
defer d.mu.Unlock()
if t, ok := d.timers[path]; ok {
t.Stop()
}
d.timers[path] = time.AfterFunc(delay, func() {
d.mu.Lock()
delete(d.timers, path)
d.mu.Unlock()
fn(path)
})
}
// cancel drops any pending ingest for path (e.g. the file was deleted).
func (d *fileDebouncer) cancel(path string) {
d.mu.Lock()
defer d.mu.Unlock()
if t, ok := d.timers[path]; ok {
t.Stop()
delete(d.timers, path)
}
}
func New(kdb *db.KnoxDB, dirs []string) *Watcher {
// Detect Obsidian vault
vault, _ := ingest.DetectObsidianVault()
@@ -86,17 +127,13 @@ func (w *Watcher) Start() error {
log.Printf("[knox] obsidian vault: %s", w.vault)
}
debounceMap := make(map[string]time.Time)
debounce := newFileDebouncer()
browserTicker := time.NewTicker(browserInterval)
giteaTicker := time.NewTicker(10 * time.Minute)
gitTicker := time.NewTicker(10 * time.Minute)
threadTicker := time.NewTicker(10 * time.Minute)
gossipTicker := time.NewTicker(gossipInterval)
browserRunning := false
giteaRunning := false
gitRunning := false
threadRunning := false
gossipRunning := false
var browserRunning, giteaRunning, gitRunning, threadRunning, gossipRunning atomic.Bool
for {
select {
@@ -104,63 +141,76 @@ func (w *Watcher) Start() error {
if !ok {
return nil
}
if !w.isRelevantEvent(event) {
continue
// fsnotify is non-recursive: files appearing inside newly created
// subdirectories would otherwise be invisible to the daemon.
isDir := false
if event.Has(fsnotify.Create) {
if info, err := os.Stat(event.Name); err == nil && info.IsDir() {
isDir = true
if err := watcher.Add(event.Name); err != nil {
log.Printf("[knox] cannot watch new dir %s: %v", event.Name, err)
} else {
log.Printf("[knox] watching new dir %s", event.Name)
}
}
}
now := time.Now()
if last, ok := debounceMap[event.Name]; ok && now.Sub(last) < w.debounce {
if event.Has(fsnotify.Remove) {
// Drop any pending ingest for a deleted file. (No tombstone is
// written yet — the entry lingers until reconcile/prune.)
debounce.cancel(event.Name)
continue
}
if isDir || !w.isRelevantEvent(event) {
continue
}
debounceMap[event.Name] = now
trigger := eventOpName(event.Op)
log.Printf("[knox] %s %s", trigger, filepath.Base(event.Name))
w.ingestFile(event.Name, trigger)
name := event.Name
debounce.schedule(name, w.debounce, func(path string) {
w.ingestFile(path, trigger)
})
case <-browserTicker.C:
if browserRunning {
if !browserRunning.CompareAndSwap(false, true) {
continue
}
browserRunning = true
go func() {
defer func() { browserRunning = false }()
defer browserRunning.Store(false)
w.ingestBrowserHistory()
}()
case <-giteaTicker.C:
if giteaRunning {
if !giteaRunning.CompareAndSwap(false, true) {
continue
}
giteaRunning = true
go func() {
defer func() { giteaRunning = false }()
defer giteaRunning.Store(false)
w.ingestGitea()
}()
case <-gitTicker.C:
if gitRunning {
if !gitRunning.CompareAndSwap(false, true) {
continue
}
gitRunning = true
go func() {
defer func() { gitRunning = false }()
defer gitRunning.Store(false)
w.ingestGit()
}()
case <-threadTicker.C:
if threadRunning {
if !threadRunning.CompareAndSwap(false, true) {
continue
}
threadRunning = true
go func() {
defer func() { threadRunning = false }()
defer threadRunning.Store(false)
w.autoThread()
}()
case <-gossipTicker.C:
if gossipRunning {
if !gossipRunning.CompareAndSwap(false, true) {
continue
}
gossipRunning = true
go func() {
defer func() { gossipRunning = false }()
defer gossipRunning.Store(false)
w.syncGossip()
}()
@@ -175,44 +225,38 @@ func (w *Watcher) Start() error {
func (w *Watcher) seed(watcher *fsnotify.Watcher) {
for _, dir := range w.dirs {
abs, _ := filepath.Abs(dir)
if err := watcher.Add(abs); err != nil {
log.Printf("[knox] cannot watch %s: %v", abs, err)
continue
if err := w.watchTree(watcher, dir); err != nil {
log.Printf("[knox] cannot watch %s: %v", dir, err)
}
log.Printf("[knox] watching %s", abs)
}
// Watch Obsidian vault
// Watch Obsidian vault (every subdirectory, non-recursively mirrored)
if w.vault != "" {
if err := watcher.Add(w.vault); err != nil {
if err := w.watchTree(watcher, w.vault); err != nil {
log.Printf("[knox] cannot watch obsidian vault %s: %v", w.vault, err)
} else {
log.Printf("[knox] watching %s (obsidian)", w.vault)
}
}
// Seed existing files
for _, ing := range w.fileIngesters {
for _, dir := range w.dirs {
patterns := []string{
filepath.Join(dir, "*"),
filepath.Join(dir, "*", "SKILL.md"),
// Seed existing files anywhere under the watched dirs. fsnotify watches the
// whole tree, so live events cover any depth; this walk covers startup so
// pre-existing nested files (e.g. skills at two+ levels) are indexed too.
for _, dir := range w.dirs {
_ = filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
if err != nil || d.IsDir() {
return nil // skip unreadable entries; dirs are handled by the watch
}
for _, pattern := range patterns {
entries, _ := filepath.Glob(pattern)
for _, path := range entries {
if !MatchesIngester(path, ing.SourceID()) {
continue
}
for _, ing := range w.fileIngesters {
if MatchesIngester(path, ing.SourceID()) {
w.ingestFileWith(path, ing, "seed")
return nil
}
}
}
return nil
})
}
// Seed Obsidian notes via the full walk: skips dot-dirs (.trash, .obsidian)
// and covers all depths — glob patterns would match dot-dirs and miss depth >2.
// and covers all depths.
if w.vault != "" {
if notes, err := ingest.NewObsidianIngester(w.vault).IngestAll(); err == nil {
for _, r := range notes {
@@ -222,6 +266,39 @@ func (w *Watcher) seed(watcher *fsnotify.Watcher) {
}
}
// watchTree adds a directory and every non-hidden subdirectory to the watcher,
// mirroring fsnotify's non-recursive API with an explicit walk. Hidden
// directories (.git, .obsidian, .trash) are skipped so their churn doesn't burn
// inotify watches.
func (w *Watcher) watchTree(watcher *fsnotify.Watcher, root string) error {
rootAbs, err := filepath.Abs(root)
if err != nil {
return err
}
if err := watcher.Add(rootAbs); err != nil {
return err
}
log.Printf("[knox] watching %s", rootAbs)
return filepath.WalkDir(rootAbs, func(path string, d fs.DirEntry, err error) error {
if err != nil {
return nil
}
if !d.IsDir() {
return nil
}
if path != rootAbs && strings.HasPrefix(d.Name(), ".") {
return filepath.SkipDir
}
if path == rootAbs {
return nil
}
if err := watcher.Add(path); err != nil {
log.Printf("[knox] cannot watch %s: %v", path, err)
}
return nil
})
}
func (w *Watcher) ingestFile(path, trigger string) {
for _, ing := range w.fileIngesters {
if MatchesIngester(path, ing.SourceID()) {
@@ -456,29 +533,23 @@ func (w *Watcher) syncGossip() {
return
}
before := 0
if n, err := w.knoxDB.EntryCount(); err == nil {
before = n
}
pulled := Run(w.knoxDB, w.metrics, peers)
Run(w.knoxDB, w.metrics, peers)
// If new observations arrived, reconcile to pick up entries/threads they
// imply (deterministic log → derived rebuild).
if after, err := w.knoxDB.EntryCount(); err == nil && after > before {
// A pull only appends to the observation log; the entries cache and
// auto-threads are derived state that reconcile rebuilds. Comparing entry
// counts can never trigger this (pushes/pulls never touch entries directly),
// so reconcile fires on the sweep's newly-inserted observation count.
if pulled > 0 {
created, linked, err := Reconcile(w.knoxDB)
if err != nil {
log.Printf("[knox] gossip reconcile: %v", err)
return
}
log.Printf("[knox] gossip reconcile done: %d created, %d linked", created, linked)
log.Printf("[knox] gossip reconcile done: %d created, %d linked after pulling %d obs", created, linked, pulled)
}
}
func (w *Watcher) isRelevantEvent(event fsnotify.Event) bool {
if event.Has(fsnotify.Remove) || event.Has(fsnotify.Rename) {
return false
}
name := filepath.Base(event.Name)
// Opencode session diffs and logs
if strings.HasPrefix(name, "ses_") || strings.HasSuffix(name, ".log") {
@@ -512,6 +583,8 @@ func eventOpName(op fsnotify.Op) string {
return "inotify:WRITE"
case op.Has(fsnotify.Chmod):
return "inotify:CHMOD"
case op.Has(fsnotify.Rename):
return "inotify:RENAME"
default:
return "inotify:UNKNOWN"
}