v0.4.0: DB plugin orchestrator → engine plugin orchestrator#17
Merged
Conversation
Adds plugins/engine/api/proto/engine.proto — the v0.4.0 generic
EngineProvider contract that replaces DBProvider. Wire shape changes:
Port int → repeated PortSpec ports (multi-port)
InitialDatabases repeated string → repeated ResourceSpec (type-discriminated)
PortRangeResponse {low, high} → map<string, PortRange> (per-role range)
ReadyCheckRequest.port int → repeated int32 ports
CleanupRequest.port int → repeated int32 ports
DownRequest.port int → repeated int32 ports
PortSpec = {role, port} so single-port engines emit Role:"main" and
multi-port engines (rabbitmq AMQP+Management, kafka broker+controller,
NATS client+monitor) emit one entry per role. ResourceSpec = {type,
name, params} so DB schemas, kafka topics, minio buckets, rabbitmq
vhosts, consul KVs all share the same slot, with engine-specific
tuning carried in `params`.
Synced docs:
plugins/engine/api/CONTRACT.md 11 invariants in prose, refers to
the new multi-port EnvVars naming
convention (BOUGH_<ENGINE>_<ROLE>_PORT)
docs/MIGRATION-v0.3-to-v0.4.md side-by-side YAML, plugin-author
checklist, v0.4.x fallback table,
v0.5.0 removal timeline
Makefile `proto` target regenerates both engine.proto (canonical) and
the legacy db.proto stubs in parallel during the v0.4.0 transition;
the db.proto half goes away in Λ-7.1 when plugins/db/ is git-mv'd
into plugins/engine/.
Builds and the existing test suite stay green — plugins/db/* still
references the old db.proto stubs, plugins/engine/api/proto/ stands
alone with no consumer yet.
plugins/engine/api/ now carries the v0.4.0 contract surface:
interface.go EngineProvider (Up / Down / ReadyCheck / Cleanup /
PortRangeDefault / EnvVars) with the new multi-port
signatures (`ports []int` and `[]PortSpec`)
types.go PortSpec / ResourceSpec / PortRange / UpReq / DownReq
/ EnvVarsReq
client.go host-side grpcClient + portSpecsToProto helpers
server.go plugin-side grpcServer + portSpecsFromProto helpers
plugin.go EngineProviderPlugin go-plugin adapter
handshake.go Handshake (BOUGH_ENGINE_PLUGIN, v2) + LegacyHandshake
(BOUGH_DB_PLUGIN, v1) for v0.4.x fallback to v0.3.x
binaries; LegacyDBProviderPluginKey ditto
shims.go PickMainPort / PickFirstResourceName helpers exposed
for single-port engines to bridge v0.4.0 internals
without rewriting their docker.go signatures
plugins/db/api/ stays put for now — the four existing plugins still
import it. internal/pluginhost is rewired to the engine adapter +
LegacyHandshake fallback in Λ-7.2, then plugins/db/ is git mv'd to
plugins/engine/ in Λ-7.3.
go build ./... + go vet + golangci-lint clean.
…inhost (Λ-7.2 partial)
internal/{config,allocator,registry,pluginhost,cli/{helpers,remove,create}} are
all on the v0.4 EngineProvider rails. Outstanding build breakage:
internal/cli/status.go cfg.Databases → cfg.Engines (3 sites)
internal/cli/verify.go cfg.Databases → cfg.Engines (1 site)
conformance/lifecycle.go prov.PortRangeDefault returns map now;
UpReq/DownReq/EnvVarsReq take new shape
conformance/faults.go api.EngineProvider import alias (now `engineapi`)
Resuming in the next session — picking up at the four files above to
get build-passing, then continuing Λ-7.3 (plugins/db → plugins/engine
git mv + shim).
… composite keys status.go / verify.go switch from cfg.Databases to cfg.Engines and now read PortRanges[role] composite-keyed against the registry's <kind>.<role> shape. conformance/lifecycle.go and conformance/faults.go drop the legacy plugins/db/api import alias in favour of plugins/engine/api: PortRangeDefault returns map[role], UpReq/DownReq/EnvVarsReq pass pointers with PortSpec/ResourceSpec slices, and ReadyCheck/Cleanup take []int port lists. For now the conformance suite hard-codes role "main" — multi-port lifecycle and the matching Config.MainPortRole field land in Λ-7.4. go build ./... + go vet ./... + ./internal/... unit tests all pass.
…natures (Λ-7.3)
Move plugins/db/{mysql,postgres,redis,elasticsearch} to plugins/engine/<kind>
and rewire each Provider against plugins/engine/api:
- Up: req *UpReq (Ports []PortSpec + InitialResources)
- Down: req *DownReq (Ports []int)
- ReadyCheck: ports []int
- Cleanup: datadir string, _ []int
- PortRangeDefault: map[string]PortRange{"main": {Low, High}}
- EnvVars: req *EnvVarsReq (Ports []PortSpec)
The shim helpers api.PickMainPort / api.PickFirstResourceName keep the
single-port engines' internals (dockerUp, dockerDown, MYSQL_DATABASE env,
postgres POSTGRES_DB env, etc.) signature-compatible with the v0.3.x logic.
cmd/bough-plugin-<kind>/main.go switches to api.EngineProviderPluginKey +
api.EngineProviderPlugin. The cmd/_smoke-docker-* helpers move with them.
Makefile + status.go doc-comment updated for the new path; conformance test
header paths now point at plugins/engine/<kind>.
go build ./... + go test ./plugins/engine/... + ./internal/... all pass.
…ble longest-prefix host (Λ-7.4)
The lifecycle test now iterates over every role declared by PortRangeDefault
and allocates one port per role from each range's Low end. Up / EnvVars
receive the full PortSpec slice; ReadyCheck / Down / Cleanup take the
flattened []int. Single-port plugins keep their existing single-PortSpec
shape; multi-port plugins (rabbitmq amqp+management, kafka broker+
controller, NATS client+monitor+cluster) get every role exercised end-to-end.
- Config.MainPortRole defaults to "main"; multi-port plugins override.
Faults always target the main role (port-conflict, datadir-perm and
image-pull-failure are single-role concerns).
- invariants.go::extractDialableAddrs walks each `*_PORT` key back
through `_<segment>` until it finds a matching `*_HOST`, so the
bough naming convention `BOUGH_<ENGINE>_<ROLE>_PORT` shares one
`BOUGH_<ENGINE>_HOST` without per-role host duplication.
- mock_plugin gains BOUGH_MOCK_FAIL_MODE=multi-port: declares two
roles, binds both, emits BOUGH_MOCK_HOST + per-role _PORT/_URL.
- TestRun_MockPlugin_MultiPort_GreenPath drives the full multi-port
lifecycle through the mock subprocess (= every clause of the
contract exercised without a real container image).
- docs/PLUGIN_AUTHOR_GUIDE.md gains a multi-port section with the
rabbitmq author's view of PortRangeDefault, Up, EnvVars and the
MainPortRole knob. Stale plugins/db/ paths fixed.
… / canonical paths (Λ-7.5) - README.md Quick start uses .bough.yaml + schema_version 2 + engines: + port_ranges: + initial_resources, with a commented multi-port rabbitmq example. The lead-in copy drops "database engine" for "engine" and flags rabbitmq/kafka/nats/minio as future additions. - README.md repo layout shows plugins/engine/ (not plugins/db/) and the registry path .bough-ports.json (legacy on fallback). - examples/plugin-template/README.md + conformance_test.go gain a Multi-port section pointing at the PLUGIN_AUTHOR_GUIDE walkthrough and a MainPortRole TODO marker. Stale plugins/db/ paths updated. All host + plugin + conformance tests still pass.
CHANGELOG.md gains a v0.4.0 entry detailing every breaking rename (DBProvider→EngineProvider, plugins/db→plugins/engine, .worktree- isolation.yaml→.bough.yaml, etc.) alongside the v0.4.x fallback table and the v0.5.0 removal timeline, with a follow-up Added section for multi-port engines, Config.MainPortRole, AssertReachable longest- prefix host lookup, and the new shim helpers. .goreleaser.yaml header is refreshed: the v0.1-era "MySQL plugin together" comment now describes the four bundled engine plugins and flags that external plugin authors ship their own binaries. GoReleaser snapshot --clean smoke-built 4 OS/arch × 5 binaries in 49s, each tarball bundling bough + the four bough-plugin-<kind> binaries.
…dable phases
The four cmd/_smoke-docker-<kind>/ binaries were 55-78 lines of copy-pasted
"mkdir tempdir → Up → ReadyCheck → Down → Cleanup" boilerplate that drifted
on cleanup paths, log formatting and error-handling. Extract the shared
lifecycle into internal/smoketool/smoketool.go and reduce each main() to
~15 lines that spell out only its plugin + per-engine tunables.
conformance/lifecycle.go::runLifecycle (172 lines) was four near-identical
t.Run blocks per iteration with inline timing/error boilerplate. Split into
per-phase helpers (runUpPhase / runReadyCheckPhase / runEnvVarsPhase /
runDownPhase) plus runOneIteration, assertPortRangeDefault, assertCleanup,
runNativeProbeIfConfigured. runLifecycle now reads as the contract (~50
lines of orchestration), each phase is a clear ~10-line helper, and the
stale plugins/db/api/CONTRACT.md doc-ref is fixed.
internal/cli/create.go::runCreate (211 lines, 6 numbered inline phases)
split into allocateAllPorts / startEngines / materializeRepositories /
renderEnvLocals / runPostCreateHooks plus detectBackendIfNeeded and
buildEngineExtras. The deferred-kill choreography is preserved (each helper
returns whatever it managed to bring up so the caller's defer fires
regardless of partial-success). The awkward `interface{ Write(...) }`
parameter type is replaced with io.Writer.
go build ./... ./cmd/_smoke-docker-* + go test ./conformance/... ./internal/...
./plugins/engine/... all pass.
`return fmt.Errorf("Up: %w", err)` etc. trip golangci-lint's staticcheck
ST1005 ("error strings should not be capitalized"). Phase log banners
(=== Up ===) stay capitalized — those are TUI labels, not error
messages. Local `nix develop .#ci -c golangci-lint run ./...` clean.
…rd README link The Λ-7.3 git mv (plugins/db → plugins/engine) missed .github/workflows/ci.yml::Run conformance suite, so every matrix cell hit `FAIL ./plugins/db/<plugin>/... [setup failed]` on this PR. While here, drop the coast-guard/coasts comparison paragraph from README.md — the project moved on and the positioning is no longer the most useful framing for new readers.
This was referenced Jun 18, 2026
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
commented
Jul 3, 2026
ikeikeikeike
left a comment
ikeikeikeike
left a comment
Member
Author
There was a problem hiding this comment.
Retrospective /review (high effort, this PR shipped without pre-merge review). 9 findings, all CONFIRMED against current live code. 8 fixed in commit 8e91f6c on fix/pr-review-sweep-0703; the 4 shown below are inline on the lines this PR itself introduced them. The remaining 4 (registry-path split across verify/status/list/backfill, the non-engine bare-key lookup, config.migrateLegacy's engines: overwrite, and Load()'s stale doc comment) are about code this PR either left untouched or where I couldn't cleanly anchor a diff line — summarized in a follow-up comment below. 1 finding (multi-port engines can't actually be provisioned end-to-end) is a real feature gap, not a bundleable fix — filed as issue #76.
ikeikeikeike
commented
Jul 3, 2026
ikeikeikeike
left a comment
ikeikeikeike
left a comment
Member
Author
There was a problem hiding this comment.
Retrospective code review of this merged PR — findings inline below, fixed in commit 8e91f6c on fix/pr-review-sweep-0703.
| if port < db.PortRange[0] || port > db.PortRange[1] { | ||
| probs = append(probs, fmt.Sprintf("%s port %d outside range %v", db.Kind, port, db.PortRange)) | ||
| for _, eng := range cfg.Engines { | ||
| for role, portRange := range eng.PortRanges { |
Member
Author
There was a problem hiding this comment.
This per-role loop demands a registry entry for every role declared in eng.PortRanges, but allocateEngines (create.go) only ever allocates/writes the "main" role (self-documented there as "v0.4.0: single-port engines only"). Any .bough.yaml engine declaring more than one port_ranges role — a valid shape per config validation, and the one docs/PLUGIN_AUTHOR_GUIDE.md shows for future multi-port plugins — permanently fails bough verify with false-positive DRIFT, since nothing will ever populate the extra key.
Fixed in commit 8e91f6c on fix/pr-review-sweep-0703: narrowed to check only "main", matching what create.go actually writes. Filed issue #76 for the underlying "multi-port can't actually be provisioned through the real CLI" gap this papers over.
| _ = ln.Close() | ||
| delete(p.listeners, req.Port) | ||
| // Roll back any newly bound listeners so a retry can succeed. | ||
| for _, ps := range req.Ports { |
Member
Author
There was a problem hiding this comment.
This rollback loop iterates every port in req.Ports, not just the ones newly bound during this call. A role reused from a prior successful Up (the continue a few lines up — up-or-reuse's whole point) gets torn down here as a side effect of an unrelated sentinel-write failure on a different, unrelated role later in the same call. This is the in-tree reference plugin any future rabbitmq/kafka/nats plugin would copy the same regression from.
Fixed in commit 8e91f6c: track newly-bound ports in a separate slice and roll back only those.
| // Handshake first and falls back to LegacyHandshake during the v0.4.x | ||
| // transition so v0.3.x binaries (= bough-plugin-mysql etc that have | ||
| // not been rebuilt against v0.4.0) still spawn. Removed in v0.5.0. | ||
| var LegacyHandshake = plugin.HandshakeConfig{ |
Member
Author
There was a problem hiding this comment.
LegacyHandshake (and LegacyDBProviderPluginKey a few lines below) are dead code from day one of this file: a repo-wide grep confirms internal/pluginhost.DiscoverFromBinary never tries this fallback, and plugins/db/api (the package these doc comments say they mirror) was deleted repo-wide by a later commit — so even the thing they'd fall back to doesn't exist.
Removed in commit 8e91f6c.
| # v0.4.0 transition: regenerate the legacy db.proto too while the | ||
| # old plugins/db/ tree still exists (deleted in Λ-7.1). After that | ||
| # this line goes away. | ||
| protoc -I $(PROTO_DIR_DB) \ |
Member
Author
There was a problem hiding this comment.
The comment two lines up says "After that this line goes away" once plugins/db/ is deleted (Λ-7.1) — plugins/db/ was deleted, but this line never was. make proto has been failing outright (protoc: plugins/db/api/proto: directory does not exist) ever since, silently, since nothing in CI runs make proto itself.
Removed in commit 8e91f6c.
ikeikeikeike
added a commit
that referenced
this pull request
Jul 3, 2026
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.
Generalises bough from "DB plugin orchestrator" to "engine plugin
orchestrator" so middleware (rabbitmq, kafka, nats, minio, …) can be
added as plugins on the same lifecycle as the bundled DB plugins.
Breaking changes are collected into one release; the v0.4.x line
keeps fallbacks for every renamed surface so existing deployments do
not have to migrate in lockstep (removed in v0.5.0).
Breaking renames (each has a v0.4.x fallback)
DBProviderEngineProviderplugins/db/plugins/engine/.worktree-isolation.yaml.bough.yamldatabases:engines:port_range:port_ranges:(per-role map)initial_databases:initial_resources:(typed).worktree-ports.json.bough-ports.json<kind><kind>.<role>BOUGH_DB_PLUGINBOUGH_ENGINE_PLUGINdb-providerengine-providerMulti-port engines
PortRangeDefaultreturnsmap[string]PortRange(one entry per role).UpReq.Portsis[]PortSpecso plugins bind every role.EnvVarsemitsBOUGH_<ENGINE>_HOSTshared + per-roleBOUGH_<ENGINE>_<ROLE>_PORT/_URL.conformance.Config.MainPortRole(default"main") targets faultsat a single role; lifecycle still exercises every declared role.
TestRun_MockPlugin_MultiPort_GreenPathproves the wiringend-to-end against the in-tree mock.
See
CHANGELOG.mdfor the full v0.4.0 entry anddocs/MIGRATION-v0.3-to-v0.4.mdfor the plugin-author / operator migration walkthrough.
Test plan
go build ./...go vet ./...go test ./conformance/... ./internal/... ./plugins/engine/...(single-port + multi-port mock self-tests pass)
goreleaser release --snapshot --clean(4 OS/arch × 5 binaries,archive bundles each set)
redis, elasticsearch}) goes green on this PR
.bough.yamland confirmthe v0.3 → v0.4 deprecation warnings render (handled in
follow-up
eiicon-company/claudebump PR — Λ-7.8)