Skip to content

feat(plugins/db/redis): Redis 7 provider#3

Merged
ikeikeikeike merged 1 commit into
mainfrom
feat/plugin-redis
Jun 14, 2026
Merged

feat(plugins/db/redis): Redis 7 provider#3
ikeikeikeike merged 1 commit into
mainfrom
feat/plugin-redis

Conversation

@ikeikeikeike

Copy link
Copy Markdown
Member

Adds bough-plugin-redis for per-worktree Redis 7 via services-flake.

Same lifecycle shape as bough-plugin-mysql: Up / ReadyCheck (redis-cli PING + TCP fallback) / Down / Cleanup / PortRangeDefault (53000-55999) / EnvVars (BOUGH_REDIS_HOST/PORT/URL).

make test + golangci-lint run clean.

Mirror of the bough-plugin-mysql / postgres shape, swapped for
pkgs.redis (Redis 7 stable line):
  - Up: nix run --impure '.#redis' -- up --tui=false, detached via
    Setsid so the WorktreeCreate hook returns before redis is ready
  - ReadyCheck: redis-cli PING preferred (asserts the PONG reply so
    a half-up daemon answering TCP but rejecting commands fails the
    probe), raw TCP DialTimeout fallback so the host does not require
    redis-cli on PATH
  - Down: graceful nix run … down then lsof + SIGTERM/SIGKILL fallback
    + stray process-compose supervisor cleanup
  - Cleanup: rm -rf <datadir>
  - PortRange default 53000-55999 (out of mysql 42000-44999, postgres
    50000-52999, prior bash-hook 33000-41999)
  - EnvVars: BOUGH_REDIS_HOST / _PORT / _URL — the URL is the
    redis://… form that go-redis / redis-py / ioredis accept verbatim

The embedded flake.nix wraps services-flake's services.redis module
with no requirePass (local dev convention; the operator never exposes
this port to the network).

cmd/bough-plugin-redis stays four lines of plugin.Serve.

.goreleaser.yaml extended for a fourth build target; the archive id
list now ships bough + mysql + redis (postgres lands separately in
its own PR).

Tests cover PortRangeDefault (default + override), EnvVars (HOST /
PORT / URL shape), deployFlake asset extraction (every fragment the
host integration relies on), Cleanup happy + empty-datadir guard.
@ikeikeikeike
ikeikeikeike merged commit 72f8b77 into main Jun 14, 2026
1 check passed
@ikeikeikeike
ikeikeikeike deleted the feat/plugin-redis branch June 14, 2026 06:26
ikeikeikeike added a commit that referenced this pull request Jun 21, 2026
…em0 namespace

Round 4 review #23 surfaced six findings that block v0.6.0 ship.
This commit clears all six in-place; remaining medium / low items
land in follow-up commits.

#1 internal/cli/capability.go: runCompile passed `currentScope(nil,
   ...)`, but currentScope dereferences cfg.Repositories on the
   default path → nil pointer panic on every `bough capability
   compile/preview`. Use an explicit Scope{Level: worktree,
   WorktreeID: "default"} literal until --scope flags land in
   v0.6.x.

#2 + #3 cmd/bough-mcp-server/main.go + internal/mcp/server.go:
   WatchStdin and Server.Run both Read from os.Stdin concurrently,
   so the watchdog stole bytes the JSON-RPC scanner needed. The
   ZOMBIE guard also only triggered on io.EOF, so wrapped errors
   left the goroutine spinning. Both go away by deleting
   WatchStdin entirely and letting Server.Run's bufio.Scanner
   detect EOF — main.go's `defer server.Shutdown()` handles the
   subprocess teardown.

#4 + #5 internal/pluginsign/verifier.go +
   internal/cli/plugin_verify.go: verifyCosign defaulted SigPath
   to ".bundle" but .goreleaser.yaml writes ".sig", and the
   command was missing the --certificate-identity /
   --certificate-oidc-issuer flags cosign 2.x requires for
   keyless verify. Default SigPath now prefers .sig (with .bundle
   fallback for legacy artifacts), CertPath defaults to the
   GoReleaser .pem companion, and the Request grows
   CertIdentity / CertIdentityRegexp / CertOIDCIssuer fields the
   CLI exposes as --cert-identity / --cert-identity-regexp /
   --cert-issuer. PreRunE now validates --pubkey is set for
   minisign and --cert-identity (or regexp) is set for cosign
   BEFORE shelling out so the operator sees an Args error rather
   than a phantom verify failure.

#6 internal/mcp/server.go: handleResourcesRead built
   Scope{Level: level} with no RepoName/WorktreeID, then asked
   the backend for a count. sqlite's scopeID returns "/" for that
   shape, so the filter never matched a real worktree row and
   the resource reported `worktree: 0` even when worktree rows
   existed. The honest fix is to stop returning a faithful count
   the host cannot guarantee: readScopesResource now lists the
   three scope tiers with descriptions and points clients at
   memory.query for counts (v0.6.x will add an explicit count API
   once Server learns about cfg.Repositories).

#7 (review #23) plugins/memory/mem0/namespace.go: scopeToMem0
   worktree case derived rootHash from `RepoName + "/" +
   WorktreeID`, so two worktrees of the same repo got different
   user_ids and the documented user-tier sharing did not hold.
   Use hashShort(RepoName) for both segments; branch identity
   continues to live entirely in session_id.

Build clean, all unit + conformance tests pass, lint 0 issues.
ikeikeikeike added a commit that referenced this pull request Jun 21, 2026
…view #23 #2/#3)

The existing conformance/mcp suite wires Server.Run through io.Pipe,
which CI-greened the pre-fix WatchStdin design where a second
goroutine on os.Stdin would have raced the bufio.Scanner inside
Run. Review #23 #2/#3 surfaced that gap; the WatchStdin function
itself is gone, but the in-process suite still cannot regress-test
the production stdio path.

This test spawns the real bough-mcp-server binary (and its
bough-plugin-memory-sqlite dependency, built into a shared temp dir
so the server's discoverSQLite finds it via PATH) and drives the
MCP initialize handshake through actual stdin/stdout pipes:

  1. Build both binaries into t.TempDir().
  2. Start cmd/bough-mcp-server with PATH=tempdir + sqlite db path.
  3. Write one JSON-RPC initialize frame; read exactly one response.
     A WatchStdin-style second reader would steal the bytes and
     ReadBytes would time out.
  4. Close stdin → Server.Run's bufio.Scanner returns false → main
     returns → defer server.Shutdown() reaps the SQLite plugin.
     If the SQLite subprocess outlives the server, cmd.Wait blocks
     past 10s and the test fires.

The build cost (~10s for two binaries) is paid once per test run.
The test runs without tags so the CI memory job already covers it;
operators running `go test ./...` locally get coverage without
configuring anything extra.

Verified locally:

  go test -run TestSubprocessLifecycle ./conformance/mcp/...
    PASS: TestSubprocessLifecycle (1.65s)
ikeikeikeike added a commit that referenced this pull request Jun 21, 2026
…em0 namespace

Round 4 review #23 surfaced six findings that block v0.6.0 ship.
This commit clears all six in-place; remaining medium / low items
land in follow-up commits.

#1 internal/cli/capability.go: runCompile passed `currentScope(nil,
   ...)`, but currentScope dereferences cfg.Repositories on the
   default path → nil pointer panic on every `bough capability
   compile/preview`. Use an explicit Scope{Level: worktree,
   WorktreeID: "default"} literal until --scope flags land in
   v0.6.x.

#2 + #3 cmd/bough-mcp-server/main.go + internal/mcp/server.go:
   WatchStdin and Server.Run both Read from os.Stdin concurrently,
   so the watchdog stole bytes the JSON-RPC scanner needed. The
   ZOMBIE guard also only triggered on io.EOF, so wrapped errors
   left the goroutine spinning. Both go away by deleting
   WatchStdin entirely and letting Server.Run's bufio.Scanner
   detect EOF — main.go's `defer server.Shutdown()` handles the
   subprocess teardown.

#4 + #5 internal/pluginsign/verifier.go +
   internal/cli/plugin_verify.go: verifyCosign defaulted SigPath
   to ".bundle" but .goreleaser.yaml writes ".sig", and the
   command was missing the --certificate-identity /
   --certificate-oidc-issuer flags cosign 2.x requires for
   keyless verify. Default SigPath now prefers .sig (with .bundle
   fallback for legacy artifacts), CertPath defaults to the
   GoReleaser .pem companion, and the Request grows
   CertIdentity / CertIdentityRegexp / CertOIDCIssuer fields the
   CLI exposes as --cert-identity / --cert-identity-regexp /
   --cert-issuer. PreRunE now validates --pubkey is set for
   minisign and --cert-identity (or regexp) is set for cosign
   BEFORE shelling out so the operator sees an Args error rather
   than a phantom verify failure.

#6 internal/mcp/server.go: handleResourcesRead built
   Scope{Level: level} with no RepoName/WorktreeID, then asked
   the backend for a count. sqlite's scopeID returns "/" for that
   shape, so the filter never matched a real worktree row and
   the resource reported `worktree: 0` even when worktree rows
   existed. The honest fix is to stop returning a faithful count
   the host cannot guarantee: readScopesResource now lists the
   three scope tiers with descriptions and points clients at
   memory.query for counts (v0.6.x will add an explicit count API
   once Server learns about cfg.Repositories).

#7 (review #23) plugins/memory/mem0/namespace.go: scopeToMem0
   worktree case derived rootHash from `RepoName + "/" +
   WorktreeID`, so two worktrees of the same repo got different
   user_ids and the documented user-tier sharing did not hold.
   Use hashShort(RepoName) for both segments; branch identity
   continues to live entirely in session_id.

Build clean, all unit + conformance tests pass, lint 0 issues.
ikeikeikeike added a commit that referenced this pull request Jun 21, 2026
…view #23 #2/#3)

The existing conformance/mcp suite wires Server.Run through io.Pipe,
which CI-greened the pre-fix WatchStdin design where a second
goroutine on os.Stdin would have raced the bufio.Scanner inside
Run. Review #23 #2/#3 surfaced that gap; the WatchStdin function
itself is gone, but the in-process suite still cannot regress-test
the production stdio path.

This test spawns the real bough-mcp-server binary (and its
bough-plugin-memory-sqlite dependency, built into a shared temp dir
so the server's discoverSQLite finds it via PATH) and drives the
MCP initialize handshake through actual stdin/stdout pipes:

  1. Build both binaries into t.TempDir().
  2. Start cmd/bough-mcp-server with PATH=tempdir + sqlite db path.
  3. Write one JSON-RPC initialize frame; read exactly one response.
     A WatchStdin-style second reader would steal the bytes and
     ReadBytes would time out.
  4. Close stdin → Server.Run's bufio.Scanner returns false → main
     returns → defer server.Shutdown() reaps the SQLite plugin.
     If the SQLite subprocess outlives the server, cmd.Wait blocks
     past 10s and the test fires.

The build cost (~10s for two binaries) is paid once per test run.
The test runs without tags so the CI memory job already covers it;
operators running `go test ./...` locally get coverage without
configuring anything extra.

Verified locally:

  go test -run TestSubprocessLifecycle ./conformance/mcp/...
    PASS: TestSubprocessLifecycle (1.65s)
ikeikeikeike added a commit that referenced this pull request Jul 3, 2026
fix: retrospective review fixes for merged PRs #1/#2/#3/#4/#5/#6/#7/#8/#14/#16/#17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant