Skip to content

refactor(pkg/dockerutil): extract shared Docker helpers from db plugins#9

Merged
ikeikeikeike merged 2 commits into
mainfrom
feat/L5c-dockerutil-pkg
Jun 17, 2026
Merged

refactor(pkg/dockerutil): extract shared Docker helpers from db plugins#9
ikeikeikeike merged 2 commits into
mainfrom
feat/L5c-dockerutil-pkg

Conversation

@ikeikeikeike

@ikeikeikeike ikeikeikeike commented Jun 16, 2026

Copy link
Copy Markdown
Member

Summary

  • Extract duplicated Docker plumbing from 4 plugin docker.go files into shared pkg/dockerutil/ package
  • Each plugin (mysql, postgres, redis, elasticsearch) shipped ~110 lines of identical helpers — client init, image pull-if-missing, container lookup/remove/reuse, host port probe, com.bough.* label schema
  • Plugins now keep only engine-specific concerns (mysqladmin/pg_isready/redis-cli/HTTP readiness probes, env schemes, ES ulimits + chown, persistence/JVM flags)
  • Net diff in plugins: -443 / +73 lines; pkg/dockerutil/ adds 218 prod + 282 test lines

Files

pkg/dockerutil/
  client.go     — NewClient()
  image.go      — PullIfMissing()
  container.go  — LookupByName() / RemoveIfExists() / UpOrReuse()
  network.go    — IsPortFree()
  labels.go     — Labels() + LabelManaged/Engine/Image/HostPort constants
  *_test.go     — 6 unit tests (Labels schema, IsPortFree contract)
  container_integration_test.go — 5 daemon-dependent tests (//go:build integration)

Test plan

  • go build ./... clean
  • go test ./... — 6 unit tests pass + all 4 plugin tests still pass
  • go test -tags integration ./pkg/dockerutil/... — 5 integration tests green against real Docker daemon
  • _smoke-docker-mysql completes Up → ReadyCheck → Down → Cleanup in 38s
  • _smoke-docker-postgres completes full cycle in 8s
  • _smoke-docker-redis completes full cycle in 3s
  • _smoke-docker-elasticsearch (skipped — JVM warmup budget, plugin unit tests + reuse of identical glue cover it)
  • golangci-lint run — net -4 issues (removed 3 unused removeIfExists + 1 minor errcheck on test cleanup; the 6 residual unused dockerCleanup + errcheck cli.Close match the pre-refactor baseline)

Depends on

  • No dependency — generated directly from main

@ikeikeikeike ikeikeikeike mentioned this pull request Jun 17, 2026
Merged
ikeikeikeike added a commit that referenced this pull request Jun 17, 2026
@ikeikeikeike
fix(lint): clear pre-existing errcheck + unused warnings
d27c1f6 
CI on main has been red since v0.2.1 because the Λ-5a Docker backend
landed three classes of lint debt that local `make format-changed`
never caught (errcheck excludes ${io.Closer} family but not the moby
SDK's *client.Client.Close, and unused helpers slipped past
`go vet`):

- defer cli.Close() / defer rc.Close() in 4 plugins: wrap each in a
  `_ = ...Close()` thunk so errcheck stops complaining without losing
  the cleanup semantics
- removeIfExists in 4 plugin docker.go files: dead code since the
  Λ-5d hardening replaced it with the upOrReuse / start-fail-cleanup
  pair, but the function definitions stayed behind
- (*Provider).dockerCleanup in 4 plugin docker.go files: dead code
  because each plugin's public Cleanup() goes through the nix code
  path's os.RemoveAll directly; the docker variant was never wired

`os` import auto-pruned by goimports in mysql / postgres / redis
docker.go after dockerCleanup removal (elasticsearch keeps os for
its MkdirAll + chown helper).

Local verification:

  golangci-lint run ./...   0 issues
  go test ./...             all packages pass

Unblocks PR #8 (Λ-5b auto-detect) and PR #9 (Λ-5c dockerutil pkg)
both of which inherited these warnings from main.
@ikeikeikeike
refactor(pkg/dockerutil): extract shared Docker helpers from db plugins
4c26202 
Each db plugin (mysql, postgres, redis, elasticsearch) shipped identical
copies of the generic Docker plumbing — client construction, image
pull-if-missing, container lookup-by-name, force-remove, --resume-idempotent
reuse, host port probe, and the com.bough.* label schema. Four times
~110 lines of verbatim duplication that any v0.2.x bug-fix had to apply
in lockstep across four files.

This change introduces pkg/dockerutil with five small files (client,
image, container, network, labels) plus unit tests for the daemon-free
helpers (Labels schema, IsPortFree contract). Each plugin's docker.go
now imports pkg/dockerutil and keeps only its engine-specific concerns:
mysqladmin/pg_isready/redis-cli/HTTP readiness probes, MYSQL_*/POSTGRES_*
env schemes, ES ulimits + datadir chown, persistence/JVM/disk-threshold
flags.

Net diff in plugins: -443 / +73 lines. pkg/dockerutil adds ~218 lines of
production code + ~282 lines of tests. Schema-stability comments on
Labels() pin the com.bough.* keys as public API so future renames break
the build instead of silently leaking containers past `bough remove`.

Verification:
- go build ./... clean
- go test ./... — 6 unit tests in pkg/dockerutil pass, all plugin tests
  still pass
- go test -tags integration ./pkg/dockerutil/... — 5 integration tests
  (LookupByName found/missing, RemoveIfExists idempotency, UpOrReuse
  skip-running + remove-stopped) all green against real Docker daemon
- _smoke-docker-{mysql,postgres,redis} cycle through Up→ReadyCheck→
  Down→Cleanup against real images in 38s / 8s / 3s respectively
- golangci-lint: net -4 issues (removed 3 unused removeIfExists from
  plugins; the 6 residual unused/errcheck warnings on cli.Close +
  unused dockerCleanup match the pre-refactor baseline)
@ikeikeikeike
fix(plugins): apply v0.2.2 lint fixes after rebase
e2e629e 
Inherits the wrap-Close + remove-unused-dockerCleanup pattern from
hotfix/v0.2.2-lint-cleanup; needed because the rebase took --theirs
on the conflicting docker.go files which carried this branch's pre-
rebase shape.
@ikeikeikeike
ikeikeikeike force-pushed the feat/L5c-dockerutil-pkg branch from 8ccf330 to e2e629e Compare June 17, 2026 01:24
@ikeikeikeike
ikeikeikeike merged commit 353fae7 into main Jun 17, 2026
1 check passed
@ikeikeikeike
ikeikeikeike deleted the feat/L5c-dockerutil-pkg branch June 17, 2026 01:29
ikeikeikeike added a commit that referenced this pull request Jun 21, 2026
@ikeikeikeike
fix(v0.6.0): review #23 follow-up — instinct export wire + traversal …
42078e7 
…guard + signing docs honest

Three review #23 follow-ups (#9, #14, #16) collected into one commit
because they share the same theme: undo the v0.6 plan's overreach in
the host-side CLI surface and the docs that backed it.

#16 — `bough instinct export` was a Μ-1.12 stub that returned "not
  yet wired". The CLI advertised the command in `--help` though, so an
  operator who tried it hit a dead end with no fix-it pointer. Wire
  it through Coordinator.Export (new method, mirrors the Import flow:
  delegates to backend.Export, emits an audit event keyed `export`).
  Same scope-inference shape as newInstinctPromoteCmd so the two
  commands see the same default scope.

#14 — internal/cli/capability.writeEmissions was joining
  emitter-supplied filenames straight into outDir. v0.6.0 ships three
  first-party emitters that all produce safe filenames, but the
  emitter registry is plugin-extensible from v0.6.x, and a buggy or
  malicious emitter returning `../../etc/foo` would have landed bytes
  outside the operator's --out-dir. Add the standard archive/tar-style
  traversal guard: filepath.Clean → absolute-or-".." reject →
  filepath.Rel(outDir) verify the canonical path stays inside.

#9 — internal/config/config.go and docs/SIGNING.md both claimed v0.6
  consumes `require_signed: true` as a spawn-time refuse-to-spawn gate.
  The Ν-1.8 implementation actually shipped the `bough plugin verify`
  CLI only; the spawn-time gate lands in v0.6.x per the four-stage
  timeline already documented in docs/SIGNING.md "Timeline" table.
  Revert the config docstring and the SIGNING.md "Configuration" para
  so the contract bough advertises matches what it enforces.

All three change unit + conformance tests still pass:

  go test ./conformance/...    →  6 packages, all ok
  go test ./internal/...        →  all ok
  go test -tags=conformance ./plugins/memory/sqlite/...  →  ok
ikeikeikeike added a commit that referenced this pull request Jun 21, 2026
@ikeikeikeike
fix(v0.6.0): review #23 follow-up — instinct export wire + traversal …
3a5d300 
…guard + signing docs honest

Three review #23 follow-ups (#9, #14, #16) collected into one commit
because they share the same theme: undo the v0.6 plan's overreach in
the host-side CLI surface and the docs that backed it.

#16 — `bough instinct export` was a Μ-1.12 stub that returned "not
  yet wired". The CLI advertised the command in `--help` though, so an
  operator who tried it hit a dead end with no fix-it pointer. Wire
  it through Coordinator.Export (new method, mirrors the Import flow:
  delegates to backend.Export, emits an audit event keyed `export`).
  Same scope-inference shape as newInstinctPromoteCmd so the two
  commands see the same default scope.

#14 — internal/cli/capability.writeEmissions was joining
  emitter-supplied filenames straight into outDir. v0.6.0 ships three
  first-party emitters that all produce safe filenames, but the
  emitter registry is plugin-extensible from v0.6.x, and a buggy or
  malicious emitter returning `../../etc/foo` would have landed bytes
  outside the operator's --out-dir. Add the standard archive/tar-style
  traversal guard: filepath.Clean → absolute-or-".." reject →
  filepath.Rel(outDir) verify the canonical path stays inside.

#9 — internal/config/config.go and docs/SIGNING.md both claimed v0.6
  consumes `require_signed: true` as a spawn-time refuse-to-spawn gate.
  The Ν-1.8 implementation actually shipped the `bough plugin verify`
  CLI only; the spawn-time gate lands in v0.6.x per the four-stage
  timeline already documented in docs/SIGNING.md "Timeline" table.
  Revert the config docstring and the SIGNING.md "Configuration" para
  so the contract bough advertises matches what it enforces.

All three change unit + conformance tests still pass:

  go test ./conformance/...    →  6 packages, all ok
  go test ./internal/...        →  all ok
  go test -tags=conformance ./plugins/memory/sqlite/...  →  ok
@ikeikeikeike ikeikeikeike mentioned this pull request Jun 28, 2026
Merged
ikeikeikeike added a commit that referenced this pull request Jun 28, 2026
@ikeikeikeike
Merge pull request #51 from ikeikeikeike/feat/ecc-eval-fidelity
c450b3f 
feat(eval): confidence demotion + Sonnet GATE5 judge + obs archive (ECC #4/#8/#9)
@ikeikeikeike ikeikeikeike mentioned this pull request Jul 3, 2026
Closed
ikeikeikeike added a commit that referenced this pull request Jul 4, 2026
@ikeikeikeike
Merge pull request #85 from ikeikeikeike/fix/pr-review-sweep-wave3
05d40dc 
fix: retrospective review fixes for merged PRs #9/#10/#11/#12/#18/#19/#20
This was referenced Jul 16, 2026
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ikeikeikeike