v0.2.1: hardening (resume idempotency + orphan cleanup + port collision)#7
Merged
Conversation
Three concrete repro-validated fixes surfaced by auba dogfood PDCA:
1. gitwt AddOrAttach: skip silently if `dst` is already a registered
worktree of `repoPath`. Mirrors threecorp
scripts/worktree-create.sh:46-50 "skip if worktree already exists".
Makes `claude --worktree F-X --resume <session>` safe to re-invoke
— every WorktreeCreate fire after the first becomes a no-op
instead of failing with "branch already exists" + "worktree already
checked out" combined error.
2. plugin dockerUp: idempotency via upOrReuse. If a container with
the deterministic name `bough-<engine>-<port>` is already running,
Up returns success without recreating. If it exists but is
stopped, the stale container is force-removed before the fresh
create. Eliminates the 30s teardown-then-recreate penalty on
every resume.
3. plugin dockerUp Start failure cleanup. The just-created container
is force-removed if ContainerStart returns an error, so a retry
is not blocked by a stale name and the operator does not see a
confusing "name already in use" downstream. macOS Docker
Desktop's vpnkit hides 0.0.0.0 publishes from `net.Listen`, so we
also detect the daemon's "port is already allocated" string and
rewrap with `docker ps --filter publish=<port>` as the actionable
next step.
Verified via 3 repro scenarios:
- `bough create F-idem` × 3 in a row:
1st cold 48 s
2nd resume 12 s (all sub-repos "attached", all containers reused)
3rd resume 13 s (same)
No errors, deterministic ports preserved across all three calls.
- mysql smoke against a probe-mysql on the same host port (43501):
Up returns
"mysql docker: host port 43501 is already published by another
container — `docker ps --filter publish=43501` to find it; raw: …"
in <1 s instead of timing out on the daemon. No `bough-mysql-43501`
container is left behind.
- mysql smoke after the probe is stopped:
Up 0.55 s + ReadyCheck 8.9 s + Down 30 s = 42 s clean cycle, no
orphan.
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
…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
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.
Summary
Three concrete repro-validated fixes surfaced by auba dogfood PDCA:
gitwt AddOrAttach idempotency — skip silently if
dstis alreadya registered worktree. Mirrors threecorp
scripts/worktree-create.sh:46-50. Makesclaude --worktree F-X --resume <session>safe to re-invoke.plugin
dockerUpidempotency + start-fail cleanup —upOrReusereturns early when a
bough-<engine>-<port>container is alreadyrunning; if
ContainerStarterrors afterContainerCreate, thejust-created container is force-removed so a retry is not blocked
by a stale name.
Port-collision actionable error — macOS Docker Desktop's vpnkit
hides 0.0.0.0 publishes from
net.Listen, so we instead catch thedaemon's "port is already allocated" string and rewrap with
docker ps --filter publish=<port>as the next step.Repro tests (all pass)
bough create F-idem× 3: 48 s / 12 s / 13 s (resume = no-op, allcontainers reused, no errors)
Test plan
go build ./...succeeds innix develop .#cigo test ./...passesFollow-ups (Λ-5b / others)
upOrReuse/isPortFree/lookupByNameinto a sharedpkg/dockerutil/package (per-plugin duplication noted in 5a)