fix(plugins/db/elasticsearch): advertise host-reachable publish address (sniff fix)#15
Merged
Merged
Conversation
auba-api boot crashed at ES init even though http://127.0.0.1:56205 was reachable from the host: [ELASTIC] elastic: http://172.17.0.4:9200 joined the cluster [ERROR] elastic: http://172.17.0.4:9200 is dead [PANIC] failed to get ESConn: no Elasticsearch node available 172.17.0.4 is the elasticsearch container's Docker bridge IP. The olivere/elastic client opened the host-reachable http://127.0.0.1:56205, then sniffed the cluster via `_nodes/http` and learned the node was at 172.17.0.4:9200 — the container-internal HTTP address, which is unreachable from the host. The client swapped to that URL for the next round-trip and crashed. Fix: tell ES to advertise the loopback the operator already uses, and override the HTTP publish port to the bough-allocated host port. Two new env vars on the elasticsearch container: network.publish_host=127.0.0.1 http.publish_port=<hostPort> After the fix `_nodes/http` reports `publish_address: 127.0.0.1:<hostPort>` and sniffing clients dial the host-reachable mapping bough already wires. Refactor: extract `buildDockerEnv(heap, hostPort)` so the regression guard tests can assert both new lines without spinning up a real Docker daemon. Two unit tests cover the contract: - TestBuildDockerEnv_PublishHostAndPort — the two new lines AND the pre-existing must-haves (single-node, security off, disk threshold, ES_JAVA_OPTS) are all present. - TestBuildDockerEnv_HeapOverride — the caller's heap choice still reaches ES_JAVA_OPTS so `extras["es.heap"]=512m` keeps working.
ikeikeikeike
added a commit
that referenced
this pull request
Jun 20, 2026
…regression tests Round 3 code review (= post-PR-open review against the v0.5 implementation plan) surfaced 5 CRITICAL + 6 HIGH + 6 MEDIUM + 5 LOW issues. This commit lands the CRITICAL fixes, the highest- priority HIGH fixes, the supporting unit tests, and the few LOW items that piggyback for free. CRITICAL fixes 1. plugins/memory/sqlite/sqlite.go Store — the previous INSERT-OR-REPLACE fall-through on a dedupe match with `UpsertSemantics: false` could either insert a duplicate row sharing the dedupe_key (silent contract violation) or wipe out the existing row's hits / last_hit_at / evidence_refs. We now reject the call explicitly with an actionable error so the caller can either set UpsertSemantics=true or change the rule. Regression test: TestStore_DedupeWithoutUpsertReturnsError. 2. plugins/memory/sqlite/sqlite.go Query / normalizeFTSTerm — stripping only ASCII `"` left FTS5 query-language injection open via unicode quotes, NEAR, column filters, and null bytes. normalizeFTSTerm now keeps only Letters, Digits, Spaces, hyphen, and underscore — none of which carry FTS5 special meaning inside a phrase quote. An empty post-normalisation term falls through to the scope-only query path. Regression test: TestQuery_FTSNormalisation. 3. internal/instinct/coordinator.go Close — added an explicit `c.events != nil` guard so a future surface change to the writer cannot silently reintroduce the nil-deref hazard. Regression test: TestCoordinator_Close_NilEvents. 4. internal/instinct/decay_scheduler.go RunOnce — Query now uses MaxTokens=0 (no token cap) so the decay walk does not stop short on a token-budget boundary; MaxResults=1000 still bounds it. The Store reinforce path (CRITICAL #1 sibling fix) now propagates the incoming state so an active row can transition to archived through Store. Regression test: TestStore_UpsertUpdatesState + TestDecayScheduler_CandidateExpiry. 5. internal/instinct/builtin_minter.go Mint — when a TraceBundle carries its own non-zero Scope, the minter now prefers it over the batch-level scope argument. Without this fix a coordinator wiring multiple worktrees through one Ingest call would collapse all candidates to a single scope and lose isolation. Regression test: TestBuiltinMinter_PrefersBundleScope. HIGH fixes 7. plugins/memory/sqlite/sqlite.go Query — the previous semantics included the over-cap row with Truncated=true, which (a) let UsedTokens exceed MaxTokens on the host's budget aggregator and (b) conflated "this row was elided" with "iteration stopped here". The new semantics stop iteration without appending the row that would breach the cap. Regression test: TestQuery_MaxTokensDoesNotExceedCap. 8. internal/cli/instinct_memory_helpers.go — discoverMemoryBackend now honours `instinct.plugin_security.allowlist` in warn-only mode. An empty allowlist resolves to "v0.5 bundled plugins only" (= sqlite reference-fallback); anything else triggers a stderr NOTICE every spawn so users see "untrusted third-party plugin running" the SECURITY.md warned about. 10. internal/observer/claude_jsonl.go — `admitted` is now an atomic.Int64. The previous plain int was read on return and written from the time.AfterFunc-driven flush goroutine, which the race detector flagged. MEDIUM / LOW piggybacks * sqlite.go: removed the dead `_ = filepath.Join` import guard and the unused `_ = domainJSON` after the bug fix; fixed the `workreeFromScopeID` → `worktreeFromScopeID` typo; threaded `source_event_id` through Scan as a named local (was binding via `new(string)` and discarding). * internal/instinct/redaction.go: removed the misleading `var _ sync.Locker = (*sync.Mutex)(nil)` "compile-time guard" that referenced an unused package. New unit coverage * internal/instinct/instinct_test.go: 9 tests covering Coordinator.Close (nil events), ConfidencePolicy ceiling + reinforce, PoisoningGuard mode dispatch + active cap + TTL, BuiltinMinter scope override, Promote chain validation + backend-pair behaviour + invalid transition rejection, DecayScheduler candidate expiry. * plugins/memory/sqlite/sqlite_regression_test.go: 4 tests pinning the CRITICAL fixes against the SQLite Provider directly. Verifying go test -race -count=1 ./internal/instinct/... ./plugins/memory/sqlite/... # → 11 new tests + existing tests all green go build -o dist/bough-plugin-memory-sqlite ./cmd/bough-plugin-memory-sqlite BOUGH_CONFORMANCE_MEMORY_PLUGIN_BIN=$PWD/dist/bough-plugin-memory-sqlite \ go test -tags=conformance -race -count=1 ./plugins/memory/sqlite/... # → Lifecycle + Bloat + Concurrency green against the real binary Outstanding (= follow-up issues, not blocker for v0.5 ship) * HIGH #11 file watch rotation fd leak — analysed and is not an actual leak (Go defers bind to variable, not value), kept as comment for clarity. No code change. * MEDIUM #15 fallback_on_error field set but unread — YAML vocabulary frozen for v0.6; documented in INSTINCTS.md. * MEDIUM #17 Import counts rows but does not re-Store — explicit v0.6 deferral per CONTRACT.md.
Merged
5 tasks
ikeikeikeike
added a commit
that referenced
this pull request
Jun 20, 2026
…#15 + #17) MEDIUM #15: coordinator.Query now consumes instinct.fallback_on_error. On a primary backend error with the flag set and a fallback backend wired in, the same QueryReq is replayed against the fallback and a query_fallback audit event is emitted. v0.5 ships only SQLite so callers pass nil for fallback (= no-op pass-through); v0.6 wires mem0 + SQLite together and the flag starts mattering. MEDIUM #17: sqlite.Import used to walk the YAML / JSONL payload and increment counters without ever re-Storing the rows, so an Import after Forget left the table empty. The new parseExportedYAML / parseExportedJSONL helpers reconstruct memapi.Instinct records and route each through the same Store path the host uses for fresh ingest (UpsertSemantics=true). The CLI reports imported / upserted / skipped so an operator can confirm the round-trip. LOW #18 follow-on: loadInstinctCoordinator anchors the default .bough/memory/events.jsonl against the monorepo root the loadConfigAndRoot helper resolves. Combined with the NewEventWriter absolute-path requirement landed in the previous commit, the CLI now writes to the same file regardless of cwd. instinct.New takes a fourth parameter (fallback) — the package is internal so callers outside the repo are not expected; v0.5.1 stays a drop-in upgrade for plugin authors and end users.
ikeikeikeike
added a commit
that referenced
this pull request
Jun 21, 2026
… propagation / cache gen / cache namespace / Export pagination #10 instinctToMetadata + writeInstinct skip zero-value time.Time so a freshly constructed Instinct does not serialise a -6795364578871 epoch into mem0 metadata or the JSONL export (which would round-trip back as a garbage "year 0001" timestamp). #11 The YAML / JSONL Export now carries dedupe_key + source_event_id alongside each row, and Import's parser returns an importRow struct that preserves both tokens. The host's Store-by-dedupe contract therefore survives sqlite Export → mem0 Import round trips end-to-end. #12 queryCache gains a monotonic invalidation generation. Query captures gen before the HTTP roundtrip and hands it to put; put silently drops the write when invalidateScope has bumped gen in the interim. A concurrent Store / Forget / Import can no longer race a slow Query and pin a stale response in cache for cacheTTL. #13 cacheKey gains a namespace field, populated via the new Provider.cacheKeyFor method, so two tenants sharing one Provider with different namespaces never share cache entries even when Scope shape matches. #15 Export now paginates: it loops GET /api/v1/memories/?page=N until a less-than-page-size batch comes back. Long scopes (>100 instincts) no longer silently truncate at the first page. Build clean, plugins/memory/mem0 unit tests pass.
ikeikeikeike
added a commit
that referenced
this pull request
Jun 21, 2026
… propagation / cache gen / cache namespace / Export pagination #10 instinctToMetadata + writeInstinct skip zero-value time.Time so a freshly constructed Instinct does not serialise a -6795364578871 epoch into mem0 metadata or the JSONL export (which would round-trip back as a garbage "year 0001" timestamp). #11 The YAML / JSONL Export now carries dedupe_key + source_event_id alongside each row, and Import's parser returns an importRow struct that preserves both tokens. The host's Store-by-dedupe contract therefore survives sqlite Export → mem0 Import round trips end-to-end. #12 queryCache gains a monotonic invalidation generation. Query captures gen before the HTTP roundtrip and hands it to put; put silently drops the write when invalidateScope has bumped gen in the interim. A concurrent Store / Forget / Import can no longer race a slow Query and pin a stale response in cache for cacheTTL. #13 cacheKey gains a namespace field, populated via the new Provider.cacheKeyFor method, so two tenants sharing one Provider with different namespaces never share cache entries even when Scope shape matches. #15 Export now paginates: it loops GET /api/v1/memories/?page=N until a less-than-page-size batch comes back. Long scopes (>100 instincts) no longer silently truncate at the first page. Build clean, plugins/memory/mem0 unit tests pass.
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.
Summary
auba-api crashed at ES init even though `http://127.0.0.1:56205\` was
reachable from the host:
`172.17.0.4` is the elasticsearch container's bridge IP. The
`olivere/elastic` client opened the host-reachable port, sniffed
`_nodes/http`, learned the node was at `172.17.0.4:9200` (= the
container-internal HTTP address, unreachable from the host), swapped
to that URL, and crashed.
Fix
Two new env vars on the ES container so it advertises the loopback
the operator already uses:
After the fix `_nodes/http` reports
`publish_address: 127.0.0.1:` and sniffing clients dial
the mapping bough already wires.
Tests
`buildDockerEnv(heap, hostPort)` extracted from `dockerUp` so the
regression guards do not need a real Docker daemon.
every pre-fix must-have (single-node, security off, disk
threshold, ES_JAVA_OPTS) are present.
flows through to the JVM heap arg.
All existing tests pass; `golangci-lint run ./...` 0 issues.